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

Merkle trees first commit #612

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Merkle trees first commit #612

wants to merge 12 commits into from

Conversation

jonas-lj
Copy link
Contributor

No description provided.

@jonas-lj jonas-lj requested a review from kchalkias June 21, 2023 12:52
@jonas-lj jonas-lj marked this pull request as ready for review June 21, 2023 14:34
fastcrypto-data/Cargo.toml Outdated Show resolved Hide resolved
@jonas-lj jonas-lj requested a review from benr-ml July 24, 2023 06:39
//! # use fastcrypto::hash::Sha256;
//! # use fastcrypto_data::merkle_tree::*;
//! let elements = [[1u8], [2u8], [3u8]];
//! let mut tree = MerkleTree::<32, Sha256, [u8; 1]>::new();
Copy link
Contributor

Choose a reason for hiding this comment

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

const like Sha256Length?

}

impl<const DIGEST_LENGTH: usize, H: HashFunction<DIGEST_LENGTH>, T: AsRef<[u8]>>
MerkleTree<DIGEST_LENGTH, H, T>
Copy link
Contributor

Choose a reason for hiding this comment

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

pls merge with the impl of this struct above

///
/// To avoid second-preimage attacks, a 0x00 byte is prepended to the hash data for leaf nodes (see
/// [LeafHasher]), and 0x01 is prepended when computing internal node hashes (see [InternalNodeHasher]).
pub struct MerkleTree<const DIGEST_LENGTH: usize, H: HashFunction<DIGEST_LENGTH>, T: AsRef<[u8]>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need T here? can we instead have generic insert()?

///
/// To avoid second-preimage attacks, a 0x00 byte is prepended to the hash data for leaf nodes (see
/// [LeafHasher]), and 0x01 is prepended when computing internal node hashes (see [InternalNodeHasher]).
pub struct MerkleTree<const DIGEST_LENGTH: usize, H: HashFunction<DIGEST_LENGTH>, T: AsRef<[u8]>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe instead of requiring AsRef, let's require trait Hashable or just Serialize
(AsRef requires using OnceCell in some cases, which we better avoid unless have to).

MerkleTree<DIGEST_LENGTH, H, T>
{
/// Hash an element using the hash function used for this tree.
pub fn hash(element: &T) -> [u8; DIGEST_LENGTH] {
Copy link
Contributor

Choose a reason for hiding this comment

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

when will this be used?

//! assert!(verifier.verify(index, &elements[index], &proof));
//! ```

use std::borrow::Borrow;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the use case for this data structure? there are many variants of merkle trees, e.g., key-value, with deletions, etc.
Also, what is the expected number of objects, and the expected insertion/deletion pattern?

Copy link
Collaborator

@kchalkias kchalkias left a comment

Choose a reason for hiding this comment

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

Let's resurrect that PR and get some timings as well (part of our light client story)

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