add AccountsHashConfig to manage parameters #23850
Conversation
| @@ -18,6 +22,28 @@ pub struct PreviousPass { | |||
| pub lamports: u64, | |||
| } | |||
|
|
|||
| /// parameters to calculate accounts hash | |||
There was a problem hiding this comment.
the main change
jeffwashington
force-pushed
the
mar65
branch
2 times, most recently
from
8132317
to
4ff52e0
Compare
jeffwashington
force-pushed
the
mar65
branch
3 times, most recently
from
35c8feb
to
22278c0
Compare
Codecov Report
@@ Coverage Diff @@
## master #23850 +/- ##
=========================================
- Coverage 81.8% 81.7% -0.2%
=========================================
Files 581 583 +2
Lines 158312 159164 +852
=========================================
+ Hits 129518 130041 +523
- Misses 28794 29123 +329 |
runtime/src/accounts_db.rs
Outdated
| )>, | ||
| filler_account_suffix: Option<&Pubkey>, | ||
| num_hash_scan_passes: Option<usize>, | ||
| config: &mut AccountsHashConfig<'_>, |
There was a problem hiding this comment.
Can the HashStats parameter be removed from the config struct so that this parameter no longer needs to be mut? I didn't see anything else that needed to be mutable, and the stats don't seem to be related to configuration IMO. And then the HashStats would stay as its own parameter to this function.
There was a problem hiding this comment.
I can consider this in a future pr.
runtime/src/accounts_hash.rs
Outdated
| @@ -18,6 +22,28 @@ pub struct PreviousPass { | |||
| pub lamports: u64, | |||
| } | |||
|
|
|||
| /// parameters to calculate accounts hash | |||
| pub struct AccountsHashConfig<'a> { | |||
There was a problem hiding this comment.
Do you anticipate this struct being used in more places? If no, then maybe (1) it would be better to have in accounts_db.rs, and (2) nit: maybe rename to CalculateAccountsHashConfig? Wdyt?
There was a problem hiding this comment.
yes, I imagine it will be passed through a few functions. Just trying to keep the name shorter. Maybe CalcAccountsHashConfig satisfies it all.
There was a problem hiding this comment.
#1 - I'm trying to move things out of accounts_db.rs over time since it is so large. This config is related to accounts hashing, so I was thinking I'd put it here.
jeffwashington
force-pushed
the
mar65
branch
2 times, most recently
from
4003ad6
to
e5f8b30
Compare
mergify
bot
dismissed
brooksprumo’s stale review
Pull request has been modified.
Problem
parameters and clarity to calculate accounts are already unmanageably large.
Summary of Changes
Add a config struct to make things more clear and reduce complexity of signatures.
Fixes #