* Reorder imports to follow convention
Place the imports from `std` at the top.
* Add transaction errors for double spends
Add a variant for each pool. They represent a double spend inside a
transaction.
* Add `check::spend_conflicts` implementation
Checks if a transaction has spend conflicts, i.e., if a transaction
spends a UTXO more than once or if it reveals a nullifier more than
once.
* Reject transactions with internal spend conflicts
The transaction verifier should reject transactions that spend the same
transparent UTXO or that reveal the same nullifier.
* Add transparent spend consensus rule
Add it to the documentation to help with understanding and auditing it.
Co-authored-by: teor <teor@riseup.net>
* Use different nullifiers by default
Don't use the same nullifier twice when mocking a
`sprout::JoinSplitData` because it will lead to an invalid transaction.
* Test transactions with repeated spend outpoints
Since that represents a spend conflict, they should be rejected.
* Test duplicate nullifiers in joinsplit
Check if a mock transaction with a joinsplit that reveals the same
nullifier twice is rejected.
* Test duplicate nullifiers across joinsplits
Check if a duplicate nullifier in two different joinsplits in the same
transaction is rejected.
* Test V4 transaction with duplicate Sapling spend
Check if a V4 transaction that has a duplicate Sapling spend is
rejected.
* Test V5 transaction with duplicate Sapling spend
Check if a V5 transaction that has a duplicate Sapling spend is
rejected.
* Test V5 transaction with duplicate Orchard actions
Check if a V5 transaction that has duplicate Orchard actions is rejected
by the transaction verifier.
Co-authored-by: teor <teor@riseup.net>
* Make Amount arithmetic more generic
To modify generated amounts, we need some extra operations on `Amount`.
We also need to extend existing operations to both `NonNegative` and
`NegativeAllowed` amounts.
* Add a constrain method for ValueBalance
* Derive Eq for ValueBalance
* impl Neg for ValueBalance
* Make some Amount arithmetic expectations explicit
* Explain why we use i128 for multiplication
And expand the overflow error details.
* Expand Amount::sum error details
* Make amount::Error field order consistent
* Rename an amount::Error variant to Constraint, so it's clearer
* Add specific pool variants to ValueBalanceError
* Update coinbase remaining value consensus rule comment
This consensus rule was updated recently to include coinbase transactions,
but Zebra doesn't check block subsidy or miner fees yet.
* Add test methods for modifying transparent values and shielded value balances
* Temporarily set values and value balances to zero in proptests
In both generated chains and proptests that construct their own transactions.
Using zero values reduces value calculation and value check test coverage.
A future change will use non-zero values, and fix them so the check passes.
* Add extra fields to remaining transaction value errors
* Swap the transparent value balance sign to match shielded value balances
This makes the signs of all the chain value pools consistent.
* Use a NonNegative constraint for transparent values
This fix:
* makes the type signature match the consensus rules
* avoids having to write code to handle negative values
* Allocate total generated transaction input value to outputs
If there isn't enough input value for an output, set it to zero.
Temporarily reduce all generated values to avoid overflow.
(We'll remove this workaround when we calculate chain value balances.)
* Consistently use ValueBalanceError for ValueBalances
* Make the value balance signs match the spec
And rename and document methods so their signs are clearer.
* Convert amount::Errors to specific pool ValueBalanceErrors
* Move some error changes to the next PR
* Add extra info to remaining transaction value errors (#2585)
* Distinguish between overflow and negative remaining transaction value errors
And make some error types cloneable.
* Add methods for updating chain value pools (#2586)
* Move amount::test to amount::tests:vectors
* Make ValueBalance traits more consistent with Amount
- implement Add and Sub variants with Result and Assign
- derive Hash
* Clarify some comments and expects
* Create ValueBalance update methods for blocks and transactions
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
* 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 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
* 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.
Janito Vaqueiro Ferreira Filho
Conrado Gouvea
teor
Alfredo Garcia
Deirdre Connolly
Deirdre Connolly
Henry de Valence