Skip to content

Conversation

@voidash
Copy link
Contributor

@voidash voidash commented Jun 21, 2024

This is the first rewrite of MMR based on https://codeberg.org/treyd/supermerkle.

Closes #74

@voidash voidash requested review from bewakes and delbonis June 21, 2024 10:23
Copy link
Contributor

@bewakes bewakes left a comment

Choose a reason for hiding this comment

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

Looks in shape, some questions and feedbacks.
Seems like the code needs to be run through a formatter once.

Copy link
Contributor

@delbonis delbonis left a comment

Choose a reason for hiding this comment

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

There's a few issues here. I should have been more clear that the "chunk" concept was specific to the original application the provided code was used for and that we just need to talk about it in terms of leafs as hashes. But the main issue we really gotta resolve is the excessive allocations, some bookkeeping, and module layout.

voidash added 2 commits June 25, 2024 17:07
- Concept of chunk is removed
- H type used instead of Sha256 for trait that uses Digest
- new verify function
@john-light john-light marked this pull request as ready for review June 25, 2024 13:11
Copy link
Contributor

@delbonis delbonis left a comment

Choose a reason for hiding this comment

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

The bigger stuff has been resolved but main issue is still that the types are improperly defined/used.

…emove hash_leaves function, \n - remove redundant prooflen from MerkleHash
Copy link
Contributor

@delbonis delbonis left a comment

Choose a reason for hiding this comment

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

Just minor cleanup now.

Comment on lines 220 to 239
/// verifies if the hash of the element at particular index is present as a proof
pub fn verify(&self,proof: &MerkleProof<H>,hash: MerkleHash) -> Result<bool, MerkleError> {
if proof.index as usize>= self.num as usize{
return Err(MerkleError::IndexOutOfBounds);
}

return Ok(proof.proof_verify(self, hash));
}

/// constructs a MerkleProof and then verifies it
pub fn verify_with_cohashes(&self, index: u64,hash: MerkleHash, cohashes: Vec<MerkleHash>) -> Result<bool, MerkleError> {
let proof: MerkleProof<H> = MerkleProof {
prooflen: cohashes.len(),
cohashes,
index,
hasher: PhantomData,
};

self.verify(&proof, hash)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

not new but something like this yes

@delbonis
Copy link
Contributor

I think this is in a good position now. We can roll with it.

@delbonis delbonis merged commit bbd718e into master Jun 29, 2024
@storopoli storopoli deleted the feature/mmr branch November 28, 2024 10:37
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.

Implement MMR util

4 participants