Refactor validation of Sapling shielded data in transaction::Verifier

[jvff]

Motivation

As part of NU5, there is a new transaction format and consensus rules (version 5). Zebra already had initial support for the new transaction, but the consensus rules aren't fully implemented. In order to properly validate the new transaction version, some validation code related to Sapling shielded data will be shared between V4 and V5 transactions, so this PRs refactors it to prepare for code sharing.

This is the fourth PR that's part of #1981.

Specifications

The new transaction encoding and consensus rules are detailed in the specification.

Designs

Solution

A new verify_sapling_shielded_data method is created by this PR, and the verify_v4_transaction uses it.

Two tests were also added. Both test if transactions from the test vectors are accepted by the verifier.

These tests might need some sort of fix or workaround for the shared batch verifier worker task issue (#2390). If the CI fails with a Closed error message, then it is likely the same cause. Can #2397 be merged as a temporary solution?

Review

@teor2345

Reviewer Checklist

• Code implements Specs and Designs
• Tests for Expected Behaviour
• Tests for Errors

Follow Up Work

• Rename the proptest-impl feature into something clearer (Rename proptest-impl feature #2424).
• Tests for rejecting invalid transactions according to the transaction rules (Implement transaction::Verifier rejection tests for Sapling #2426).

[teor2345]

These tests might need some sort of fix or workaround for the shared batch verifier worker task issue (#2390). If the CI fails with a Closed error message, then it is likely the same cause. Can #2397 be merged as a temporary solution?

Yes, we can merge #2397 now if you want.

I triaged it as optional because it didn't impact any other work. But now we need it for other things we can merge.

Can you please add an estimate to #2397 before you merge it?
(It is unexpected work in this sprint, so it needs an estimate.)

[teor2345]

Tests for rejecting invalid transactions according to the transaction rules:

These all look like cryptographic tests to me. Can you open a ticket?

Then I'll get one of the cryptographers to triage it, and make sure we're not duplicating that work.

[teor2345]

We could merge this code as-is.

But we might want to change the validation function arguments so they will work for V5 transactions.

[teor2345]

We need to rebase on main, and use the shared runtime.

[jvff]

These all look like cryptographic tests to me. Can you open a ticket?

Then I'll get one of the cryptographers to triage it, and make sure we're not duplicating that work.

Done: #2426.

[teor2345]

It passes