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.

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: proof verification #13

Merged
merged 9 commits into from
May 14, 2024
Merged

feat: proof verification #13

merged 9 commits into from
May 14, 2024

Conversation

rkrasiuk
Copy link
Member

@rkrasiuk rkrasiuk commented May 11, 2024

Description

Implement merkle proof verification algorithm. Also, fixes decoding for BranchNode.

@rkrasiuk rkrasiuk marked this pull request as draft May 11, 2024 14:28
@rkrasiuk rkrasiuk force-pushed the rkrasiuk/proof-verification branch from 93d31ab to a345982 Compare May 13, 2024 12:03
@rkrasiuk rkrasiuk force-pushed the rkrasiuk/proof-verification branch from a345982 to 43561d4 Compare May 13, 2024 12:05
@rkrasiuk rkrasiuk marked this pull request as ready for review May 13, 2024 12:26
Comment on lines -139 to +142
let extension = ExtensionNode::new(nibble, val.to_vec());
let mut child = vec![];
val.to_vec().as_slice().encode(&mut child);
let extension = ExtensionNode::new(nibble, child);
let rlp = extension.as_ref().rlp(&mut vec![]);
assert_eq!(rlp, hex!("c88300646f76657262"));
assert_eq!(rlp, hex!("c98300646f8476657262"));
Copy link
Member

@gakonst gakonst May 13, 2024

Choose a reason for hiding this comment

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

The child has to be a valid rlp encoded value but before it was just a hex

Copy link

Choose a reason for hiding this comment

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

Encoding looks to have been wrong, the extension node's child wasn't encoded as a string, but just plain hex without a header. Would have been decoded as 0x0604060f + 4 single bytes, since each byte of 0x76657262 is <= 0x7F

Comment on lines +115 to +122
impl TrieNode {
/// RLP encodes the node and returns either RLP(Node) or RLP(keccak(RLP(node))).
pub fn rlp(&self, buf: &mut Vec<u8>) -> Vec<u8> {
self.encode(buf);
rlp_node(buf)
}
}

Copy link
Member

Choose a reason for hiding this comment

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

This is double-rlp encoding, could you remind me why?

src/nodes/branch.rs Show resolved Hide resolved
}

// Decode without advancing
let Header { payload_length, .. } = Header::decode(&mut &bytes[..])?;
Copy link
Member

@gakonst gakonst May 13, 2024

Choose a reason for hiding this comment

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

eok had forgotten that ::decode(&mut &bytes[..]) does not advance and it does it manually, but now i wonder why shouldn't we just ::decode(&mut bytes) and let the decoder advance it?

Because values on the stack are RLP encoded, so if we did this we'd also RLP decode and we'd need to re-encode, whereas this is a "peek" at the RLP length and then you just push that to the stack w/o re-encoding & skip to next

src/nodes/branch.rs Show resolved Hide resolved
let mut buf = vec![];

let empty = BranchNode::default();
buf.clear();
Copy link
Member

Choose a reason for hiding this comment

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

unnecessary

src/nodes/branch.rs Outdated Show resolved Hide resolved
src/nodes/branch.rs Outdated Show resolved Hide resolved
Copy link
Member

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

@puma314 this is good to go thank you @rkrasiuk and god almighty

src/proof/verify.rs Outdated Show resolved Hide resolved
src/nodes/branch.rs Show resolved Hide resolved
sparse_node.encode(&mut buf);
assert_eq!(BranchNode::decode(&mut &buf[..]).unwrap(), sparse_node);

let leaf_child = LeafNode::new(Nibbles::from_nibbles(hex!("0203")), hex!("1234").to_vec());
buf.clear();
let leaf_rlp = leaf_child.as_ref().rlp(&mut buf);
Copy link
Member

Choose a reason for hiding this comment

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

NB: This does rlp once, and then rlp_node so it handles the "double rlp"ing which we did manually above

src/nodes/mod.rs Outdated Show resolved Hide resolved
src/nodes/mod.rs Show resolved Hide resolved
src/nodes/mod.rs Outdated Show resolved Hide resolved
src/nodes/mod.rs Show resolved Hide resolved
Comment on lines 16 to 18
proof: I,
root: B256,
key: B256,
Copy link
Member

Choose a reason for hiding this comment

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

API should be: fn(root: B256, key: B256, value: Option<Vec>, proof: I) -> Ok(())

expected_value = match TrieNode::decode(&mut &node[..])? {
TrieNode::Branch(branch) => 'val: {
if let Some(next) = target.get(walked_path.len()) {
let mut stack_ptr = branch.as_ref().first_child_index();
Copy link
Member

Choose a reason for hiding this comment

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

TODO: The Branch Node's stack may have an arbitrary length whereas it should equal to the number of 1s in the state mask

Comment on lines 51 to 57
if branch.state_mask.is_bit_set(index) {
if index == *next {
walked_path.push(*next);
break 'val Some(branch.stack[stack_ptr].clone());
}
stack_ptr += 1;
}
Copy link
Member

Choose a reason for hiding this comment

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

get the next level's node from the stack and increment stack pointer

@rkrasiuk rkrasiuk merged commit 4fc6151 into main May 14, 2024
20 checks passed
@rkrasiuk rkrasiuk deleted the rkrasiuk/proof-verification branch May 14, 2024 05:54
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.

3 participants