Skip to content

Commit

Permalink
Merge pull request eqlabs#1948 from eqlabs/sistemd/pathfinder-get-pro…
Browse files Browse the repository at this point in the history
…of-might-be-missing

feat(rpc): pathfinder_getProof rpc error if proof missing
  • Loading branch information
sistemd authored Apr 11, 2024
2 parents ad7cc2b + 0f0645a commit 586ef94
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 29 deletions.
8 changes: 4 additions & 4 deletions crates/merkle-tree/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ impl<'tx> ContractsStorageTree<'tx> {
contract: ContractAddress,
block: BlockNumber,
key: &BitSlice<u8, Msb0>,
) -> anyhow::Result<Vec<TrieNode>> {
) -> anyhow::Result<Option<Vec<TrieNode>>> {
let root = tx
.contract_root_index(block, contract)
.context("Querying contract root index")?;

let Some(root) = root else {
return Ok(Vec::new());
return Ok(None);
};

let storage = ContractStorage {
Expand Down Expand Up @@ -183,13 +183,13 @@ impl<'tx> StorageCommitmentTree<'tx> {
tx: &'tx Transaction<'tx>,
block: BlockNumber,
address: &ContractAddress,
) -> anyhow::Result<Vec<TrieNode>> {
) -> anyhow::Result<Option<Vec<TrieNode>>> {
let root = tx
.storage_root_index(block)
.context("Querying storage root index")?;

let Some(root) = root else {
return Ok(Vec::new());
return Ok(None);
};

let storage = StorageTrieStorage {
Expand Down
13 changes: 6 additions & 7 deletions crates/merkle-tree/src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,17 +510,16 @@ impl<H: FeltHash, const HEIGHT: usize> MerkleTree<H, HEIGHT> {
root: u64,
storage: &impl Storage,
key: &BitSlice<u8, Msb0>,
) -> anyhow::Result<Vec<TrieNode>> {
) -> anyhow::Result<Option<Vec<TrieNode>>> {
// Manually traverse towards the key.
let mut nodes = Vec::new();

let mut next = Some(root);
let mut height = 0;
while let Some(index) = next.take() {
let node = storage
.get(index)
.context("Resolving node")?
.context("Node is missing from storage")?;
let Some(node) = storage.get(index).context("Resolving node")? else {
return Ok(None);
};

let node = match node {
StoredNode::Binary { left, right } => {
Expand Down Expand Up @@ -595,7 +594,7 @@ impl<H: FeltHash, const HEIGHT: usize> MerkleTree<H, HEIGHT> {
nodes.push(node);
}

Ok(nodes)
Ok(Some(nodes))
}

/// Traverses from the current root towards destination node.
Expand Down Expand Up @@ -1944,7 +1943,7 @@ mod tests {
storage: &impl Storage,
) -> anyhow::Result<Vec<Vec<TrieNode>>> {
keys.iter()
.map(|k| TestTree::get_proof(root, storage, k))
.map(|k| TestTree::get_proof(root, storage, k).map(Option::unwrap))
.collect()
}

Expand Down
15 changes: 14 additions & 1 deletion crates/rpc/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,21 @@ impl RpcContext {
Self::for_tests_on(pathfinder_common::Chain::SepoliaTestnet)
}

#[cfg(test)]
pub fn for_tests_with_trie_pruning(trie_prune_mode: pathfinder_storage::TriePruneMode) -> Self {
Self::for_tests_impl(pathfinder_common::Chain::SepoliaTestnet, trie_prune_mode)
}

#[cfg(test)]
pub fn for_tests_on(chain: pathfinder_common::Chain) -> Self {
Self::for_tests_impl(chain, pathfinder_storage::TriePruneMode::Archive)
}

#[cfg(test)]
pub fn for_tests_impl(
chain: pathfinder_common::Chain,
trie_prune_mode: pathfinder_storage::TriePruneMode,
) -> Self {
use pathfinder_common::Chain;
use starknet_gateway_client::test_utils::GATEWAY_TIMEOUT;

Expand All @@ -78,7 +91,7 @@ impl RpcContext {
Chain::Custom => unreachable!("Should not be testing with custom chain"),
};

let storage = super::test_utils::setup_storage();
let storage = super::test_utils::setup_storage(trie_prune_mode);
let sync_state = Arc::new(SyncState::default());
let (_, rx) = tokio_watch::channel(Default::default());

Expand Down
4 changes: 4 additions & 0 deletions crates/rpc/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ pub enum ApplicationError {
transaction_index: usize,
error: String,
},
#[error("Proof is missing")]
ProofMissing,
/// Internal errors are errors whose details we don't want to show to the end user.
/// These are logged, and a simple "internal error" message is shown to the end
/// user.
Expand Down Expand Up @@ -131,6 +133,7 @@ impl ApplicationError {
ApplicationError::UnexpectedError { .. } => 63,
// doc/rpc/pathfinder_rpc_api.json
ApplicationError::ProofLimitExceeded { .. } => 10000,
ApplicationError::ProofMissing => 10001,
// https://www.jsonrpc.org/specification#error_object
ApplicationError::GatewayError(_)
| ApplicationError::Internal(_)
Expand Down Expand Up @@ -204,6 +207,7 @@ impl ApplicationError {
"limit": limit,
"requested": requested,
})),
ApplicationError::ProofMissing => None,
ApplicationError::ValidationFailureV06(error) => Some(json!(error)),
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,10 @@ pub mod test_utils {
use std::collections::HashMap;

// Creates storage for tests
pub fn setup_storage() -> Storage {
pub fn setup_storage(trie_prune_mode: pathfinder_storage::TriePruneMode) -> Storage {
use pathfinder_merkle_tree::contract_state::update_contract_state;

let storage = StorageBuilder::in_memory().unwrap();
let storage = StorageBuilder::in_memory_with_trie_pruning(trie_prune_mode).unwrap();
let mut connection = storage.connection().unwrap();
let db_txn = connection.transaction().unwrap();

Expand Down
77 changes: 62 additions & 15 deletions crates/rpc/src/pathfinder/methods/get_proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub enum GetProofError {
Internal(anyhow::Error),
BlockNotFound,
ProofLimitExceeded { limit: u32, requested: u32 },
ProofMissing,
}

impl From<anyhow::Error> for GetProofError {
Expand All @@ -37,6 +38,7 @@ impl From<GetProofError> for crate::error::ApplicationError {
}
GetProofError::BlockNotFound => Self::BlockNotFound,
GetProofError::Internal(internal) => Self::Internal(internal),
GetProofError::ProofMissing => Self::ProofMissing,
}
}
}
Expand Down Expand Up @@ -193,7 +195,8 @@ pub async fn get_proof(
// be a "non membership" proof.
let contract_proof =
StorageCommitmentTree::get_proof(&tx, header.number, &input.contract_address)
.context("Creating contract proof")?;
.context("Creating contract proof")?
.ok_or(GetProofError::ProofMissing)?;
let contract_proof = ProofNodes(contract_proof);

let contract_state_hash = tx
Expand Down Expand Up @@ -224,20 +227,25 @@ pub async fn get_proof(
.context("Querying contract's nonce")?
.unwrap_or_default();

let storage_proofs = input
.keys
.iter()
.map(|k| {
ContractsStorageTree::get_proof(
&tx,
input.contract_address,
header.number,
k.view_bits(),
)
.map(ProofNodes)
})
.collect::<anyhow::Result<Vec<_>>>()
.context("Get proof from contract state treee")?;
let mut storage_proofs = Vec::new();
for k in &input.keys {
let proof = ContractsStorageTree::get_proof(
&tx,
input.contract_address,
header.number,
k.view_bits(),
)
.context("Get proof from contract state tree")?
.ok_or_else(|| {
let e = anyhow!(
"Storage proof missing for key {:?}, but should be present",
k
);
tracing::warn!("{e}");
e
})?;
storage_proofs.push(ProofNodes(proof));
}

let contract_data = ContractData {
class_hash,
Expand Down Expand Up @@ -278,4 +286,43 @@ mod tests {
let err = get_proof(context, input).await.unwrap_err();
assert_matches::assert_matches!(err, GetProofError::ProofLimitExceeded { .. });
}

#[tokio::test]
async fn proof_pruned() {
let context =
RpcContext::for_tests_with_trie_pruning(pathfinder_storage::TriePruneMode::Prune {
num_blocks_kept: 0,
});
let mut conn = context.storage.connection().unwrap();
let tx = conn.transaction().unwrap();

// Ensure that all storage tries are pruned, hence the node does not store historic proofs.
tx.insert_storage_trie(
&pathfinder_storage::TrieUpdate {
nodes_added: vec![(Felt::from_u64(0), pathfinder_storage::Node::LeafBinary)],
nodes_removed: (0..100).collect(),
},
BlockNumber::GENESIS + 3,
)
.unwrap();
tx.commit().unwrap();
let tx = conn.transaction().unwrap();
tx.insert_storage_trie(
&pathfinder_storage::TrieUpdate {
nodes_added: vec![(Felt::from_u64(1), pathfinder_storage::Node::LeafBinary)],
nodes_removed: vec![],
},
BlockNumber::GENESIS + 4,
)
.unwrap();
tx.commit().unwrap();

let input = GetProofInput {
block_id: BlockId::Latest,
contract_address: contract_address!("0xdeadbeef"),
keys: vec![storage_address_bytes!(b"storage addr 0")],
};
let err = get_proof(context, input).await.unwrap_err();
assert_matches::assert_matches!(err, GetProofError::ProofMissing);
}
}
4 changes: 4 additions & 0 deletions doc/rpc/pathfinder_rpc_api.json
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,10 @@
},
"required": ["limit", "requested"]
}
},
"PROOF_MISSING": {
"code": 10000,
"message": "Merkle trie proof is not available"
}
}
}
Expand Down

0 comments on commit 586ef94

Please sign in to comment.