Optimize merkle proofs for efficient verification in Solidity

[doubledup]

This is an optimisation to proof verification in the beefy-mmr pallet. It's a breaking change for proof verification as it modifies the order in which leaves are hashed. For more context, see #12820.

Sorting each pair of hashes is more efficient because it removes the need to determine the order in which hashes should be combined. The order in which hashes should be combined can be stored in a proof by:

• storing a boolean for each hash in the proof representing the side on which it should be hashed.
• storing the number of leaves and leaf index and walking from the leaf to the root of the complete binary tree with the given number of leaves.
• sorting the hashes when creating & verifying proofs.

This sorting is more efficient than the alternatives because it is a single comparison between hashes and requires no extra space in the proof structure, unlike the other options.

There is no Polkadot companion PR, as there are no changes to the APIs used by Polkadot.

Gas estimates

I've put up a separate repo to show the different proof structures working with verification from Snowbridge (Unoptimised) & OpenZeppelin (Optimised). The gas estimates I get are:

MerkleProofTest:testOptimisedProof() (gas: 11179)
MerkleProofTest:testUnoptimisedProofLengthIndex() (gas: 18899)
MerkleProofTest:testUnoptimisedProofSidesArray() (gas: 18079)

This looks like a 38% reduction in gas costs (100 - 11179/18079*100), but it is an estimate and please call out any issues you see with the testing process.

Tests

OpenZeppelin MerkleProof verification

The test case testOptimisedProofBeefyMerkleTree uses the OpenZeppelin MerkleProof library to verify a proof generated after this change. This MerkleProof library uses the optimization in this PR, so this test shows that this optimization is compatible with an existing proof library. See the test case for the code used to generate the proof.

Substrate

Running cargo test --features=runtime-benchmarks only produces failures for some benchmarks, but the same errors occur on master, at least as of 13664c3.

Polkadot

Set up a Polkadot repo that uses this branch for its Substrate dependencies per the CONTRIBUTING doc. Running cargo test --features=runtime-benchmarks exits successfully. As far as I can tell, this should be a breaking change, but it's possible that there are no test cases in Polkadot for the proofs generated by beefy-mmr.

[cla-bot-2021]

User @doubledup, please sign the CLA here.

[acatangiu]

To add some context: this PR does not target MMR code or logic, but only BEEFY authority set (validator set) Merkle proof.

BEEFY justifications also known as SignedCommitment identify the authority set only by the ValidatorSetId.

pallet-beefy-mmr can then be used to get the BeefyAuthoritySet which ties a ValidatorSetId to a MerkleRoot over all validator keys in that set.

Changing how the above MerkleRoot is built (its internal hashing mechanism) should have no effect on BEEFY protocol or on pallet-mmr. It only affect Light Clients that need this root to confirm that the commitments are signed by the correct validator set.

As such, afaict PR looks good.

[vgeddes]

@acatangiu @doubledup beefy-merkle-tree is also used to generate the extra data stored in MMR leafs. Concretely, this is the root for the latest parachain heads in the current relay chain block:

https://github.com/paritytech/polkadot/blob/8ac29d3846075ec98fbb386d4aa829687df97efe/runtime/rococo/src/lib.rs#L1285

So our BEEFY light client has to verify proofs both for validators and parachain headers.

[Lederstrumpf]

lgtm 👍. I've looked into it and this trick remains compatible with batch proofs & prefix/ancestry proofs (say, prove that mmr root r descends from an older root r').

Prefix proofs won't matter for these authority set membership checks, but batch proofs I reckon you'll eventually wanna use for gas savings in verifyCommitment, so that membership of the entire subset of validators being sampled can be checked within one proof verification (back of the envelope saving third of proof items in worst case for validator set of 300 with subset sample size 10).

[doubledup]

@acatangiu Removed double-hashing as it's not strictly necessary for this change, though it is useful for preventing second-preimage attacks. I can't find an existing issue or PR discussing "double hash" or "second preimage" - is it worth opening an issue to discuss it?

[Lederstrumpf]

bot merge

[Polkadot-Forum]

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/exploring-beefy-authority-set-subsampling-optimisations/3106/1