Fast sync child trie support. #9239
Conversation
cheme
added
A0-please_review
cheme
added
A0-please_review
primitives/state-machine/src/lib.rs
Outdated
| #[derive(PartialEq, Eq, Clone)] | ||
| pub struct KeyValueState { | ||
| /// Storage key in parent states. | ||
| pub parent_storages: Vec<Vec<u8>>, |
There was a problem hiding this comment.
Would it make sense to identify subtree by storage root instead of a key paths?
There was a problem hiding this comment.
Looks doable: instead of removing root from parent trie on response, we would register their storage key and build a payload that work for client.
It is a bit more work but I think it is a good idea.
There was a problem hiding this comment.
4bd673f implements it, it also avoid adding two child trie with same root in export (only for a reply).
| if let Some(e) = response.entries.last() { | ||
| self.last_key = e.key.clone(); | ||
| let mut complete = true; | ||
| if self.last_key.len() == 2 && response.entries[0].entries.len() == 0 { |
There was a problem hiding this comment.
response.entries could be empty here?
Also, why self.last_key.len() == 2? Could use some explaining in the comment.
There was a problem hiding this comment.
It is empty when the whole change set is into a child trie: then we do not have the parent key in parent payload (already define in previous payload). I will add comment.
|
@bkchr @andresilva Would be good to get reviews on this one in for warp sync as well, as it contains changes to the protocol. |
|
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
stale
bot
added
the
A5-stale
|
Just need a second review. Note that it already contains the fix for #9715, wish it was merge 2 month ago and did not have to investigate an refix it. |
stale
bot
removed
the
A5-stale
|
Now that the substrate/polkadot version with the warp sync protocol has been released, changes to the protocol would be incompatible. I guess we need to bump the protocol version. |
|
Yes looks needed. I was also thinking of moving the state sync message in their own protobuf file, but there is not much use in doing it except it separates things. |
|
bot merge |
* state machine proofs. * initial implementation * Remove todo. * Extend test and fix import. * fix no proof, with proof ko. * fix start at logic. * Restore response size. * Rework comments. * Add explicit ref * Use compact proof. * ref change * elaborato on empty change set condition. * KeyValueState renaming. * Do not add two time child trie with same root to sync reply. * rust format * Fix merge. * fix warnings and fmt * fmt * update protocol id to V2
* state machine proofs. * initial implementation * Remove todo. * Extend test and fix import. * fix no proof, with proof ko. * fix start at logic. * Restore response size. * Rework comments. * Add explicit ref * Use compact proof. * ref change * elaborato on empty change set condition. * KeyValueState renaming. * Do not add two time child trie with same root to sync reply. * rust format * Fix merge. * fix warnings and fmt * fmt * update protocol id to V2
This PR extend fast sync to support child trie.
When export reach a child root it includes it in the proof and switch to the child trie proof.
When child trie proof is added it resumes parent trie export.
Start key is now a list of nested trie storage key.
Currently only two level of trie are supported so it could also make sense to switch api from
start_key: Vec<Vec<u8>>tochild_storage_key: Option<Vec<u8>>, start_key: Vec<u8>.0d08f53 switches to compact proof usage (can be revert if needed).