merkle.rs icon indicating copy to clipboard operation
merkle.rs copied to clipboard

Guard against malformed lemmas.

Open afck opened this issue 7 years ago • 1 comments

#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:

  • Always do full validation anyway, so that you can't even deserialize a lemma whose hashes don't match?

  • Validate only the structure (i.e. the above requirement) on deserialization?

  • Change the struct definition into one of the following?

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>)>,
}

afck avatar Jun 15 '18 07:06 afck

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.

psivesely avatar Jun 15 '18 15:06 psivesely