Receipts Trie code together with Generic Struct Extraction and Non-consecutive Blocks#453
Receipts Trie code together with Generic Struct Extraction and Non-consecutive Blocks#453Zyouell wants to merge 55 commits into
Conversation
- Move the dependencies (including version) to the workspace `Cargo.toml`. - Update the available dependencies. As update `alloy` to `0.4` (`0.5` and `0.6` of alloy could not work with integration test). - Keep the current version of `ethereum-types` and `rlp`, since both is related with our forked `eth-trie.rs`. Integration test could work.
nicholas-mainardi
changed the base branch from
zyouell/receipt-rebase
to
zyouell/receipt-ncb-merge
nicholas-mainardi
changed the base branch from
zyouell/receipt-ncb-merge
to
zyouell/receipt-rebase
nicholas-mainardi
left a comment
nicholas-mainardi
left a comment
There was a problem hiding this comment.
Looks definitely better now, thanks for the changes, most of the issues seem addressed. Left some other comments but most of them are minor ones and should be quicker to address than the previous ones
| /// Extension extra input should be a single child proof | ||
| Extension(Vec<u8>), | ||
| /// Branch extra inputs should be a list of child proofs | ||
| Branch(Vec<Vec<u8>>), |
There was a problem hiding this comment.
For Branch and Extension variant I think that we shouldn't have the proofs here as inputs, as proofs are expected to be stored and handled in a storage handled by the caller. Instead we should employ proof identifier here, so that the external code can use these identifiers to fetch the proofs from the proof storage
| "Error while generating proof for node {{ inner: {:?} }}", | ||
| e | ||
| )) | ||
| })?; |
There was a problem hiding this comment.
The generated proof should also be added to the proof storage here, as we do with any other generated proof
There was a problem hiding this comment.
I was under the impression the integration test didn't store proofs for value extraction, only the final extraction proofs, at least the current code in storage_trie.rs doesn't store proofs for any of the nodes it generates proofs for
nicholas-mainardi
left a comment
nicholas-mainardi
left a comment
There was a problem hiding this comment.
Besides for the planner changes, everything else looks good to me! Added another quick comment for the IVC proof logic.
| Some(key) => Ok(Some(self.storage.get_proof_exact(&ProofKey::IVC(key))?)), | ||
| None => Ok(None), | ||
| }, | ||
| Err(_) => Ok(None), |
There was a problem hiding this comment.
Shouldn't we return the error here instead? I think that we are returning None because we are relying on the fact that get_previous_epoch will return an error only if the current block number bn does not exist in the tree, which would happen if this is the first block we are proving with IVC and there were no events extracted. But this looks as a rather unsafe assumption to me. I think it would be better to check, before calling get_previous_epoch, whether index.current_epoch() == bn, and return None if this isn't the case; if we have this check before calling get_previous_epoch, then we can just return the error here. Wdyt?
4 participants
This PR adds the functionality to the Receipt based circuits to work even if a block contains no relevant receipts. It does this by using dummy proofs for the
value_extractionproof on the MPT side and anemptyproof for the block proof on theverifiable_dbside.This way the IVC proof still links all the blocks together but if no rows are added to the table for a specific block the tree doesn't change. This PR resolves CRY-37.