feat: resharding proof #12418
feat: resharding proof #12418
Conversation
Longarithm
force-pushed
the
sw-r-proof
branch
from
96aa280 to
eb5cc5c
Compare
| pub nodes: HashMap<CryptoHash, Arc<[u8]>>, | ||
| /// Allows using in-memory tries to construct the trie node changes entirely | ||
| /// (for both in-memory and on-disk updates) because it's much faster. | ||
| pub enum TrackingMode<'a> { |
There was a problem hiding this comment.
Oh this is an infinitely better API! Thank you so much!
| .entry(value) | ||
| .and_modify(|rc| *rc += 1) | ||
| .or_insert(1); | ||
| nodes_tracker.refcount_inserted_values.entry(value).and_modify(|rc| *rc += 1).or_insert(1); |
There was a problem hiding this comment.
Quick question, isn't it incorrect to return early for the GenericTrieValue::MemtrieAndDisk case above? Why would we not want to track those nodes?
There was a problem hiding this comment.
Oh yeah, that's confusing method.
In fact, we return early in MemtrieOnly node, not MemtrieAndDisk. We do it because we have don't have full value, so we can't compute its hash and we can't make a record.
Ideally GenericTrieValue should be consistent with nodes_tracker by design. That's a TODO for the future.
Made a little reordering to check GenericTrieValue enum variant only once.
| let nodes_tracker = match mode { | ||
| TrackingMode::None => None, | ||
| TrackingMode::Refcounts => Some(TrieChangesTracker::default()), | ||
| TrackingMode::RefcountsAndAccesses(recorder) => Some(TrieChangesTracker { |
There was a problem hiding this comment.
nit: TrieChangesTracker::new_with_recorder(recorder)?
There was a problem hiding this comment.
what about initializing a TrieChangesTracker from a TrackingMode?
| let partial_state_len = match &partial_state { | ||
| let trie_changes = mem_trie_update.retain_split_shard(&boundary_account, retain_mode); | ||
| let partial_storage = trie_recorder.recorded_storage(); | ||
| let partial_state_len = match &partial_storage.nodes { |
| let nodes_tracker = match mode { | ||
| TrackingMode::None => None, | ||
| TrackingMode::Refcounts => Some(TrieChangesTracker::default()), | ||
| TrackingMode::RefcountsAndAccesses(recorder) => Some(TrieChangesTracker { |
There was a problem hiding this comment.
what about initializing a TrieChangesTracker from a TrackingMode?
| fn record<M: ArenaMemory>(&mut self, node: &MemTrieNodeView<'a, M>) { | ||
| let node_hash = node.node_hash(); | ||
| let raw_node_serialized = borsh::to_vec(&node.to_raw_trie_node_with_size()).unwrap(); | ||
| self.refcount_deleted_hashes.entry(node_hash).and_modify(|rc| *rc += 1).or_insert(1); |
There was a problem hiding this comment.
nit: Would the following work?
self.refcount_deleted_hashes.entry(node_hash).or_default() += 1;
// or
self.refcount_deleted_hashes.entry(node_hash).or_insert(0) += 1;
| root: CryptoHash, | ||
| track_trie_changes: bool, | ||
| ) -> Result<MemTrieUpdate<HybridArenaMemory>, StorageError> { | ||
| mode: TrackingMode<'a>, |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12418 +/- ##
==========================================
- Coverage 71.70% 71.43% -0.27%
==========================================
Files 843 838 -5
Lines 170777 169315 -1462
Branches 170777 169315 -1462
==========================================
- Hits 122454 120954 -1500
- Misses 42964 43013 +49
+ Partials 5359 5348 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Generate proof for resharding (retain_multi_range) and check that it is valid against existing retain tests, already including memtrie and disktrie.
Surprisingly, #12390 allows to make the change almost invisible. The actual work here is to move
&mut TrieRecorderinsideTrieChangesTrackerwhich is insideMemTrieUpdate.Reasoning: when we work with
MemTrieUpdate, it is in fact unique owner of the logic to record proof (what I called "2nd stage" in the previous PR, after "1st stage" of trie lookups). And now, if we are sure that proof doesn't need values,MemTrieUpdatecan fully hold the&mut, because memtrie has all the necessary information, and we can record nodes directly on node access instead of doing it retroactively (which was really bad).TrieRecordernow is passed as a separate mode to process memtrie updates. We indeed need three modes - on loading we don't record anything, for non-validators we save trie changes, for validators we also save proofs. And after that, all we need to do forretain_split_shardis to createTrieRecorderand in the end get recorded storage from it.Next step will be to validate this proof in the actual state witness processing...