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

Implement proof generation and verification #662

Merged
merged 18 commits into from
Jul 25, 2024

Conversation

danlaine
Copy link

@danlaine danlaine commented May 24, 2024

This PR implements a first pass on proof generation and verification. It:

  • Removes dead code
  • Adds Proof, ProofNode types
  • Changes the ValueDigest type to be generic
  • Removes the Preimage type. I couldn't figure out how to implement a blanket implementation for to_hash and write with the additional generics V and H.
  • Implements Merkle.prove and Proof.verify

@danlaine danlaine self-assigned this May 24, 2024
Base automatically changed from remove-shale-hash-refactor to remove-shale June 24, 2024 15:49
@danlaine danlaine force-pushed the remove-shale-implement-proof branch from 9b1b45d to 129a23e Compare July 10, 2024 19:10
@danlaine danlaine marked this pull request as ready for review July 11, 2024 15:01
@danlaine danlaine changed the title [WIP] Implement proof Implement proof generation and verification Jul 11, 2024
@@ -79,7 +77,7 @@ impl<T: ReadLinearStore> api::DbView for HistoricalRev<T> {
async fn single_key_proof<K: api::KeyType>(
&self,
_key: K,
) -> Result<Option<Proof<Vec<u8>>>, api::Error> {
) -> Result<Option<Proof>, api::Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Removing this generic means each ProofNode can be of a different type, which isn't what we want I think. All ProofNodes within a proof should either be owned or borrowed (currently all of them are borrowed).

I'd rather see the generic here and have ProofNode use the type from it's parent.

Copy link
Author

Choose a reason for hiding this comment

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

The generic is added back in https://github.com/ava-labs/firewood/pull/682/files. I could merge that PR into this one if you want.

firewood/src/hashednode.rs Outdated Show resolved Hide resolved
firewood/src/hashednode.rs Outdated Show resolved Hide resolved
firewood/src/merkle.rs Show resolved Hide resolved
firewood/src/merkle.rs Outdated Show resolved Hide resolved
firewood/src/merkle.rs Outdated Show resolved Hide resolved
firewood/src/proof.rs Outdated Show resolved Hide resolved
@rkuris rkuris self-requested a review July 24, 2024 17:26
@danlaine danlaine changed the base branch from remove-shale to remove-shale-refactor-merkle July 24, 2024 18:30
// TODO do type assertion on error
}
}
// #[ignore = "unimplemented"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should make sure this still compiles. Why was it commented out?

@danlaine danlaine merged commit 8f4eb62 into remove-shale-refactor-merkle Jul 25, 2024
4 checks passed
@danlaine danlaine deleted the remove-shale-implement-proof branch July 25, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants