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

Merkle Proofs #716

Merged
merged 28 commits into from
Mar 2, 2018
Merged

Merkle Proofs #716

merged 28 commits into from
Mar 2, 2018

Conversation

antiochp
Copy link
Member

@antiochp antiochp commented Feb 19, 2018

  • Functionality to generate a Merkle proof for a given entry in the output MMR.

  • Merkle proof consists of the following -

    • hash of the node in the tree (leaf node representing an output)
    • hash of the overall root of the tree
    • list of hashes representing the peaks (matches the root when hashed together)
    • list of sibling hashes to construct branch from node to peak
    • list of left/right sibling positions to allow the branch to be reconstructed correctly
  • A Merkle proof allows us to -

    • prove inclusion of the node beneath its peak (via the branch of sibling hashes)
    • prove peaks combine to form overall root
    • prove inclusion of an (unspent) output for a given block_header by verifying the utxo_root matches the root of the Merkle proof
    • if an output is unspent at a given block header then it is at least as old as that block
  • API serializes/deserializes Merkle proofs as hex

  • Wallet maintains Merkle proof (and corresponding block hash) for each unspent coinbase output

  • Spending a coinbase output requires the block hash and Merkle proof to be provided in the input

    • these are used to verify coinbase maturity (without requiring full block data for verification)

TODO -

  • Cannot use rewind to put the MMR back into a state where we can create an old Merkle proof (because of pruning?) - confirm we can only create a Merkle proof at the same time the block is originally created?
    We can now successfully generate a Merkle proof via sumtree.rewind().
    This definitely works for a full archival node.
    This will in principle work for a pruned non-archive node - but we need to confirm this.
  • cleanup test failures

We uncovered a subtle bug in our sumtree.rewind() logic - #727 (FIXED).

@antiochp antiochp changed the title Merkle Proofs [WIP] Merkle Proofs Feb 19, 2018
@antiochp
Copy link
Member Author

antiochp commented Feb 20, 2018

@ignopeverell I keep getting tripped up with MMR rewind...

I think I have managed to (finally) convince myself that we cannot use rewind() to reliably put the output MMR into the correct state for a historical block, and we are unable to create a Merkle proof for the coinbase output for a historical block.
Outputs in the MMR may have been removed/pruned and rewind may not handle this correctly.

So am I right in thinking the Merkle proof can only be created at the time the block itself is created (we cannot reliable regenerate the exact same utxo_root otherwise)?
And that the miner will need to create any necessary Merkle proofs when they successfully mine a block (and presumably store them somewhere to make them available to the wallet)?

@ignopeverell
Copy link
Contributor

I think you're correct. Note that you should be able to rewind to that position for a little while, even in presence of pruning. But when the local storage gets compacted then actual data you would need gets removed. So we could wait 10-20 blocks to avoid storing data that could be useless in the common 1-block reorg case, but beyond that you're taking a chance. Note that waiting a bit sounds like a premature optimization to me at this point, just mentioning it as an example.

@antiochp
Copy link
Member Author

@ignopeverell After fixing rewind() and ignoring the remove list (read_from_file() vs existing read()) we can now generate Merkle proofs reliably and correctly on a full archival node.
As discussed - we should have all the non-leaf nodes in the tree even after pruning/compacting.

Planning to investigate if this works as-is with a pruned tree or if we need to modify the tree compacting behavior to retain leaf siblings to keep Merkle proofs working post-compaction step on the output PMMR.

I think we're getting close to landing this PR (hopefully).

@antiochp antiochp changed the title [WIP] Merkle Proofs Merkle Proofs Mar 1, 2018
@antiochp
Copy link
Member Author

antiochp commented Mar 1, 2018

This is ready for review.
We will want to revisit this when we tackle pruning/compaction of the PMMR (we need to ensure we still have access to the hashes of all siblings, including a potentially removed sibling of the leaf node).
But this works for a "removed" node in a non-compacted MMR currently.

@ignopeverell @yeastplume

Copy link
Contributor

@ignopeverell ignopeverell left a comment

Choose a reason for hiding this comment

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

Looks good to me! Mostly commented on the numerous leftovoer TODOs :-)

api/src/types.rs Outdated
state.serialize_field("output_type", &self.output_type)?;
state.serialize_field("commit", &util::to_hex(self.commit.0.to_vec()))?;
state.serialize_field("switch_commit_hash", &self.switch_commit_hash.to_hex())?;
state.serialize_field("spent", &self.spent)?;
state.serialize_field("proof", &self.proof)?;
state.serialize_field("proof_hash", &self.proof_hash)?;

// TODO - rework this

This comment was marked as spam.

@@ -660,6 +662,30 @@ impl Block {
Ok(())
}

// TODO - how do we verify Merkle Proof here?

This comment was marked as spam.

@@ -470,14 +480,38 @@ impl Transaction {
self.kernels.verify_sort_order()?;
Ok(())
}

// TODO - how do we verify Merkle Proof here?

This comment was marked as spam.

let merkle_proof = coin.merkle_proof.clone();
let merkle_proof = merkle_proof.unwrap().merkle_proof();

// TODO - we need a Merkle Proof here to spend a coinbase output

This comment was marked as spam.

} else {
println!("MISMATCH BECAUSE THE BLOODY THING MISMATCHES");

This comment was marked as spam.

This comment was marked as spam.

@yeastplume
Copy link
Member

Cannot find fault here.. really well done. Ensuring siblings still exist on compaction is a fairly trivial implementation detail, all we need to do is ensure the top peak in the prune list isn't actually excluded (just everything underneath it.

@antiochp
Copy link
Member Author

antiochp commented Mar 2, 2018

@yeastplume yeah I'm planning to go take a look at the compaction stuff later today I think.

@ignopeverell thanks for that - I'd kind of skipped that last pass through all those... working through them now.

@antiochp
Copy link
Member Author

antiochp commented Mar 2, 2018

Just waiting for final travis run then I'll merge.

@antiochp antiochp merged commit cc12798 into mimblewimble:master Mar 2, 2018
@antiochp antiochp deleted the merkle_proofs branch March 2, 2018 20:47
dpc pushed a commit to dpc/grin that referenced this pull request Oct 24, 2018
* family_branch() to recursively call family() up the branch
todo
  - we hit a peak, then we need to get to the root somehow
  - actually get the hashes to build the proof

* wip

* some additional testing around merkle tree branches

* track left/right branch for each sibling as we build the merkle path up

* MerkleProof and basic (incomplete) verify fn

* I think a MerkleProof verifies correctly now
need to test on test case with multiple peaks

* basic pmmr merkle proof working

* MerkleProof now serializable/deserializable

* coinbase maturity via merkle proof basically working

* ser/deser merkle proof into hex in api and wallet.dat

* cleanup

* wip - temporarily saving merkle proofs to the commit index

* assert merkle proof in store matches the rewound version
there are cases where it does not...

* commit

* commit

* can successfully rewind the output PMMR and generate a Merkle proof
need to fix the tests up now
and cleanup the code
and add docs for functions etc.

* core tests passing

* fixup chain tests using merkle proofs

* pool tests working with merkle proofs

* api tests working with merkle proof

* fix the broken comapct block hashing behavior
made nonce for short_ids explicit to help with this

* cleanup and comment as necessary

* cleanup variety of TODOs
@yeastplume yeastplume mentioned this pull request Feb 23, 2022
26 tasks
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

Successfully merging this pull request may close these issues.

3 participants