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

Guard against malformed lemmas. #41

Open
afck opened this issue Jun 15, 2018 · 1 comment
Open

Guard against malformed lemmas. #41

afck opened this issue Jun 15, 2018 · 1 comment

Comments

@afck
Copy link
Contributor

afck commented Jun 15, 2018

#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>)>,
}
@psivesely
Copy link
Contributor

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.

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

No branches or pull requests

2 participants