check Merkleproofs in one place

[tromp]

the verify_inputs method in transaction.rs checks whether merkle proofs of coinbase inputs verify in isolation, that is, without relating their node and root to anything outside the proof.

this is clearly necessary to make the proof relevant and AFAICT is done in method
verify_maturity

                        // Is our Merkle Proof valid? Does node hash up consistently to the root?
                        if !merkle_proof.verify() {
                                return Err(Error::MerkleProof);
                        }

                        // Is the root the correct root for the given block header?
                        if merkle_proof.root != header.output_root {
                                return Err(Error::MerkleProof);
                        }

                        // Does the hash from the MMR actually match the one in the Merkle Proof?
                        if merkle_proof.node != hash {
                                return Err(Error::MerkleProof);
                        }

But that one also verifies the proof. Which makes verify_inputs rather redundant.
(Unless there can be a long time between the two verifies in which resources can be unjustly tied up
I would find the verify method a lot more sensible if it takes node and root as arguments,
allowing for all the above code to be replaced by

                        if !merkle_proof.verify(hash, header.output_root) {
                                return Err(Error::MerkleProof);
                        }

which also avoid the possibility of doing possibly bogus checks in isolation.

Not only that, but it also makes Merkle proofs significantly shorter, since they no longer need to store the node and root themselves.

[antiochp]

👍 Makes sense to me to simplify this and require the node and root to be passed in as args to verify().
Discussed with @tromp on gitter - trivial today to create an invalid Merkle proof that is self-verifying in isolation (against a fake root etc.)

[yeastplume]

housekeeping.. what's the status on this?

[tromp]

Looking at verify_maturity in the current code, it has not been fixed yet...

[ignopeverell]

Believe #1203 is going to get rid of Merkle proofs altogether...

[antiochp]

@tromp - you need to check testnet3 branch.
But as @ignopeverell mentioned - looks like we can verify coinbase maturity without needing to use Merkle proofs.
We think we can leverage them for relative output locktimes.
But we don't need them for coinbase maturity.

[tromp]

@tromp - you need to check testnet3 branch.

the questionable code is no longer present in testnet3 transaction.rs so we can close this issue...

[antiochp]

Closing.