Reexecute block / Pathfinder Integration continuation #306
Conversation
Problem: we need a way to reexecute blocks with Blockifier. Solution: implement the `StateReader` trait using the `starknet-rs` crate to fetch all the relevant data from a Pathfinder node.
Problem: the code needed to only partially load programs based on visited PCs was left as a TODO. We were wondering if this code could be the cause of some hashing issues. Solution: implement bytecode segment loading properly. We should now support visited PCs to a reasonable extent.
notlesh
changed the title
Problem: we are facing issues integrating the model of `OsSingleStarknetStorage` with Pathfinder, as we do not have a method to traverse the tree to reach arbitrary leaves. Solution: Make the OS generic on the per-contract storage type instead of just the storage. This allows users to further customize the behaviour of the OS when it comes to storage.
…notlesh/reexecute-blocks
| } | ||
| } else { | ||
| log::warn!("No class hash available for contract {}", contract_address); | ||
| }; | ||
| } | ||
|
|
||
| // query storage proofs for each accessed contract |
There was a problem hiding this comment.
Its redundant, but add that you are query storage proofs for the class trie. (as difference as previous ones)
There was a problem hiding this comment.
Do you mean that storage proofs should be class proofs?
There was a problem hiding this comment.
something like: query class trie storage proofs for each accessed contract
crates/bin/prove_block/src/main.rs
Outdated
| let class_hashes: Vec<&Felt252> = class_hash_to_compiled_class_hash.keys().collect(); | ||
| let class_proofs = get_class_proofs(&pathfinder_client, &args.rpc_provider, block_number, &class_hashes[..]) | ||
| .await | ||
| .expect("Failed to fetch class proofs"); | ||
|
|
||
| // write facts from class proof | ||
| for proof in class_proofs.values() { | ||
| for node in &proof.class_proof { | ||
| match node { | ||
| TrieNode::Binary { left, right } => { | ||
| // log::info!("writing binary node..."); | ||
| let fact = BinaryNodeFact::new((*left).into(), (*right).into())?; | ||
| fact.set_fact(&mut initial_state.ffc_for_class_hash).await?; | ||
| } | ||
| TrieNode::Edge { child, path } => { | ||
| // log::info!("writing edge node..."); | ||
| let fact = EdgeNodeFact::new((*child).into(), NodePath(path.value.to_biguint()), Length(path.len))?; | ||
| fact.set_fact(&mut initial_state.ffc_for_class_hash).await?; | ||
| } |
There was a problem hiding this comment.
Here we dont need the previous ones (as with contract proofs)?
Not necessary to add now, but im curious :)
If we need, lests make sure to add this in the ticket that refers to class data stuff
There was a problem hiding this comment.
By previous are you referring to using block_number instead of block_number - 1?
That's a good catch, and probably something really subtle since the class isn't likely to change from block to block...
There was a problem hiding this comment.
Changed to block-1 here: Moonsong-Labs@3386814
That said, I wonder if we actually need both like we do with the storage proofs... @odesenfans WDYT?
There was a problem hiding this comment.
Yup! But in the blocks that decalre stuff can happen! Maybe leave a comment or TODO just for us to be aware of this
There was a problem hiding this comment.
Oh, good point, that's a simple case where this would be relevant.
There was a problem hiding this comment.
crates/bin/prove_block/src/main.rs
Outdated
| }) | ||
| .collect(); | ||
|
|
||
| log::debug!("class_hash_to_compiled_class_hash ({}):", class_hash_to_compiled_class_hash.len()); | ||
| for (ch, cch) in &class_hash_to_compiled_class_hash { |
There was a problem hiding this comment.
put better names to this variables
There was a problem hiding this comment.
| let os_input = StarknetOsInput { | ||
| contract_state_commitment_info, | ||
| contract_state_commitment_info: Default::default(), |
There was a problem hiding this comment.
Why are you putting a default?
There was a problem hiding this comment.
This isn't relevant until the end of the OS when we compute the state roots, which is coming $soon
There was a problem hiding this comment.
Also, see all the commented-out code above this -- it'll be implemented soon
* Move process_function_invocations() out of main() * Remove debug logging * Fail fast when txn has no sender_address * Remove stale comment * Use ffc_for_class_hash instead of clone_with_different_hash() * Remove verbose class hash logging * Query class proofs at previous block * Not afraid of Carpal tunnel * Add TODO about fetching classes at current block * fmt
This PR continues improving the
prove_blockbinary. Specifically:Sepoliablock76793)starknet-coretypes used by PathfinderIssue Number: N/A
Type
Description
Breaking changes?