Commit Graph

41 Commits

Author SHA1 Message Date
Janito Vaqueiro Ferreira Filho 0e89236405
Reject V5 transactions before NU5 activation (#2285)
* 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>
2021-06-15 10:15:59 +10:00
Janito Vaqueiro Ferreira Filho b44d81669f
Move the check in `transaction::check::sapling_balances_match` to `V4` deserialization (#2234)
* 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>
2021-06-04 08:53:00 +10:00
Alfredo Garcia 75d29aca24
Add V5 transparent and sapling to transaction::check, add missing coinbase PrevOut check (#2070)
* 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>
2021-04-28 10:43:00 +10:00
teor 247620320e
Rename shielded_data to sapling_shielded_data as needed (#2072)
This change help avoid bugs that confuse sapling with orchard (or sprout).

```sh
fastmod shielded_data sapling_shielded_data
```
2021-04-27 10:37:53 -03:00
Alfredo Garcia 48a8a7b851
Refactor Sapling data and use it in V4 (#1946)
* 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>
2021-04-01 07:34:25 +10:00
Deirdre Connolly 0ffab6d589
Ed25519 async batch verification for JoinSplit signatures (#1952)
* 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>
2021-03-31 09:08:19 +10:00
teor 16c073c4e0 Comment: transparent sighash in script verifier 2021-03-25 16:34:16 -04:00
teor 83bdc1a9f3 Add a metric for binding sig errors
But keep ignoring those errors until we fix binding sigs in #1939
2021-03-25 16:34:16 -04:00
teor e4f58f018a Re-enable joinsplit and sapling spend auth verification 2021-03-25 16:34:16 -04:00
teor f8dc989e99 Move a quick check earlier in transaction validation 2021-03-25 16:34:16 -04:00
Deirdre Connolly 7efc700aca
Merge pull request #1713 from ZcashFoundation/use-groth16-batch-math
Use batch optimizations, load params in groth16::Verifier, verify Spend & Output descriptions in transaction verifier
2021-03-24 12:28:25 -04:00
teor 38585b6116
Fix whitespace for rustfmt 2021-03-22 11:18:37 +10:00
teor a6f916671a
Add a TODO comment for a missing no-coinbase check 2021-03-22 08:59:51 +10:00
Alfredo Garcia 8883543a85
Add transaction version 5 stubs (#1824)
* 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>
2021-03-04 07:56:41 +10:00
teor 132c7fe4a5 Add a comment about a mempool request bug
Transaction verifier mempool requests should take the next block height,
because some consensus rules depend on the exact height.

See #1683.
2021-02-05 16:40:34 -05:00
Jane Lusby 0ac259430a Implement Async Batch verification API for groth16
This PR is the first step in getting a groth16 proving system fully
integrated with the rest of zebra. This PR implements the initial async
API, but none of the actual batching logic necessary for our eventual
verifier design.

Once the batch verification API from bellman has been implemented we
will need to swap out the "Batch" type defined in this crate with the
new `batch::Verifier` defined in bellman.
2021-02-05 14:52:48 -05:00
Jane Lusby cfc339e8ea
export new precompute api in zebra-script (#1493)
* export new precompute api in zebra-script
* remove old API in favor of precompute API
* add multi use test cases and bump version
* update implementation to actually match henry's design
* Add safety comment for zebra-script

Co-authored-by: teor <teor@riseup.net>
2020-12-18 13:18:28 +10:00
Henry de Valence b9347e01e4 consensus: ignore sighash-derived checks until #1377 2020-11-24 16:28:22 -08:00
Henry de Valence 61b3286085 consensus: add NU plumbing to block,tx,script verifiers
Closes #1367 by propagating the network upgrade through the service
requests.
2020-11-25 08:53:07 +10:00
Henry de Valence 342eb166ff state: track UTXO provenance
This commit changes the state system and database format to track the
provenance of UTXOs, in addition to the outputs themselves.
Specifically, it tracks the following additional metadata:

- the height at which the UTXO was created;
- whether or not the UTXO was created from a coinbase transaction or
  not.

This metadata will allow us to:

- check the coinbase maturity consensus rule;
- check the coinbase inputs => no transparent outputs rule;
- implement lookup of transactions by utxo (using the height to find the
  block and then scanning the block) for a future RPC mechanism.

Closes #1342
2020-11-23 22:18:43 -08:00
Henry de Valence e4f38582fd consensus: add remaining checks trace counter to tx verifier 2020-11-23 14:16:39 +10:00
Henry de Valence a2a70e1fc1 consensus: fix same-block utxo lookups
The UTXO query system assumes that a transaction will only request
information about UTXOs created in prior blocks.  But transactions are
allowed to spend UTXOs created by prior transactions in the same block.

This doesn't fit with the existing query model, so instead of trying to
change it, allow the script verifier to take an additional set of known
UTXOs, and propagate this set from the block.
2020-11-23 14:16:39 +10:00
Henry de Valence f0810b028d state,consensus,sync: shorten span lengths
These changes help reduce the size of the resulting spans, making the
output more compact.  Together they save about 30-40 characters.
2020-11-23 14:16:39 +10:00
teor 196dc6369c Delete outdated transaction comments 2020-11-22 23:11:00 -05:00
Henry de Valence 2eceff421f consensus: remove incorrect check
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.
2020-11-21 14:09:15 -05:00
Henry de Valence ace1103462 consensus: fix bug in tx input/output presence check
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).
2020-11-21 14:09:15 -05:00
Henry de Valence 96ee32e5d2 consensus: fix bug in coinbase joinsplit/spend check
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.
2020-11-21 14:09:15 -05:00
Henry de Valence b116cfcd76 consensus: add debug event on wrong version check
Adding this check reveals that the WrongVersion errors aren't coming
from the correct WrongVersion check.
2020-11-21 14:09:15 -05:00
Henry de Valence d1ee7f263a consensus: add debug span to TransactionVerifier 2020-11-21 14:09:15 -05:00
Deirdre Connolly 8cac287aa2 Tidy TransactionError 2020-10-28 21:44:13 -04:00
Deirdre Connolly b2df84fc59 Dedupe VerifyTransactionError into TransactionError 2020-10-28 21:44:13 -04:00
Deirdre Connolly 1d646e6a27 Make Clippy happy 2020-10-28 21:44:13 -04:00
Deirdre Connolly 1ce2eea35f Add coinbase shielded descriptions check 2020-10-28 21:44:13 -04:00
Deirdre Connolly 1653aca570 Add shielded_balances_match check 2020-10-28 21:44:13 -04:00
Deirdre Connolly 612148fbdd consensus: add transaction::check module 2020-10-28 21:44:13 -04:00
Alfredo Garcia bcb027ebc5 change canopy.pdf to stable protocol.pdf 2020-10-28 11:34:53 -04:00
Deirdre Connolly e796132057 Allow dead_code for the transaction::Request for now (mempool) 2020-10-20 11:16:22 -04:00
Henry de Valence d4ce3eb054 consensus: improve docs
- remove no longer accurate documentation about transaction verifier;
- add description of the role of the crate.
2020-10-20 11:16:22 -04:00
Jane Lusby 4a36c4485a consensus: integrate script, transaction Verifiers 2020-10-20 11:16:22 -04:00
Henry de Valence 248c318906 consensus: sketch of transaction::Verifier
This squashes the previous sequence of commits to let us separate out
the structural skeleton (which unblocks other work and is not
consensus-critical) from the actual checks (which don't block other work
and are consensus-critical).

Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
2020-10-20 11:16:22 -04:00
teor 8b5ec155f0
Consensus refactor (#629)
* Flatten consensus::verify::* to consensus::*
* Move consensus::*::tests into their own files
* Move CheckpointList into its own file
* Move Progress and Target into a types module

QueuedBlock and QueuedBlockList can stay in checkpoint.rs, because
they are tightly coupled to CheckpointVerifier.
2020-07-10 16:51:01 +10:00