Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Local devnet: fix block less than 10 #426

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Local devnet: fix block less than 10 #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
51c5693
handle when contract data is None
ftheirs Nov 5, 2024
6611d38
handle empty contract_proof
ftheirs Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions crates/bin/prove_block/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,15 @@ pub async fn prove_block(
let updated_root = block_hash_storage_proof.class_commitment.unwrap_or(Felt::ZERO);
let previous_root = previous_block_hash_storage_proof.class_commitment.unwrap_or(Felt::ZERO);

let previous_contract_trie_root = previous_block_hash_storage_proof.contract_proof[0].hash::<PedersenHash>();
let current_contract_trie_root = block_hash_storage_proof.contract_proof[0].hash::<PedersenHash>();
// On devnet and until block 10, the storage_root_idx might be None and that means that contract_proof is empty
let previous_contract_trie_root = match previous_block_hash_storage_proof.contract_proof.first() {
Some(proof) => proof.hash::<PedersenHash>(),
None => Felt252::ZERO,
};
let current_contract_trie_root = match block_hash_storage_proof.contract_proof.first() {
Some(proof) => proof.hash::<PedersenHash>(),
None => Felt252::ZERO,
};

let previous_contract_proofs: Vec<_> =
previous_storage_proofs.values().map(|proof| proof.contract_proof.clone()).collect();
Expand Down
9 changes: 6 additions & 3 deletions crates/bin/prove_block/src/reexecute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use blockifier::transaction::objects::TransactionExecutionInfo;
use blockifier::transaction::transaction_execution::Transaction;
use blockifier::transaction::transactions::ExecutableTransaction;
use cairo_vm::Felt252;
use rpc_client::pathfinder::proofs::{PathfinderProof, TrieNode};
use rpc_client::pathfinder::proofs::{ContractData, PathfinderProof, TrieNode};
use rpc_client::RpcClient;
use starknet::core::types::{BlockId, StarknetError};
use starknet::providers::{Provider as _, ProviderError};
Expand Down Expand Up @@ -163,8 +163,11 @@ pub(crate) fn format_commitment_facts<H: HashFunctionType>(
impl PerContractStorage for ProverPerContractStorage {
async fn compute_commitment(&mut self) -> Result<CommitmentInfo, CommitmentInfoError> {
// TODO: error code
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ftheirs please add a comment that explain the motivation of this fix.
In general storage_proofs needs to have this field.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically which contracts are and why the start having contract_data in block 10

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In pathfinder, the structure from the requested type is defined with Opt:
https://github.com/Moonsong-Labs/pathfinder/blob/c45b23d2d6407079ad19f84ef0b23f913063d7cd/crates/rpc/src/pathfinder/methods/get_proof.rs#L182-L199
Besides, when we work with a devnet, we're returning in an early part of the function. We get a None when we retrieve the contract_state_hash from the DB, and this leads to the early return where most of the fields are empty/none
https://github.com/Moonsong-Labs/pathfinder/blob/c45b23d2d6407079ad19f84ef0b23f913063d7cd/crates/rpc/src/pathfinder/methods/get_proof.rs#L283-L294

When we bump into this condition, the fix just create a default ContractData structure with all fields in zero

let contract_data =
self.storage_proof.contract_data.as_ref().expect("storage proof should have a contract_data field");
let contract_data = match self.storage_proof.contract_data.as_ref() {
None => &ContractData::default(),
Some(data) => data,
};
shamsasari marked this conversation as resolved.
Show resolved Hide resolved

let updated_root = contract_data.root;

let commitment_facts = format_commitment_facts::<PedersenHash>(&contract_data.storage_proofs);
Expand Down
4 changes: 2 additions & 2 deletions crates/rpc-client/src/pathfinder/proofs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl TrieNode {
}
}

#[derive(Debug, Clone, Deserialize)]
#[derive(Debug, Clone, Deserialize, Default)]
pub struct ContractData {
/// Root of the Contract state tree
pub root: Felt,
Expand Down Expand Up @@ -75,7 +75,7 @@ impl ContractData {

#[derive(Debug, Clone, Deserialize)]
pub struct PathfinderProof {
pub state_commitment: Felt,
pub state_commitment: Option<Felt>,
pub class_commitment: Option<Felt>,
pub contract_proof: Vec<TrieNode>,
pub contract_data: Option<ContractData>,
Expand Down
Loading