This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Compact proof utilities in sp_trie. #8574
Merged
Merged
Changes from 18 commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
22b8699
validation extension in sp_io
cheme 4de9f7a
need paths
cheme f0ea2af
arc impl
cheme 71202a9
missing host function in executor
cheme f848b44
io to pkdot
cheme 1979b6a
decode function.
cheme 927a096
encode primitive.
cheme b034ec0
trailing tab
cheme 757ff30
multiple patch
cheme 7dba01b
fix child trie logic
cheme 1a2a637
Merge branch 'master' into validation_io, and revert skip value specific
cheme c948254
Merge branch 'master' into validation_io
cheme 7ef5a41
restore master versionning
cheme 360c324
bench compact proof size
cheme 4edc38a
trie-db 22.3 is needed
cheme 6f899c0
line width
cheme d3eb91c
split line
cheme 4475edd
fixes for bench (additional root may not be needed as original issue was
cheme e6e4682
Merge branch 'master' into validation_io
cheme dba397c
Merge branch 'validation_io' of github.com:cheme/substrate into valid…
cheme c0fc8fd
revert compact from block size calculation.
cheme 8f353dd
Merge branch 'master' into validation_io
cheme 7ebb54d
Merge branch 'master' into validation_io
cheme bf91187
New error type for compression.
cheme c37046e
Adding test (incomplete (failing)).
cheme 7daabb3
Merge branch 'master' into validation_io
cheme cfd923a
There is currently no proof recording utility in sp_trie, removing
cheme f25fc34
small test of child root in proof without a child proof.
cheme 19c132e
remove empty test.
cheme 515c2fd
remove non compact proof size
cheme 8aadbf1
Merge branch 'master' into validation_io
cheme f867d7f
Missing revert.
cheme d639940
proof method to encode decode.
cheme File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ use std::cell::{Cell, RefCell}; | |
use std::collections::HashMap; | ||
|
||
use hash_db::{Prefix, Hasher}; | ||
use sp_trie::{MemoryDB, prefixed_key, StorageProof}; | ||
use sp_trie::{MemoryDB, prefixed_key, StorageProof, encode_compact}; | ||
use sp_core::{ | ||
storage::{ChildInfo, TrackedStorageKey}, | ||
hexdisplay::HexDisplay | ||
|
@@ -118,6 +118,7 @@ pub struct BenchmarkingState<B: BlockT> { | |
read_write_tracker: RefCell<ReadWriteTracker>, | ||
whitelist: RefCell<Vec<TrackedStorageKey>>, | ||
proof_recorder: Option<ProofRecorder<HashFor<B>>>, | ||
proof_recorder_root: Cell<B::Hash>, | ||
} | ||
|
||
impl<B: BlockT> BenchmarkingState<B> { | ||
|
@@ -130,7 +131,7 @@ impl<B: BlockT> BenchmarkingState<B> { | |
let mut state = BenchmarkingState { | ||
state: RefCell::new(None), | ||
db: Cell::new(None), | ||
root: Cell::new(root), | ||
root: Cell::new(root.clone()), | ||
genesis: Default::default(), | ||
genesis_root: Default::default(), | ||
record: Default::default(), | ||
|
@@ -140,6 +141,7 @@ impl<B: BlockT> BenchmarkingState<B> { | |
read_write_tracker: Default::default(), | ||
whitelist: Default::default(), | ||
proof_recorder: record_proof.then(Default::default), | ||
proof_recorder_root: Cell::new(root.clone()), | ||
}; | ||
|
||
state.add_whitelist_to_tracker(); | ||
|
@@ -169,6 +171,7 @@ impl<B: BlockT> BenchmarkingState<B> { | |
self.db.set(Some(db.clone())); | ||
if let Some(recorder) = &self.proof_recorder { | ||
recorder.write().clear(); | ||
self.proof_recorder_root.set(self.root.get()); | ||
} | ||
let storage_db = Arc::new(StorageDb::<B> { | ||
db, | ||
|
@@ -517,14 +520,37 @@ impl<B: BlockT> StateBackend<HashFor<B>> for BenchmarkingState<B> { | |
self.state.borrow().as_ref().map_or(sp_state_machine::UsageInfo::empty(), |s| s.usage_info()) | ||
} | ||
|
||
fn proof_size(&self) -> Option<u32> { | ||
fn proof_size(&self) -> Option<(u32, u32)> { | ||
self.proof_recorder.as_ref().map(|recorder| { | ||
let proof = StorageProof::new(recorder | ||
.read() | ||
.iter() | ||
.filter_map(|(_k, v)| v.as_ref().map(|v| v.to_vec())) | ||
.collect()); | ||
proof.encoded_size() as u32 | ||
let proof_size = proof.encoded_size() as u32; | ||
let proof_recorder_root = self.proof_recorder_root.get(); | ||
let compact_size = if proof_recorder_root == Default::default() || proof_size == 1 { | ||
// empty trie | ||
proof_size | ||
} else { | ||
let compact_proof = encode_compact::<sp_trie::Layout<HashFor<B>>>( | ||
proof, | ||
proof_recorder_root, | ||
); | ||
if compact_proof.is_err() { | ||
panic!( | ||
"proof rec root {:?}, root {:?}, genesis {:?}, rec_len {:?}", | ||
self.proof_recorder_root.get(), | ||
self.root.get(), | ||
self.genesis_root, | ||
proof_size, | ||
); | ||
} | ||
let compact_proof = compact_proof.unwrap(); | ||
compact_proof.encoded_size() as u32 | ||
}; | ||
|
||
(proof_size, compact_size) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please only return the compact size. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
}) | ||
} | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we return here both?
What is the rational behind this?
If we "compress" always, we are not interested in the uncompressed size? (And we actually should not call this compress, maybe something different)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point was to use this to check the size delta from batch.
At this point we 'compress' never, but if 'proof_size' in bench is only here for PoV, then keeping only one value could be fine.
(I don't know if we want to compress 'always' the parachain could choose, even if there is no real point in not doing it).
So yes I can remove it, just it does not really cost much to keep it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it is relevant to have both the compressed and uncompressed size for analysis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is that relevant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This question was also not answered yet.
I don't see any reason to return 2 values here. What is the benefit of this? We will use the compact one only anyway.