-
Notifications
You must be signed in to change notification settings - Fork 2
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 : added pathfinder_getClassProof method #1
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,4 +24,4 @@ pathfinder-var.env | |
.vscode/ | ||
|
||
# mdbook compilation | ||
**/book/ | ||
**/book/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
mod get_proof; | ||
mod get_transaction_status; | ||
|
||
pub(crate) use get_proof::get_proof; | ||
pub(crate) use get_proof::{get_proof, get_proof_class}; | ||
pub(crate) use get_transaction_status::get_transaction_status; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3,7 +3,8 @@ use pathfinder_common::prelude::*; | |||||
use pathfinder_common::trie::TrieNode; | ||||||
use pathfinder_common::BlockId; | ||||||
use pathfinder_crypto::Felt; | ||||||
use pathfinder_merkle_tree::{ContractsStorageTree, StorageCommitmentTree}; | ||||||
use pathfinder_merkle_tree::class::ClassStorageTree; | ||||||
use pathfinder_merkle_tree::{ClassCommitmentTree, ContractsStorageTree, StorageCommitmentTree}; | ||||||
use serde::{Deserialize, Serialize}; | ||||||
use serde_with::skip_serializing_none; | ||||||
|
||||||
|
@@ -16,6 +17,13 @@ pub struct GetProofInput { | |||||
pub keys: Vec<StorageAddress>, | ||||||
} | ||||||
|
||||||
#[derive(Deserialize, Debug, PartialEq, Eq)] | ||||||
pub struct GetProofInputClass { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Following the naming of the endpoint ( |
||||||
pub block_id: BlockId, | ||||||
pub class_hash: ClassHash, | ||||||
pub keys: Vec<StorageAddress>, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here the For the class proof, we do not have a notion of keys: we just want the proof that the class is defined in the class tree. You can just remove this parameter. |
||||||
} | ||||||
|
||||||
// FIXME: allow `generate_rpc_error_subset!` to work with enum struct variants. | ||||||
// This may not actually be possible though. | ||||||
#[derive(Debug)] | ||||||
|
@@ -151,6 +159,18 @@ pub struct GetProofOutput { | |||||
contract_data: Option<ContractData>, | ||||||
} | ||||||
|
||||||
#[derive(Debug, Serialize)] | ||||||
#[skip_serializing_none] | ||||||
pub struct GetProofOutputClass { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
/// Required to verify that the hash of the class commitment and the root of | ||||||
/// the [contract_proof](GetProofOutput::contract_proof) matches the | ||||||
/// [state_commitment](Self#state_commitment). Present only for Starknet | ||||||
/// blocks 0.11.0 onwards. | ||||||
class_commitment: Option<ClassCommitment>, | ||||||
/// Membership / Non-membership proof for the queried contract classes | ||||||
class_proof: ProofNodes, | ||||||
} | ||||||
|
||||||
/// Returns all the necessary data to trustlessly verify storage slots for a | ||||||
/// particular contract. | ||||||
pub async fn get_proof( | ||||||
|
@@ -278,6 +298,95 @@ pub async fn get_proof( | |||||
jh.await.context("Database read panic or shutting down")? | ||||||
} | ||||||
|
||||||
/// Returns all the necessary data to trustlessly verify class changes for a | ||||||
/// particular contract. | ||||||
pub async fn get_proof_class( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
context: RpcContext, | ||||||
input: GetProofInputClass, | ||||||
) -> Result<GetProofOutputClass, GetProofError> { | ||||||
const MAX_KEYS: usize = 100; | ||||||
if input.keys.len() > MAX_KEYS { | ||||||
return Err(GetProofError::ProofLimitExceeded { | ||||||
limit: MAX_KEYS as u32, | ||||||
requested: input.keys.len() as u32, | ||||||
}); | ||||||
} | ||||||
|
||||||
let block_id = match input.block_id { | ||||||
BlockId::Pending => { | ||||||
return Err(GetProofError::Internal(anyhow!( | ||||||
"'pending' is not currently supported by this method!" | ||||||
))) | ||||||
} | ||||||
other => other.try_into().expect("Only pending cast should fail"), | ||||||
}; | ||||||
|
||||||
let storage = context.storage.clone(); | ||||||
let span = tracing::Span::current(); | ||||||
|
||||||
let jh = tokio::task::spawn_blocking(move || { | ||||||
let _g = span.enter(); | ||||||
let mut db = storage | ||||||
.connection() | ||||||
.context("Opening database connection")?; | ||||||
|
||||||
let tx = db.transaction().context("Creating database transaction")?; | ||||||
|
||||||
// Use internal error to indicate that the process of querying for a particular | ||||||
// block failed, which is not the same as being sure that the block is | ||||||
// not in the db. | ||||||
let header = tx | ||||||
.block_header(block_id) | ||||||
.context("Fetching block header")? | ||||||
.ok_or(GetProofError::BlockNotFound)?; | ||||||
|
||||||
let class_commitment = match header.class_commitment { | ||||||
ClassCommitment::ZERO => None, | ||||||
other => Some(other), | ||||||
}; | ||||||
|
||||||
// Generate a proof for this class. If the class does not exist, this will | ||||||
// be a "non membership" proof. | ||||||
let class_proof = ClassCommitmentTree::get_proof(&tx, header.number, input.class_hash) | ||||||
.context("Creating contract proof")? | ||||||
.ok_or(GetProofError::ProofMissing)?; | ||||||
let class_proof = ProofNodes(class_proof); | ||||||
|
||||||
let class_root_exists = tx | ||||||
.class_root_exists(header.number) | ||||||
.context("Fetching class root existence")?; | ||||||
|
||||||
if !class_root_exists { | ||||||
return Ok(GetProofOutputClass { | ||||||
class_commitment, | ||||||
class_proof, | ||||||
}); | ||||||
}; | ||||||
|
||||||
let mut class_proofs = Vec::new(); | ||||||
for k in &input.keys { | ||||||
let proof = ClassStorageTree::get_proof(&tx, header.number, k.view_bits()) | ||||||
.context("Get proof from class tree")? | ||||||
.ok_or_else(|| { | ||||||
let e = anyhow!( | ||||||
"Storage proof missing for key {:?}, but should be present", | ||||||
k | ||||||
); | ||||||
tracing::warn!("{e}"); | ||||||
e | ||||||
})?; | ||||||
class_proofs.push(ProofNodes(proof)); | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Concluding the point on the |
||||||
|
||||||
Ok(GetProofOutputClass { | ||||||
class_commitment, | ||||||
class_proof, | ||||||
}) | ||||||
}); | ||||||
|
||||||
jh.await.context("Database read panic or shutting down")? | ||||||
} | ||||||
|
||||||
#[cfg(test)] | ||||||
mod tests { | ||||||
use pathfinder_common::macro_prelude::*; | ||||||
|
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.
After looking at the Pathfinder implementation, I don't see the point of this structure. While the contract tree is made of one main tree that maps contract addresses to contract state, and then N subtrees that map the storage key and values for each contract, the class tree is unique and maps class hashes to class metadata. This is already implemented by
ClassCommitmentTree
.Long story short,
ClassStorageTree
does not appear to be relevant and IMO can be deleted entirely.