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.
This change has two benefits:
* reduces conflicts with the sled refactor and any replacement
* allows the function to be called independently for testing
`check_contextual_validity` mistakenly used the new block's hash to try
to get the parent block from the state. This caused a panic, because the
new block isn't in the state yet.
Use `StateService::chain` to get the parent block, because we'll be
using `chain` for difficulty adjustment contextual verification anyway.
* Add internal iterator API for accessing relevant chain blocks
* get blocks from all chains in non_finalized state
* Impl FusedIterator for service::Iter
* impl ExactSizedIterator for service::Iter
* let size_hint find heights in side chains
Co-authored-by: teor <teor@riseup.net>
* Add transcript test for requests while state is empty
* Add happy path test for each query once the state is populated
* let populate logic handle out of order blocks
* Add a maximum queued height metric to the finalized state
And rename all the finalized state metrics to contain "finalized".
* Use i32 and -1 instead of Option<Height>
Co-authored-by: Jane Lusby <jlusby42@gmail.com>
Prior to this PR we realized that the RFC had been drafted with the assumption that chains would be ordered from best to worst in `NonFinalizedState`. This assumption was incorrect, since `BTreeSet` only ever orders values in ascending order. This discrepancy was noticed and fixed in the code, but there were still some inconsistencies that needed to be cleaned up.
This PR updates all the incorrect or confusing comments about chain ordering in the RFC and code.
Prior to this PR `memory_state` defined and implemented functionality for three different types, `Chain`, `NonFinalizedState`, and `QueuedBlocks`. Each of these components will need a fair number of unit tests, and I realized that as its currently organized it would be difficult to organize the tests or at a glance figure out which tests are testing which components.
This PR changes the organization of `memory_state` such that each component it exports is defined in its own module. In follow up PRs each module will get its own test module, which will focus exclusively on unit tests for the item defined there-in.
- [Tracking Issue](https://github.com/ZcashFoundation/zebra/issues/1250)
* make service use both finalized and non-finalized state
* Document new functions
* add documentation to sled fns
* cleanup tip fn now that errors are gone
* rename height unwrap fn
## Motivation
While working on the block locator fix PR together with Henry we noticed that we'd accidentally serialized entire transactions in `tx_by_hash`, instead of serializing just the height of the block and the index of the transaction within the block, as described by the original RFC.
## Solution
We've fixed it by adding a `TransactionLocation` new type, which handles the sled format traits. We've removed the sled format impls for `Transaction` to prevent inserting the wrong data in the future. Finally we've bumped the database format to reflect the change in the format on the disk and its incompatibility with previous versions.
Closes#1026
Because of the way that sled uses this parameter, the actual in-memory
size may be much larger. Dialing this down should help avoid high
memory usage.
* Run large checkpoint sync tests in CI
* Improve test child output match error context
* Add a debug_stop_at_height config
* Use stop at height in acceptance tests
And add some restart acceptance tests, to make sure the stop at
height feature works correctly.
## Motivation
The zebra-state service needs to be able to handle duplicate blocks.
## Solution
This implements changes already outlined by [The State
RFC](https://zebra.zfnd.org/dev/rfcs/0005-state-updates.html). We check for
successfully committed blocks first, since interacting with the queued blocks
struct at this point just complicates the implimentation. If the block has not
already been committed we then check if the block has already been queued, if
not we handle the block normally (normally here being the bit we already had
implemented).
## Documentation Changes
- [x] Update the state RFC to match the ways this fix departs from the design
- the main thing is that I switched the order of checking for duplicates
- [x] ~~Add newly added functions to the state rfc~~ Decided not to do this because they're minor getters that don't influence the rest of the design and aren't exposed as part of the API
- [x] Document newly added functions inline
## Testing
## Related Issues
- fixes https://github.com/ZcashFoundation/zebra/issues/1182
- tracking issue https://github.com/ZcashFoundation/zebra/issues/1049
Co-authored-by: teor <teor@riseup.net>
We already use an actor model for the state service, so we get an
ordered sequence of state queries by message-passing. Instead of
performing reads in the futures we return, this commit performs them
synchronously. This means that all sled access is done from the same
task, which
(1) might reduce contention
(2) allows us to avoid using sled transactions when writing to the
state.
Co-authored-by: Jane Lusby <jane@zfnd.org>
Co-authored-by: Jane Lusby <jane@zfnd.org>
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
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.
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>
* implement most of the chain functions
* implement fork
* fix outpoint handling in Chain struct
* update expect for work
* split utxo into two sets
* update the Chain definition
* remove allow attribute in zebra-state/lib.rs
* merge ChainSet type into MemoryState
* Add error messages to asserts
* export proptest impls for use in downstream crates
* add testjob for disabled feature in zebra-chain
* try to fix github actions syntax
* add module doc comment
* update RFC for utxos
* add missing header
* working proptest for Chain
* propagate back results over channel
* Start updating RFC to match changes
* implement queued block pruning
* and now it syncs wooo!
* remove empty modules
* setup config for proptests
* re-enable missing_docs lint
* update RFC to match changes in impl
* add documentation
* use more explicit variable names
* Begin work on RFC5 implementation
* I think this is necessary
* holy shit supertrait implemented via subtrait
* implement most of the chain functions
* change to slightly better name
* implement fork
* fix outpoint handling in Chain struct
* update expect for work
* resolve review comment
* split utxo into two sets
* update the Chain definition
* just a little more
* update comment
* Apply suggestions from code review
Co-authored-by: teor <teor@riseup.net>
* apply changes from code review
* remove allow attribute in zebra-state/lib.rs
* Update zebra-state/src/memory_state.rs
Co-authored-by: teor <teor@riseup.net>
* merge ChainSet type into MemoryState
* rename state impl types
* Add error messages to asserts
* checkpoint so I can split off arbitrary changes into a PR
* export proptest impls for use in downstream crates
* add testjob for disabled feature in zebra-chain
* run rustfmt
* try to fix github actions syntax
* differentiate name
* prove that github action tests zebra-chain build without features
* revert change from last commit now that test is running
* remove accidentally introduced newline
* checkpoint
* add module doc comment
* update RFC for utxos
* add missing header
* working proptest for Chain
* apply change from chain impl PR
* setup config for proptests
* Update zebra-chain/src/block/arbitrary.rs
Co-authored-by: teor <teor@riseup.net>
* run rustfmt
Co-authored-by: teor <teor@riseup.net>
* Begin work on RFC5 implementation
* I think this is necessary
* holy shit supertrait implemented via subtrait
* implement most of the chain functions
* change to slightly better name
* implement fork
* fix outpoint handling in Chain struct
* update expect for work
* resolve review comment
* split utxo into two sets
* update the Chain definition
* just a little more
* update comment
* Apply suggestions from code review
Co-authored-by: teor <teor@riseup.net>
* apply changes from code review
* remove allow attribute in zebra-state/lib.rs
* Update zebra-state/src/memory_state.rs
Co-authored-by: teor <teor@riseup.net>
* merge ChainSet type into MemoryState
* rename state impl types
* Add error messages to asserts
* add module doc comment
* update RFC for utxos
* add missing header
Co-authored-by: teor <teor@riseup.net>
* export proptest impls for use in downstream crates
* add testjob for disabled feature in zebra-chain
* run rustfmt
* try to fix github actions syntax
* differentiate name
* prove that github action tests zebra-chain build without features
* revert change from last commit now that test is running
* remove accidentally introduced newline
* Update .github/workflows/ci.yml
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
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
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.
The previous code filled in block height 0 for a missing coinbase height
in `SledState::commit_finalized`, since the genesis block is the only
block without a coinbase height (because of a mistake when it was
created).
However, @teor2345 noticed that this is incorrect, because we already
parse the genesis block specially and fill in its coinbase height
correctly. So instead, we can .expect it to be present, because we can
assume that all finalized blocks are valid.
The new `StateService` type wraps a `SledState` and a `MemoryState`.
This will allow the sled-related code and the in-memory code to be kept
separate, with the top-level `StateService` making method calls to one
or the other, as appropriate.
This commit removes the existing Service impl for the SledService. This
saves time in refactoring, and the code needs to be rewritten
anyways so there's no loss to deleting it now.
* 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
Jane Lusby
Henry de Valence
teor
Jane Lusby
dependabot[bot]
Alfredo Garcia
Deirdre Connolly
Ramana Venkata