Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removes remaining unused accounts data len bits #34732

Merged

Conversation

brooksprumo
Copy link
Contributor

@brooksprumo brooksprumo commented Jan 10, 2024

Problem

There are a few more bits of code related to accounts data meter and accounts data len during transaction processing that are now unused.

Summary of Changes

Each logical change is its own commit:

  • Removes accounts data len param from InvokeContext::new()
  • Removes accounts_data_len_delta field from ProcessedMessageInfo
  • Removes unused accounts data len param from process_message()

@brooksprumo brooksprumo added the work in progress This isn't quite right yet label Jan 10, 2024
@brooksprumo brooksprumo self-assigned this Jan 10, 2024
@brooksprumo brooksprumo removed the work in progress This isn't quite right yet label Jan 10, 2024
/// The change in accounts data len
pub accounts_data_len_delta: i64,
}
pub struct ProcessedMessageInfo {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few options here:

  1. Change to pub struct ProcessedMessageInfo;, without the {}
  2. Remove the struct entirely

Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove it entirely

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 0351a8a

@brooksprumo brooksprumo marked this pull request as ready for review January 10, 2024 18:34
Copy link

codecov bot commented Jan 10, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (88f8d11) 81.8% compared to head (0351a8a) 81.8%.
Report is 10 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #34732   +/-   ##
=======================================
  Coverage    81.8%    81.8%           
=======================================
  Files         824      824           
  Lines      222730   222699   -31     
=======================================
+ Hits       182293   182339   +46     
+ Misses      40437    40360   -77     

@brooksprumo brooksprumo merged commit e9a6bb3 into solana-labs:master Jan 10, 2024
35 checks passed
@brooksprumo brooksprumo deleted the accounts-data-size/remove-misc branch January 10, 2024 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants