Skip to content

Conversation

@frankykevin
Copy link
Member

No description provided.

@vans163 vans163 force-pushed the main branch 2 times, most recently from 4b305d9 to 6824044 Compare December 20, 2025 20:34
Err(e) => {
Ok((atoms::error(), e).encode(env))
}
}
Copy link

Choose a reason for hiding this comment

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

Suggested change
}
}
}

};

// Convert proof nodes to Vec<Vec<u8>>
let proof_nodes: Vec<Vec<u8>> = proof.iter().map(|b| b.as_slice().to_vec()).collect();
Copy link

Choose a reason for hiding this comment

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

Suggested change
let proof_nodes: Vec<Vec<u8>> = proof.iter().map(|b| b.as_slice().to_vec()).collect();
let proof_nodes = proof.iter().map(|b| b.as_slice());

let proof_nodes: Vec<Vec<u8>> = proof.iter().map(|b| b.as_slice().to_vec()).collect();

// Verify the proof
match mpt::verify_proof(root.as_slice(), &index_bytes, &proof_nodes) {
Copy link

Choose a reason for hiding this comment

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

Suggested change
match mpt::verify_proof(root.as_slice(), &index_bytes, &proof_nodes) {
match mpt::verify_proof(root.as_slice(), &index_bytes, proof_nodes) {

Comment on lines +63 to +67
pub fn verify_proof(
root: &[u8],
key: &[u8],
proof: &[Vec<u8>],
) -> Result<Vec<u8>, String> {
Copy link

Choose a reason for hiding this comment

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

Suggested change
pub fn verify_proof(
root: &[u8],
key: &[u8],
proof: &[Vec<u8>],
) -> Result<Vec<u8>, String> {
pub fn verify_proof<'a>(
root: &'a [u8],
key: &'a [u8],
proof: impl Iterator<Item = &'a [u8]>,
) -> Result<Vec<u8>, String> {

Comment on lines +74 to +76
if proof.is_empty() {
return Err("Proof is empty".to_string());
}
Copy link

Choose a reason for hiding this comment

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

Can't be checked if proof is iterator, but should be covered with error below

for (i, node) in proof.iter().enumerate() {
// Verify node hash matches expected
let node_hash = KeccakHasher::hash(node);
if node_hash.as_ref() != expected_hash.as_slice() {
Copy link

Choose a reason for hiding this comment

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

Vec compares to fixed array. if expected_hash != node_hash {

if node_hash.as_ref() != expected_hash.as_slice() {
// For inline nodes (< 32 bytes), the node itself might be the "hash"
if node.len() < 32 && node == &expected_hash {
// Inline node, continue
Copy link

Choose a reason for hiding this comment

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

You've already explained what is being verified above, no need to add an empty branch with comment

let item_count = rlp.item_count().map_err(|e| format!("RLP decode error at node {}: {}", i, e))?;

match item_count {
2 => {
Copy link

Choose a reason for hiding this comment

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

It's more clearer to have const defined above with an explanation

/// # Returns
/// * `Ok(value)` - Proof is valid and value was found
/// * `Err(_)` - Proof is invalid
pub fn verify_proof(
Copy link

Choose a reason for hiding this comment

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

This function requires proper tests to verify the logic inside

Comment on lines +163 to +175
if key_index >= key_nibbles.len() {
// We've consumed all key nibbles, value is in position 16
let value_rlp = rlp.at(16)
.map_err(|e| format!("Failed to get value at branch node {}: {}", i, e))?;
if value_rlp.is_empty() {
return Err(format!("No value at branch node terminus at node {}", i));
}
let value: Vec<u8> = value_rlp.as_val()
.map_err(|e| format!("Failed to decode value at branch node {}: {}", i, e))?;
return Ok(value);
}

let nibble = key_nibbles[key_index] as usize;
Copy link

Choose a reason for hiding this comment

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

Suggested change
if key_index >= key_nibbles.len() {
// We've consumed all key nibbles, value is in position 16
let value_rlp = rlp.at(16)
.map_err(|e| format!("Failed to get value at branch node {}: {}", i, e))?;
if value_rlp.is_empty() {
return Err(format!("No value at branch node terminus at node {}", i));
}
let value: Vec<u8> = value_rlp.as_val()
.map_err(|e| format!("Failed to decode value at branch node {}: {}", i, e))?;
return Ok(value);
}
let nibble = key_nibbles[key_index] as usize;
let Some(&nibble) = key_nibbles.get(key_index) else {
// We've consumed all key nibbles, value is in position 16
let value_rlp = rlp.at(16)
.map_err(|e| format!("Failed to get value at branch node {}: {}", i, e))?;
if value_rlp.is_empty() {
return Err(format!("No value at branch node terminus at node {}", i));
}
let value: Vec<u8> = value_rlp.as_val()
.map_err(|e| format!("Failed to decode value at branch node {}: {}", i, e))?;
return Ok(value);
};
key_index += 1;
let next = rlp.at(nibble as usize)

Comment on lines +116 to +128
if key_index >= key_nibbles.len() {
return Err(format!("Key too short for path at node {}", i));
}
if *nibble != key_nibbles[key_index] {
return Err(format!(
"Path mismatch at node {}, nibble {}: expected {}, got {}",
i,
key_index,
key_nibbles[key_index],
nibble
));
}
key_index += 1;
Copy link

Choose a reason for hiding this comment

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

Suggested change
if key_index >= key_nibbles.len() {
return Err(format!("Key too short for path at node {}", i));
}
if *nibble != key_nibbles[key_index] {
return Err(format!(
"Path mismatch at node {}, nibble {}: expected {}, got {}",
i,
key_index,
key_nibbles[key_index],
nibble
));
}
key_index += 1;
match key_nibbles.get(key_index) {
Some(expected_nibble) if nibble == expected_nibble => key_index += 1,
Some(expected_nibble) => {
return Err(format!(
"Path mismatch at node {}, nibble {}: expected {}, got {}",
i, key_index, expected_nibble, nibble
));
}
None => return Err(format!("Key too short for path at node {}", i)),
}

Comment on lines +32 to +36
if encoded.is_empty() {
return (vec![], false);
}

let first = encoded[0];
Copy link

Choose a reason for hiding this comment

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

Suggested change
if encoded.is_empty() {
return (vec![], false);
}
let first = encoded[0];
let Some(&first) = encoded.first() else {
return (vec![], false);
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants