Rename CRDS SnapshotHash to SnapshotHashes #20421
Conversation
brooksprumo
force-pushed
the
crds-snapshot-hashes
branch
from
ad8958f to
1ca2b74
Compare
| SnapshotHashes(SnapshotHashes), | ||
| AccountsHashes(SnapshotHashes), |
There was a problem hiding this comment.
In this PR comment (#20374 (comment)), @behzadnouri mentioned that following the naming scheme of (old) SnapshotHashes wasn't the right call. So here's the update to make this better.
| pub struct SnapshotHashes { | ||
| pub from: Pubkey, | ||
| pub hashes: Vec<(Slot, Hash)>, | ||
| pub wallclock: u64, | ||
| } |
There was a problem hiding this comment.
And here's the actual struct. The singular "SnapshotHash" was a little strange to me, since there's a vector of hashes inside...
Happy to do so! @mvines Also, looks like the frozen abi digest changed (test failed). This is another case where changing the digest hash is OK, yes? |
Yep. The name here doesn't make it into the ABI. But careful in general, some types names do make it into the RPC interface. Those types usually contain a |
Codecov Report
@@ Coverage Diff @@
## master #20421 +/- ##
=========================================
- Coverage 82.8% 82.8% -0.1%
=========================================
Files 485 485
Lines 135950 135950
=========================================
- Hits 112659 112621 -38
- Misses 23291 23329 +38 |
This reverts commit ceb0179.
In future PRs, I'll be adding new types for handling snapshot hashes. One will be an actual alias (or struct) for
SnapshotHash. I'll be using that in bootstrap, snapshot packager service, and gossip. So, to avoid confusion, I'm renaming theSnapshotHashin CRDS to beSnapshotHashes.Related to #20423