SpinResearch / merkle.rs

:christmas_tree: Merkle tree in Rust
BSD 3-Clause "New" or "Revised" License
128 stars 23 forks source link

Guard against malformed lemmas. #41

Open afck opened 6 years ago

afck commented 6 years ago

36 adds a panic if Lemma::index is called on a malformed Lemma, i.e. one where any of its sublemmas or itself violates the requirement that sibling_hash.is_some() == sub_lemma.is_some(). A malformed lemma currently can't be constructed, but it could certainly be deserialized, and calling validate is expensive because it does a lot of hashing. Should we:

pub struct Lemma {
    pub node_hash: Vec<u8>,
    pub sub_lemma: Option<(Positioned<Vec<u8>>, Box<Lemma>)>,
}

pub struct Lemma {
    pub node_hash: Vec<u8>,
    pub sub_lemma: Option<Box<SubLemma>>, // where `SubLemma` contains the sibling hash and position
}

pub struct Lemma {
    pub count: usize,
    // The positions can be computed from the lemma's index.
    pub index: usize,
    pub node_hash: Vec<u8>,
    pub sub_lemma: Option<(Vec<u8>, Box<Lemma>)>,
}
psivesely commented 6 years ago

I'm not sure I'm strongly in favor of any of these options.

It seems like there must be use cases where the data to be deserialized will be totally trusted, and the first two bullets would add uneccessary overhead.

As far as the structs go, I like the first one the most (I think if we add the index it should go in the Proof struct). Assuming #36 gets merged as-is, then using this structure would allow us to avoid panic in the index methods for Lemma and Proof. However, #42 also would allow us to get rid of these index methods by placing them in the Proof struct and by combining computing the index with proof construction a well as combining its validation with proof validation.