Neptune-Crypto / neptune-core

anonymous peer-to-peer cash
Apache License 2.0
24 stars 7 forks source link

Review Changes Resulting from Dropping `leaf_index` from `MmrMembershipProof` #170

Closed aszepieniec closed 1 month ago

aszepieniec commented 1 month ago

This is an artificial PR designed to focus reviewer attention on specific changes. Merger or non-merger is irrelevant; and branch asz/transaction-consensus will continue to be the most up-to-date topic branch.

This PR captures a sequence of commits following a dependency update. The new dependency on tasm-lib master points to triton-vm v42-alpha6, which in turn points to the most up-to-date version of twenty-first, which drops the field leaf_index from MmrMembershipProof. This last elision is what required a lot of attention to a) make compile and b) fix consequently failing tests.

A lot of the work to anticipate this change was already done, but you really only know the impact of a change once things start breaking down.

Here is what I want the reviewer's attention on: a lot of the failing tests deal with mutator sets somehow. The tests pass now, but I'm not sure I fixed them in the right way. Divining their intention was difficult, the mechanics of batch applying records with batches of to-be-preserved membership proofs is notoriously difficult. I'm afraid I a) neutered tests in otherwise benign ways; or worse b) neutered tests while breaking the (up till now) (probably) correct mutator set logic.