* Validate transparent coinbase output maturity and shielding
- Add a CoinbaseSpendRestriction enum and Transaction method
- Validate transparent coinbase spends in non-finalized chains
* Don't use genesis created UTXOs for spends in generated block chains
* Refactor out a new_transaction_ordered_outputs function
* Add Transaction::outputs_mut for tests
* Generate valid transparent spends in arbitrary block chains
* When generating blocks, fixup the block contents, then the block hash
* Test that generated chains contain at least one transparent spend
* Make generated chains long enough for reliable tests
* Add transparent and shielded input and output methods to Transaction
* Split chain generation into 3 functions
* Test that unshielded and immature transparent coinbase spends fail
* Comment punctuation
* Clarify a comment
* Clarify probability calculation
* Test that shielded mature coinbase output spends succeed
* Add an OrderedUtxo type for validation of spends within a block
This change allows us to check that transparent spends use outputs from
earlier in their block. (But we don't actually do that check yet.)
We need to keep the order of UTXOs when we're contextually verifying
each new block that is added to a chain. But the block order is
irrelevant for UTXOs stored in the state.
* Take ownership in utxos_from_ordered_utxos
* Delete a confusing comment
* Always use librustzcash for sighash and remove old sighash code
Also added ZIP-143 test vectors
* Remove librustzcash_sighash test that is no longer needed
* Make `verify_sapling_shielded_data` more generic
Prepare to support V5 transactions which have a shared anchor.
* Verify Sapling shielded data in V5 transactions
Call the `verify_sapling_shielded_data` method and add the respective
asynchronous checks to the set of V5 checks.
* Fix expect message in V4 transaction test
It was using the same message as the previous test, even though the test
searches with different criteria.
* Test V5 transaction with Sapling spends
Create a fake V5 transaction that has Sapling spends and check that the
verifier accepts the transaction.
* Ignore rejected V5 transaction test for now
Because now it needs the `sighash` implementation for V5 to be ready.
* Reference V5 `sighash` PR in comment
So that it is easier to check if it's possible to remove the
`should_panic` or not.
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
* Remove `sapling shielded pool` TODO
V5 transactions now have Sapling shielded pool properly validated.
* Link to some extra issues in TODO comment
Some other issues are also necessary for full V5 validation.
* Add a TODO in the main code to fix the tests
Some tests are blocked due to missing features required for full V5
validation. Once those features are implemented, they should be updated
to remove the `#[should_panic]` attribute so that they actually run and
check the code correctly.
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
* add disabled sprout pool check
* change method name
* change error name
* fix typo
* make the success test case in other tx than the coinbase
* use new `height` method instead of deriving `PartialOrd` in `NetworkUpgrade`
* move check of network upgrade into function, rename, docs
* increase test coverage
* fix comment
* Refactor to create `verify_sapling_shielded_data`
Move the code to verify Sapling shielded data into a new helper method
that returns `AsyncChecks`.
* Test verifying a Sapling transaction with spends
Use the test vectors to find a transaction that has Sapling spends and
test if it the verifier considers it valid.
* Create a helper method to list test transactions
Transforms the block test vectors into a list of transactions and block
heights for each transaction.
* Use new helper function in V4 Sapling spend test
Also use the block height for that transaction as specified in the test
vector.
* Test V4 tx. with Sapling outputs but no spends
Find a transaction V4 vector that has Sapling outputs but no spends, and
check that the verifier accepts it.
* Add a `zebra_test::RUNTIME` shared runtime
Create a lazily instantiated Tokio runtime that can be shared by tests.
* Split tests that require a shared runtime
Split two tests that were previously in one because of the need to share
a single Tokio runtime. With the `zebra_test::RUNTIME`, they can now
share the runtime without having to be a single test.
* Stop trying to verify coinbase inputs using the script verifier
And create tests to catch similar bugs earier.
* Use Testnet in NU5 tests that temporarily should_panic
We've marked these tests as should_panic until there is a NU5 activation
height. But Testnet will have an activation height first, so we should
prefer it in the tests. (Or use both networks.)
* Refactor to create `verify_sprout_shielded_data`
Move the join split verification code into a new
`verify_sprout_shielded_data` helper method that returns an
`AsyncChecks` set.
* Test if signed V4 tx. join splits are accepted
Create a fake V4 transaction with a dummy join split, and sign it
appropriately. Check if the transaction verifier accepts the
transaction.
* Test if unsigned V4 tx. joinsplit data is rejected
Create a fake V4 transaction with a dummy join split. Do NOT sign this
transaction's join split data, and check that the verifier rejects the
transaction.
* Join tests to share Tokio runtime
Otherwise one of the tests might fail incorrectly because of a
limitation in the test environment. `Batch` services spawn a task in the
Tokio runtime, but separate tests can have separate runtimes, so sharing
a `Batch` service can lead to the worker task only being available for
one of the tests.
* Add missing documentation
Document methods to describe what they do and why.
* Create an `AsyncChecks` type alias
Make it simpler to write the `FuturesUnordered` type with boxed futures.
This will also end up being used more when refactoring to return the
checks so that the `call` method can wait on them.
* Create `verify_transparent_inputs_and_outputs`
Refactors the verification of the transparent inputs and outputs into a
separate method.
* Refactor transparent checks to use `call_all`
Instead of pushing the verifications into a stream of unordered futures,
use the `ServiceExt::call_all` method to build an equivalent stream
after building a stream of requests.
* Replace `CallAll` with `FuturesUnordered`
Make it more consistent with the rest of the code, and make sure that
the `len()` method is available to use for tracing.
Co-authored-by: teor <teor@riseup.net>
* Refactor to move wait for checks into a new method
Allow the code snipped to be reused by other transaction
version-specific check methods.
* Verify transparent inputs in V5 transactions
Use the script verifier to check the transparent inputs in a V5
transaction.
* Check `has_inputs_and_outputs` for all versions
Check if a transaction has inputs and outputs, independently of the
transaction version.
* Wait for checks in `call` method
Refactor to move the repeated code into the `call` method. Now the
validation methods return the set of asynchronous checks to wait for.
* Add helper function to mock transparent transfers
Creates a fake source UTXO, and then the input and output that represent
spending that UTXO. The initial UTXO can be configured to have a script
that either accepts or rejects any spend attempt.
* Test if transparent V4 transaction is accepted
Create a fake V4 transaction that includes a fake transparent transfer
of funds. The transfer uses a script to allow any UTXO to spend it.
* Test transaction V4 rejection based on script
Create a fake transparent transfer where the source UTXO has a script
that rejects spending. The script verifier should not accept this
transaction.
* Test if transparent V5 transaction is accepted
Create a mock V5 transaction that includes a transparent transfer of
funds. The transaction should be accepted by the verifier.
* Test transaction V5 rejection based on script
Create a fake transparent transfer where the source UTXO has a script
that rejects spending. The script verifier should not accept this
transaction.
* Update `Request::upgrade` getter documentation
Simplify it so that it won't become updated when #1683 is fixed.
Co-authored-by: teor <teor@riseup.net>
* Add a `Transaction::version` getter
Returns the version of the transaction as a `u32`.
* Add `Transaction::is_overwintered` helper method
Returns if the `fOverwintered` flag should be set for the transaction's
version.
* Use new helpers to serialize transaction version
Reduce the repeated code and make it less error-prone with future
changes.
* Add getter methods to `transaction::Request` type
Refactor to move the type deconstruction code into the `Request` type.
The main objective is to make it easier to split the call handler into
methods that receive the request directly.
* Refactor to create `verify_v4_transaction` helper
Split the code specific to V4 transactions into a separate helper
method.
* Create `verify_v5_transaction` helper method
Prepare a separate method to have the validation code.
* Add `UnsupportedByNetworkUpgrade` error variant
An error for when a transaction's version isn't supported by the network
upgrade of the block it's included or for the current network upgrade if
the transaction is for the mempool.
* Verify a V5 transaction's network upgrade
For now, only NU5 supports V5 transactions.
* Test that V5 transaction is rejected on Canopy
Create a fake V5 transaction and try to verify it using a block height
from Canopy's activation. The verifier should reject the transaction
with an error saying that the network upgrade does not support that
transaction version.
* Test if V5 tx. is accepted after NU5 activation
Create a fake V5 transaction and pretend it is placed in a block that
has a height after the NU5 activation. The test should succeed, but
since the NU5 activation height has not been specified yet (neither for
the testnet nor the mainnet), for now this test is marked as
`should_panic`.
* Add `TODO` comment to the code
Add more detail to what's left to do, and link to the appropriate PRs.
* Use `u32` to store transaction version
Use a type consistent with how the version is specified.
Co-authored-by: teor <teor@riseup.net>
Co-authored-by: teor <teor@riseup.net>
* Add a `at_least_one!` macro for testing
Similar to the `vec!` macro, but doesn't allow creating an empty list.
* Test if `has_inputs_and_outputs` considers actions
Create a dummy transaction with no inputs and no outputs, and add a
dummy Orchard action to it. The `check::has_inputs_and_outputs`
should succeed, because the consensus rule considers having Orchard
actions as having inputs and/or outputs.
* Refactor to create helper function
Move the code to create a fake Orchard shielded data instance to a
helper function in `zebra_chain::transaction::arbitrary`, so that other
tests can also use it.
* Test coinbase V5 transaction with enable spends
A V5 coinbase transaction that has Orchard shielded data MUST NOT have
the enable spends flag set.
* Test if coinbase without enable spends is valid
A coinbase transaction with Orchard shielded data and without the enable
spends flag set should be valid.
* Add a security comment about the `at_least_one!` macro
This macro must not be used outside tests, because it allows memory denial
of service.
Co-authored-by: teor <teor@riseup.net>
* Implement `PartialEq<i64>` for `Amount`
Allows to compare an `Amount` instance directly to an integer.
* Add `SerializationError::BadTransactionBalance`
Error variant representing deserialization of a transaction that doesn't
conform to the Sapling consensus rule where the balance MUST be zero if
there aren't any shielded spends and outputs.
* Validate consensus rule when deserializing
Return an error if the deserialized V4 transaction has a non-zero value
balance but doesn't have any Sapling shielded spends nor outputs.
* Add consensus rule link to field documentation
Describe how the consensus rule is validated structurally by
`ShieldedData`.
* Clarify that `value_balance` is zero
Make the description more concise and objective.
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
* Update field documentation
Include information about how the consensus rule is guaranteed during
serialization.
Co-authored-by: teor <teor@riseup.net>
* Remove `check::sapling_balances_match` function
The check is redundant because the respective consensus rule is
validated structurally by `ShieldedData`.
* Test deserialization of invalid V4 transaction
A transaction with no Sapling shielded spends and no outputs but with a
non-zero balance value should fail to deserialize.
* Change least-significant byte of the value balance
State how the byte index is calculated, and change the least
significant-byte to be non-zero.
Co-authored-by: teor <teor@riseup.net>
* Add a `Transaction::orchard_shielded_data` getter
Allows accessing the Orchard shielded data if it is present in the
transaction, regardless of the transaction version.
* Refactor `orchard_nullifiers` to use new getter
Allows making the method more concise.
* Add `CoinbaseHasEnableSpendsOrchard` error variant
Used when the validation rule is not met.
* Implement `enableSpendsOrchard` in coinbase check
The flag must not be set for the coinbase transaction.
* Refactor `Transaction::orchard_*` getters
Use the fact that `Option<T>` implements `Iterator<T>` to simplify the
code and remove the need for boxing the iterators.
Co-authored-by: teor <teor@riseup.net>
* Fix documentation comment
Was missing a slash to become documentation.
* Add documentation link to type reference
Just to help navigation a bit.
* Implement `Transaction::orchard_actions()` getter
Returns an iterator to iterator over the actions in the Orchard shielded
data (if there is one, otherwise it returns an empty iterator).
* Add V5 support for `has_inputs_and_outputs`
Checks if the transaction has Orchard actions. If it does, it is
considered to have inputs and outputs.
* Refactor transaction test vectors
Make it easier to reuse the fake V5 transaction converter in other test
vectors.
* Move helper function to `zebra-chain` crate
Place it together with some other helper functions, including the one
that actually creates the fake V5 transaction.
* Test transaction with no inputs
`check::has_inputs_and_outputs` should return an error indicating that
the transaction has no inputs.
* Test transaction with no outputs
`check::has_inputs_and_outputs` should return an error indicating that
the transaction has no outputs.
* Note that transaction is fake in `expect` message
Should make the message easier to find, and also gives emphasis to the
fact that the transaction is a fake conversion to V5.
Co-authored-by: teor <teor@riseup.net>
Co-authored-by: teor <teor@riseup.net>
* validate sapling v5 tx
* Make itertools dependency optional
We only need itertools when the `proptest-impl` feature is enabled.
* Check if V4 and V5 coinbase transactions contain PrevOut transparent inputs
This is a bugfix on V4 transaction validation. The PrevOut consensus
rule was not explicitly stated in the Zcash spec until April 2021.
(But it was implied by Bitcoin, and partially implemented by Zebra.)
Also do the shielded sapling input check for V5 transactions.
* Add spec and orchard TODOs to has_inputs_and_outputs
Also make the variable names match the spec.
* Sort transaction functions to match v5 data order
* Simplify transaction input and output checks
Move counts or iterators into `Transaction` methods, so we can remove
duplicate code, and make the consensus rule logic clearer.
* Update sapling_balances_match for Transaction v5
- Quote from the spec
- Explain why the function is redunant for v5
- Rename the function so it's clear that it is sapling-specific
Co-authored-by: teor <teor@riseup.net>
* start refactoring transaction v4 for transaction v5
- move ShieldedData to sapling
- add AnchorVariant
- rename shielded_data to sapling_shielded data in V4
- move value_balance into ShieldedData
- update prop tests for new structure
* add AnchorVariant to Spend
- make anchor types available from sapling crate
- update serialize
* change shielded_balances_match() arguments
* change variable name anchor to shared_anchor in ShieldedData
* fix empty value balance serialization
* use AnchorV in shielded spends
* Rename anchor to per_spend_anchor
* Use nullifiers function directly in non-finalized state
* Use self.value_balance instead of passing it as an argument
* Add missing fields to ShieldedData PartialEq
* Derive Copy for tag types
* Add doc comments for ShieldedData refactor
* Implement a per-spend anchor compatibility iterator
Co-authored-by: teor <teor@riseup.net>
* Ed25519 async batch verification for JoinSplit signatures
We've been verifying JoinSplitSigs one-by-one pre-ZIP-215. Now as we're post-ZIP-215,
we can take advantage of the batch math to validate this signatures.
I would have pumped all the joinsplits in our MAINNET_BLOCKS test vectors but these
signatures are over the sighash, which needs the NU code to compute, and once we're
doing all that set up, we're basically doing transaction validation, so.
Resolves#1944
* Repoint to latest ed25519-zebra commit with note to point at 3.0 when released
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
Co-authored-by: teor <teor@riseup.net>
* add transaction V5 stub
* add v5_strategy
* deduplicate version group ids
* Update comment for V5 transactions
* Add V5 transactions to non_finalized_state
Currently these are all `unimplemented!(...)`
* Fix struct matches
* Apply trivial panic message changes
* add zcash_deserialize for V5
* make all tx versions explicit in sprout and sapling nullifier functions
* match exhaustively in sprout and sapling nullifier functions
* fix matches in zebra-consensus
* fix NU5 strategy
* We're still deciding if v5 transactions support Sprout
Co-authored-by: teor <teor@riseup.net>
This consensus rule is supposed to apply to transactions whose
transparent inputs are the *outputs* of previous coinbase
transactions, not to transactions with coinbase inputs. Because that
logic is different enough from this logic, and requires different data
flow, it's cleaner to just remove this check for now.
Making this check's match statement exhaustive revealed a bug similar to
the previous commit. The logic in the spec is written in terms of
numbers, but our data is internally represented in terms of enums
(ADTs). This kind of cross-representation rule translation is a bug
surface, which we can avoid by converting to counts and summing up. (We
should use one style at a time).
This function caused spurious "WrongVersion" errors, because the match
pattern in the first arm was non-exhaustive, but the fallthrough match
arm was present and assumed it would only be reached if the version was
incorrect.
This commit cleans up the implemenation, splits out the error variants,
and renames the check to be more precise.
To avoid this kind of bug in the future, two guidelines are useful:
1. Avoid fallthrough cases that circumvent non-exhaustive match checks;
2. Avoid nested conditionals, preferring a "straight-line" sequence of
match arm => result pairs rather than nested matches or matches with
conditionals inside.
teor
Alfredo Garcia
Conrado Gouvea
Janito Vaqueiro Ferreira Filho
Deirdre Connolly
Deirdre Connolly
Henry de Valence