-
Notifications
You must be signed in to change notification settings - Fork 10
feat: customizable LeanIMT
implementation
#62
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
base: main
Are you sure you want to change the base?
feat: customizable LeanIMT
implementation
#62
Conversation
04026bf
to
f94ba61
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good base to start with!
There are some changes and features we will need but let's tackle them incrementally.
crates/lean-imt/src/lean_imt.rs
Outdated
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] | ||
pub struct LeanIMT { | ||
/// Nodes | ||
nodes: Vec<Vec<Vec<u8>>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will have poor cache efficiency as each hash will be at its own heap address. I recommend flattening at least the hashes so its Vec<Vec<u8>>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about this?
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
pub struct LeanIMT<const LEAF_SIZE: usize> {
#[serde(bound(serialize = "[u8; LEAF_SIZE]: Serialize"))]
#[serde(bound(deserialize = "[u8; LEAF_SIZE]: Deserialize<'de>"))]
nodes: Vec<Vec<[u8; LEAF_SIZE]>>,
}
This will fail to compile if LEAF_SIZE
is greater than 32
and you're trying to do serialization/deserialization. Otherwise works just fine. It's an upper limit on LEAF_SIZE
if serde is used. Imo makes sense since this is a common, safe size for a hash output.
By doing this all hashes will be stored in the vec and not at random addresses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue with this is that the hash size has to be known at compile time. I think its not too hard to just use dynamic indexing at runtime depending on the provided hasher.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right, we're forcing a specified output of hash function. I think we could make this temporary tradeoff in favor of performance, and then work on an update for a flattened flexible node structure, so we can have this and semaphore-rs
going for the moment. I can mention this problem in the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to know your thoughts @vplasencia
331d688
to
3a133a7
Compare
Description
This PR introduces an implementation of the Lean Incremental Merkle Tree:
HashedLeanIMT
struct that supports arbitrary hash functions through a traitLeanIMTHasher
trait to allow users to provide their own hashing functionsRelated Issue(s)
Closes lean-imt TLSNotary requirements
Closes Create a crate for the LeanIMT #51
Checklist
cargo fmt
without getting any errors