* 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>
Zebra's latest alpha checkpoints on Canopy activation, continues our work on NU5, and fixes a security issue.
Some notable changes include:
## Added
- Log address book metrics when PeerSet or CandidateSet don't have many peers (#1906)
- Document test coverage workflow (#1919)
- Add a final job to CI, so we can easily require all the CI jobs to pass (#1927)
## Changed
- Zebra has moved its mandatory checkpoint from Sapling to Canopy (#1898, #1926)
- This is a breaking change for users that depend on the exact height of the mandatory checkpoint.
## Fixed
- tower-batch: wake waiting workers on close to avoid hangs (#1908)
- Assert that pre-Canopy blocks use checkpointing (#1909)
- Fix CI disk space usage by disabling incremental compilation in coverage builds (#1923)
## Security
- Stop relying on unchecked length fields when preallocating vectors (#1925)
* 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>
* change `poll_ready()` and `call()` of `ChainVerifier`
* add bound, move max_checkpoint_height
* add buffers to the checkpoint and block verifiers
And rename the chain verifier fields so `block` means `Arc<Block>`, and
`block_verifier` means `Buffer<BlockVerifier, ...>`.
* Fix the error types
* Use `ServiceExt::oneshot` in `ChainVerifier::call`
And:
* make the code look like the `main` branch as much as possible
* document the `poll_ready`/`call` invariant
* Use `ServiceExt::oneshot` in `chain::init`
Co-authored-by: teor <teor@riseup.net>
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.
* Bump versions where appropriate
Tested with cargo install --locked --path etc
* Remove fixed panics from 'Known Issues'
* Change to alpha release series in the README
Co-authored-by: teor <teor@riseup.net>
The clippy unknown lints attribute was deprecated in
nightly in rust-lang/rust#80524. The old lint name now produces a
warning.
Since we're using `allow(unknown_lints)` to suppress warnings, we need to
add the canonical name, so we can continue to build without warnings on
nightly.
But we also need to keep the old name, so we can continue to build
without warnings on stable.
And therefore, we also need to disable the "removed lints" warning,
otherwise we'll get warnings about the old name on nightly.
We'll need to keep this transitional clippy config until rustc 1.51 is
stable.
## Motivation
This PR is motivated by the regression identified in https://github.com/ZcashFoundation/zebra/issues/1349. That PR notes that the metrics stopped working for most of the crates other than `zebrad`.
## Solution
This PR resolves the regression by deduplicating the `metrics` crate dependency. During a recent change we upgraded the metrics version in `zebrad` and a couple other of our crates, but we never updated the dependencies in `zebra-state`, `zebra-consensus`, or `zebra-network`. This caused the metrics macros to attempt to retrieve the current metrics exporter through the wrong function. We would install the metrics exporter in `0.13`, but then attempt to look it up through the `0.12` crate, which contains a different instance of the metrics exporter static variable which is unset. Doing this causes the metrics macros to return `None` for the current exporter after which they just silently give up.
## Related Issues
closes https://github.com/ZcashFoundation/zebra/issues/1349
## Follow Up Work
I noticed we have quite a few duplicate dependencies in our tree. We might be able to save some compilation time by auditing those and deduplicating them as much as possible.
- https://github.com/ZcashFoundation/zebra/issues/1582
Co-authored-by: teor <teor@riseup.net>
* 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>
* create readme for utils and checkpoints
* add link to checkpoint usage to book
Co-authored-by: Deirdre Connolly <durumcrustulum@gmail.com>
Co-authored-by: teor <teor@riseup.net>
Previously we set the crate versions to 3.x, so that the major version was
aligned with the NU version. But we want to be able to make API changes
independently of the NU schedule.
Change the Merkle root validation logic to also check that a block does not
contain duplicate transactions. This check is redundant with later
double-spend checks, but is a useful defense-in-depth.
As a side effect of computing Merkle roots, we build a list of
transaction hashes. Instead of discarding these, add them to
PreparedBlock and FinalizedBlock so that they can be reused rather than
recomputed.
This commit adds Merkle root validation to:
1. the block verifier;
2. the checkpoint verifier.
In the first case, Bitcoin Merkle tree malleability has no effect,
because only a single Merkle tree in each malleablity set is valid (the
others have duplicate transactions).
In the second case, we need to check that the Merkle tree does not contain any
duplicate transactions.
Closes#1385Closes#906
UTXO requests during transaction input verification can time out because:
1. The block that creates the UTXO is queued for download or verify, but
it hasn't been committed yet. The creating block might spend UTXOs
that come from other recent blocks, so UTXO verification can depend on
a (non-contiguous) sequence of block verifications.
In this case, Zebra should wait for additional block download and
verify tasks to complete.
2. The block that creates the UTXO isn't queued for download. This can
happen because the block is gossiped block that's much higher than the
current tip, or because a peer sent the syncer a bad list of block
hashes.
In this case, Zebra should discard the timed out block, and restart
the sync.
We need to choose a timeout that balances these two cases, so we time
out after 180 seconds.
Assuming Zebra can download at least 1 MB per second, 180 seconds is
enough time to download a few hundred blocks. So Zebra should be able to
download and verify the next block before the UTXOs that it creates time
out. (Since Zebra has already verified all the blocks before the next
block, its UTXO requests should return immediately.)
Even if some peers time out downloads, a block can only be pending
download for 80 seconds (4 retries * 20 second timeout) before the
download fails. So the UTXO timeout doesn't need to be much larger than
this overall download timeout - because the download timeout will happen
first on slow networks.
Alternately, if the download for the creating block was never queued,
Zebra should timeout as soon as possible - so it can restart the sync
and download the creating block.
As a side-effect, a lower UTXO timeout also makes it slightly easier to
debug UTXO issues, because unsatisfiable queries fail faster.
The state service API says explicitly that AwaitUTXO requests should be coupled
with a timeout layer. I didn't add this when I was testing and fixing the UTXO
lookup code (#1348, #1358) because causing zebrad to hang on a failed
dependency was useful for identifying cases where the code wasn't useful (and
then inspecting execution traces).
As a side effect, this ticket resolves most of the hangs in #1389, because
far-future gossiped blocks will have their UTXO lookups time out, though we
may wish to do other work as part of debugging the combined sync+gossip logic.
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
This change introduces two new types:
- `PreparedBlock`, representing a block which has undergone semantic
validation and has been prepared for contextual validation;
- `FinalizedBlock`, representing a block which is ready to be finalized
immediately;
and changes the `Request::CommitBlock`,`Request::CommitFinalizedBlock`
variants to use these types instead of their previous fields.
This change solves the problem of passing data between semantic
validation and contextual validation, and cleans up the state code by
allowing it to pass around a bundle of data. Previously, the state code
just passed around an `Arc<Block>`, which forced it to needlessly
recompute block hashes and other data, and was incompatible with the
already-known but not-yet-implemented data transfer requirements, namely
passing in the Sprout and Sapling anchors computed during contextual
validation.
This commit propagates the `PreparedBlock` and `FinalizedBlock` types
through the state code but only uses their data opportunistically, e.g.,
changing .hash() computations to use the precomputed hash. In the
future, these structures can be extended to pass data through the
verification pipeline for reuse as appropriate. For instance, these
changes allow the sprout and sapling anchors to be propagated through
the state.
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.
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.
The BlockVerifier constructed a tracing span and manually entered it
inside of an async block. Manually entering spans inside async blocks
can cause problems where the span might not be entered and exited
correctly as the resulting future is polled. Instead, using the
.instrument creates a wrapper future that handles the bookkeeping.
I changed the span name and contents to be consistent with the spans in
the checkpoint verifier.
This change is mostly mechanical, with the exception of the changes to the
`tower-batch` middleware. This middleware was adapted from `tower::buffer`,
and the `tower::buffer` code was changed to implement its own bounded queue,
because Tokio 0.3 removed the `mpsc::Sender::poll_send` method. See
ddc64e8d4d
for more context on the Tower changes. To match Tower as closely as possible
in order to be able to upstream `tower-batch`, those changes are copied from
`tower::Buffer` to `tower-batch`.
This change explicitly documents cancellation contracts for our Tower services,
and tries to correct a bug in the implementation of the CheckpointVerifier,
which duplicates information from the state service but did not ensure that it
would be kept in sync.
* Implement Expanded to Compact Difficulty
* Implement Arbitrary for CompactDifficulty
Remove the derive, and generate values from random block
hashes.
* Implement Arbitrary for ExpandedDifficulty and Work
* Use Arbitrary for CompactDifficulty in Arbitrary for Block
* Test difficulty on all block test vectors
And cleanup some duplicate test code
* Round-trip tests for compact test cases
* Round-trip tests for compact difficulty in block test vectors
* Make Add for Work return PartialCumulativeWork
Remove AddAssign for Work
Rewrite a proptest using Sub for PartialCumulativeWork
Use Arbitrary for Work
* Add roundtrip work sum tests
* Add roundtrip comparison difficulty tests
* Add failing proptest cases due to test bugs
* Use Some(_) rather than _.into()
* Reduce visibility of difficulty type inner values
* Split work and other difficulty proptests
This change makes sure that rejected work values don't disable property
tests on other types.
There's no reason to return a pre-Buffer'd service (there's no need for
internal access to the state service, as in zebra-network), but wrapping
it internally removes control of the buffer size from the caller.
The previous debug output printed a message that the chain verifier had
recieved a block. But this provides no additional information compared
to printing no message in chain::Verifier and a message in whichever
verifier the block was sent to, since the resulting spans indicate where
the block was dispatched.
This commit also removes the "unexpected high block" detection; this was
an artefact of the original sync algorithm failing to handle block
advertisements, but we don't have that problem any more, so we can
simplify the code by eliminating that logic.
* Reverse displayed endianness of transaction and block hashes
* fix zebra-checkpoints utility for new hash order
* Stop using "zebrad revhex" in zebrad-hash-lookup
* Rebuild checkpoint lists in new hash order
This change also adds additional checkpoints to the end of each list.
* Replace TransactionHash with transaction::Hash
This change should have been made in #905, but we missed Debug impls
and some docs.
Co-authored-by: Ramana Venkata <vramana@users.noreply.github.com>
Co-authored-by: teor <teor@riseup.net>
This reduces the API surface to the minimum required for functionality,
and cleans up module documentation. The stub mempool module is deleted
entirely, since it will need to be redone later anyways.
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>
This commit begins the process of integrating `zcash_script` with the rest of the system for verifying scripts while syncing the block chain. It does so by adding the necessary support for looking up UTXOs from the state service and implements the first parts of the `script::Verifier` for looking up the necessary UTXOs in the state and then generating the necessary call to `zcash_script` to verify the script itself.
Co-authored-by: teor <teor@riseup.net>
To match the other check::* functions.
Automated commit, created using the script:
sed -i 's/subsidy_is_correct/subsidy_is_valid/' \
$(grep -r subsidy_is_correct zebra* | cut -d: -f1 | sort -u)
* add general and founders reward subsidy modules
* validate founders reward
* Use funding streams after Canopy on testnet
ZIP-1014 only applies to mainnet, where Canopy is at the first halving.
On testnet, Canopy is before the first halving, and the dev fund rules
apply from Canopy. (See ZIP-214.)
Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Jane Lusby <jlusby42@gmail.com>
* pass all test vectors through current subsidy validation
* Add testnet and halving subsidy tests
* add subsidy validation error tests
* rename block validation methods
* add network to block verifier
* add amount operators
* Implement Ord, Eq, and Hash for Amount
* Implement Add<Height> for Height
And make the existing Height operators do range checks.
* Apply operator suggestions
Co-authored-by: Jane Lusby <jlusby42@gmail.com>
* Send the final checkpoint block to the checkpoint verifier
Also:
* route blocks with no height to the block verifier
* update an incorrect comment
* Add missing {
* rustfmt
This addresses at least three pain points:
- we were affected by bugs that were already fixed in git, but not in
the released crate;
- we can use service combinators to transform requests and responses;
- we can use the hedge middleware.
The version in git is still marked as 0.3.1 but these changes will be
part of tower 0.4: https://github.com/tower-rs/tower/issues/431
* refactor block and tx validation errors
* rename errors module to error
* move NoTransactions to BlockError
* clarify some errors, use dbg format for hash in error
* mnake is_coinbase_first return BlockError
* add new error types for each consensus Service
Co-authored-by: Jane Lusby <jane@zfnd.org>
Using a Buffer with size 1 is a footgun because it allows only one
sender to call poll_ready at a time. This is usually undesirable
because it means that a task or service that calls poll_ready but only
makes a service call later (potentially much later) will block all other
callers.
This makes the component verifiers both always return `poll_ready`,
because they do not exert backpressure and cannot fail.
The checkpoint verifier now immediately rejects any blocks that arrive
after it finishes checkpointing, instead of marking the service itself
as failed.
The chain verifier is agnostic to the readiness behavior of its
components, and reports readiness when they are both ready.
This is a really nice function but there might be a bug in its future
implementation: https://github.com/tower-rs/tower/issues/469
This bug may have already been fixed for the 0.4.0 release, so we could change
back then.
This test aimed to exercise both the checkpoint and block verifiers by
making a checkpoint list of length 1. However, the block verifier can't
work on any blocks below Sapling activation.
Instead of conditionally parsing the hardcoded checkpoint list and
optionally making a CheckpointVerifier, make one unconditionally, and
use the config settings to decide whether to route responses to it.
Then, fix up all of the places needed to make it compile and remove all
of the dead code.
This disables one test that can't be easily fixed at the moment, because
it tests the wrong thing: the checkpoint and block verifiers will
produce different transcripts.
It also disables the initial_tip logic for now, pending simplification
of the ChainVerifier logic.
* stop committing to the state in the ChainVerifier
* commit to the state in the BlockVerifier
* commit to the state in the CheckpointVerifier
Co-authored-by: Jane Lusby <jlusby42@gmail.com>
* Remove in-memory state service
* make the config compatible with toml again
* checkpoint commit to see how much I still have to revert
* back to the starting point...
* remove unused dependency
* reorganize error handling a bit
* need to make a new color-eyre release now
* reorder again because I have problems
* remove unnecessary helpers
* revert changes to config loading
* add back missing space
* Switch to released color-eyre version
* add back missing newline again...
* improve error message on unix when terminated by signal
* add context to last few asserts in acceptance tests
* instrument some of the helpers
* remove accidental extra space
* try to make this compile on windows
* reorg platform specific code
* hide on_disk module and fix broken link
* add CheckpointList::new_up_to(limit: NetworkUpgrade)
* if checkpoint_sync is false, limit checkpoints to Sapling
* update tests for CheckpointList and chain::init
This is the first in a sequence of changes that change the block:: items
to not include Block as a prefix in their name, in accordance with the
Rust API guidelines.
* checkpoint: reject older of duplicate verification requests.
If we get a duplicate block verification request, we should drop the older one
in favor of the newer one, because the older request is likely to have been
canceled. Previously, this code would accept up to four duplicate verification
requests, then fail all subsequent ones.
* sync: add a timeout layer to block requests.
Note that if this timeout is too short, we'll bring down the peer set in a
retry storm.
* sync: restart syncing on error
Restart the syncing process when an error occurs, rather than ignoring it.
Restarting means we discard all tips and start over with a new block locator,
so we can have another chance to "unstuck" ourselves.
* sync: additional debug info
* sync: handle lookahead limit correctly.
Instead of extracting all the completed task results, the previous code pulled
results out until there were fewer tasks than the lookahead limit, then
stopped. This meant that completed tasks could be left until the limit was
exceeded again. Instead, extract all completed results, and use the number of
pending tasks to decide whether to extend the tip or wait for blocks to finish.
* network: add debug instrumentation to retry policy
* sync: instrument the spawned task
* sync: streamline ObtainTips/ExtendTips logic & tracing
This change does three things:
1. It aligns the implementation of ObtainTips and ExtendTips so that they use
the same deduplication method. This means that when debugging we only have one
deduplication algorithm to focus on.
2. It streamlines the tracing output to not include information already
included in spans. Both obtain_tips and extend_tips have their own spans
attached to the events, so it's not necessary to add Scope: prefixes in
messages.
3. It changes the messages to be focused on reporting the actual
events rather than the interpretation of the events (e.g., "got genesis hash in
response" rather than "peer could not extend tip"). The motivation for this
change is that when debugging, the interpretation of events is already known to
be incorrect, in the sense that the mental model of the code (no bug) does not
match its behavior (has bug), so presenting minimally-interpreted events forces
interpretation relative to the actual code.
* sync: hack to work around zcashd behavior
* sync: localize debug statement in extend_tips
* sync: change algorithm to define tips as pairs of hashes.
This is different enough from the existing description that its comments no
longer apply, so I removed them. A further chunk of work is to change the sync
RFC to document this algorithm.
* sync: reduce block timeout
* state: add resource limits for sled
Closes#888
* sync: add a restart timeout constant
* sync: de-pub constants
* change several tests to transcript in consensus chain tests
* rename transcripts
* rename state transcript
* fix spandocs
* add timeout layer to tests
* run transcripts on the wrapped timeout service, remove ready calls
Log a message with the height when we get duplicate blocks.
Downgrade some verifier errors and warnings to info and debug, because
some peers on mainnet consistently provide bad blocks.
Reduce the amount of time that the block verifier waits for the previous
block, before polling again.
(Waiting for 2 seconds resulted in some apparent block verifier hangs.)
This is a temporary fix, until the state layer handles context checks.
Alfredo Garcia
Deirdre Connolly
dependabot[bot]
teor
Deirdre Connolly
Jack Grigg
Jane Lusby
Jane Lusby
Henry de Valence
Henry de Valence
Ramana Venkata