bug: Remove leaf data from BMT proofs #502
Conversation
| .ok_or(MerkleTreeError::LoadError(proof_index))? | ||
| .into_owned(); | ||
| let leaf_node = Node::from(primitive); | ||
| proof_set.push(*leaf_node.hash()); |
There was a problem hiding this comment.
This code block is what added the hashed leaf data to the proof set. This is now removed, and the proof set no longer includes the leaf data.
| let mut proof_set = VecDeque::from(self.proof_set); | ||
| proof_set.pop_front(); |
There was a problem hiding this comment.
This removes the hashed leaf data from the test-helpers reference version of the BMT proof generation. The reference Merkle tree algorithm used here is different than the one used in production. Because of the specifics of the algorithm, we can only remove the leaf data from the proof after the proof is generated.
bvrooman
requested review from
xgreenx and
a team
and removed request for
xgreenx
| @@ -1,42 +1,44 @@ | |||
| use crate::binary::{ | |||
There was a problem hiding this comment.
Maybe it makes sense to re-use the fuel_merkle::binary::verify function here?
There was a problem hiding this comment.
I think that reusing the function here is worth considering.
As part of that consideration, some factors include:
- Do we have a production use case for
fuel_merkle::binary::verifyyet? If not, it might actually make more sense to keep onlytest_helpers::binary::verifyinstead.test_helpers::binary::verifyis used specifically to verify proofs during tests (i.e. check the proofs are generated correctly), butfuel_merkle::binary::verifydoesn't have a "real" use case. - Can we simplify or improve the implementation of
fuel_merkle::binary::verify? If so, we can usetest-helpers::binary::verifyto quickly check the correctness of any refactors to the production version.
Related issues:
fuel-merkleBMT proofs #501