-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: update DB creation circuits for generic extraction (Part 2) #393
base: generic-mpt-extraction
Are you sure you want to change the base?
feat: update DB creation circuits for generic extraction (Part 2) #393
Conversation
b5a75ac
to
8515c8e
Compare
0257177
to
7f9cb61
Compare
verifiable-db/src/cells_tree/mod.rs
Outdated
|
||
let cell = CellWire::new(b); | ||
let values_digest = cell.split_values_digest(b); | ||
let metadata_digest = cell.split_metadata_digest(b); |
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 not using directly split_and_accumulate
methods?
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.
Fixed in commit ca0a971.
.chain(empty_hash.elements) | ||
.chain(iter::once(cell.identifier)) | ||
let metadata_digests = cell.split_metadata_digest(b); | ||
let values_digests = cell.split_values_digest(b); |
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.
Same as above: why not using split_and_accumulate
?
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.
Fixed in commit ca0a971.
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.
Seems to me that on this file there were no changes in that commit? Only the other related comments seems to be addressed, no?
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.
Sorry, I missed this fix, just commit in e37e98e.
Co-authored-by: nicholas-mainardi <[email protected]>
…eric-extraction-tree-creation
PublicInputs::new( | ||
&hash.to_targets(), | ||
&final_digest.to_targets(), | ||
&digest.individual_vd.to_targets(), |
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.
PublicInputs::new( | ||
&node_hash, | ||
&final_digest.to_targets(), | ||
&digest.individual_vd.to_targets(), |
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.
@@ -69,6 +69,8 @@ impl TestContext { | |||
cell.identifier(), | |||
cell.value(), | |||
column.multiplier, | |||
// TODO: Check mpt_metadata = cell.hash? | |||
HashOut::rand(), |
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 don't think we need to check it now because it will be invalid otherwise ?
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.
Yes, it's wrong comment, the mpt_metadata
parameter is deleted in PR #399.
…eric-extraction-tree-creation
…eric-extraction-tree-creation
…eric-extraction-tree-creation
Related section Merkle Tree Creation in Generic Extraction document