Commit Graph

30 Commits

Author SHA1 Message Date
Janito Vaqueiro Ferreira Filho 5d9893cf31
Send crawled transaction IDs to downloader (#2801)
* Rename type parameter to be more explicit

Replace the single letter with a proper name.

* Remove imports for `Request` and `Response`

The type names will conflict with the ones for the mempool service.

* Attach `Mempool` service to the `Crawler`

Add a field to the `Crawler` type to store a way to access the `Mempool`
service.

* Forward crawled transactions to downloader

The crawled transactions are now sent to the transaction downloader and
verifier, to be included in the mempool.

* Derive `Eq` and `PartialEq` for `mempool::Request`

Make it simpler to use the `MockService::expect_request` method.

* Test if crawled transactions are downloaded

Create some dummy crawled transactions, and let the crawler discover
them. Then check if they are forwarded to the mempool to be downloaded
and verified.

* Don't send empty transaction ID list to downloader

Ignore response from peers that don't provide any crawled transactions.

* Log errors when forwarding crawled transaction IDs

Calling the Mempool service should not fail, so if an error happens it
should be visible. However, errors when downloading individual
transactions can happen from time to time, so there's no need for them
to be very visible.

* Document existing `mempool::Crawler` test

Provide some depth as to what the test expect from the crawler's
behavior.

* Refactor to create `setup_crawler` helper function

Make it easier to reuse the common test setup code.

* Simplify code to expect requests

Now that `zebra_network::Request` implement `Eq`, the call can be
simplified into `expect_request`.

* Refactor to create `respond_with_transaction_ids`

A helper function that checks for a network crawl request and responds
with the given list of crawled transaction IDs.

* Refactor to create `crawler_iterator` helper

A function to intercept and respond to the fanned-out requests sent
during a single crawl iteration.

* Refactor to create `respond_to_queue_request`

Reduce the repeated code necessary to intercept and reply to a request
for queuing transactions to be downloaded.

* Add `respond_to_queue_request_with_error` helper

Intercepts a mempool request to queue transactions to be downloaded, and
responds with an error, simulating an internal problem in the mempool
service implementation.

* Derive `Arbitrary` for `NetworkUpgrade`

This is required for deriving `Arbitrary` for some error types.

* Derive `Arbitrary` for `TransactionError`

Allow random transaction errors to be generated for property tests.

* Derive `Arbitrary` for `MempoolError`

Allow random Mempool errors to be generated for property tests.

* Test if errors don't stop the mempool crawler

The crawler should be robust enough to continue operating even if the
mempool service fails to download transactions or even fails to handle
requests to enqueue transactions.

* Reduce the log level for download errors

They should happen regularly, so there's no need to have them with a
high visibility level.

Co-authored-by: teor <teor@riseup.net>

* Stop crawler if service stops

If `Mempool::poll_ready` returns an error, it's because the mempool
service has stopped and can't handle any requests, so the crawler should
stop as well.

Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Conrado Gouvea <conrado@zfnd.org>
2021-10-05 10:55:42 +10:00
Alfredo Garcia 21a3e434bc
remove some dead code attributes (#2820) 2021-10-01 15:59:59 -03:00
Janito Vaqueiro Ferreira Filho 50a5728d0b
Test if the mempool storage is cleared (#2815)
* Move mempool tests into `tests::vector` sub-module

Make it consistent with other test modules and prepare for adding
property tests.

* Reorder imports

Make it consistent with the general guidelines followed on other
modules.

* Export `ChainTipBlock` and `ChainTipSender`

Allow these types to be used in other crates for testing purposes.

* Derive `Arbitrary` for `ChainTipBlock`

Make it easy to generate random `ChainTipBlock`s for usage in property
tests.

* Refactor to move test methods into `tests` module

Reduce the repeated test configuration attributes and make it easier to
see what is test specific and what is part of the general
implementation.

* Add a `Mempool::dummy_call` test helper method

Performs a dummy call just so that `poll_ready` gets called.

* Use `dummy_call` in existing tests

Replace the custom dummy requests with the helper method.

* Test if the mempool is cleared on chain reset

A chain reset should force the mempool storage to be cleared so that
transaction verification can restart using the new chain tip.

* Test if mempool is cleared on syncer restart

If the block synchronizer falls behind and then starts catching up
again, the mempool should be disabled and therefore the storage should
be cleared.
2021-10-01 14:44:25 +00:00
Conrado Gouvea 16a4110475 Cancel all mempool download and verify tasks when a network upgrade activates 2021-09-30 16:35:39 -04:00
Conrado Gouvea 18acec6849
Send mined transaction IDs to the download/verify task for cancellation (#2786)
* Send mined transaction IDs to the download and verify task for cancellation

* Pass a HashSet of transaction hashes to be cancelled

* Add mempool_cancel_mined() test

* Fix starvation in test

* Fix typo in comment
2021-09-30 12:09:08 +10:00
Alfredo Garcia 37595c4b32
Mempool support for transaction expiration (#2774)
* mempool - support transaction expiration

* use `LatestChainTip` instead of state call

* clippy

* remove spawn task

* remove non needed async from function

* remove return value

* add a `expiry_height_mut()` method to `Transaction` for testing purposes

* fix `remove_expired_transactions()`

* add a `mempool_transaction_expiration()` test

* tidy cleanup to `expiry_height()`

* improve docs

* fix the build

* try fix macos build

* extend tests

* add doc to function

* clippy

* fix build

* start tests at block two
2021-09-29 16:52:44 +00:00
Conrado Gouvea c6878d9b63
Cancel download and verify tasks when the mempool is deactivated (#2764)
* Cancel download and verify tasks when the mempool is deactivated

* Refactor enable/disable logic to use a state enum

* Add helper test functions to enable/disable the mempool

* Add documentation about errors on service calls

* Improvements from review

* Improve documentation

* Fix bug in test

* Apply suggestions from code review

Co-authored-by: teor <teor@riseup.net>

Co-authored-by: teor <teor@riseup.net>
2021-09-29 09:06:40 +10:00
Janito Vaqueiro Ferreira Filho a0d45c38f3
Reject conflicting mempool transactions (#2765)
* Add `Transaction::spent_outpoints` getter method

Returns an iterator over the UTXO `OutPoint`s spent by the transaction.

* Add `mempool::Error::Conflict` variant

An error representing that a transaction was rejected because it
conflicts with another transaction that's already in the mempool.

* Reject conflicting mempool transactions

Reject including a transaction in the mempool if it spends outputs
already spent by, or reveals nullifiers already revealed by another
transaction in the mempool.

* Fix typo in documentation

Remove the `r` that was incorrectly added.

Co-authored-by: teor <teor@riseup.net>

* Specify that the conflict is a spend conflict

Make the situation clearer, because there are other types of conflict.

Co-authored-by: teor <teor@riseup.net>

* Clarify that the outpoints are from inputs

Because otherwise it could lead to confusion because it could also mean
the outputs of the transaction represented as `OutPoint` references.

Co-authored-by: teor <teor@riseup.net>

* Create `storage::tests::vectors` module

Refactor to follow the convention used for other tests.

* Add an `AtLeastOne::first_mut` method

A getter to allow changing the first element.

* Add an `AtLeastOne::push` method

Allow appending elements to the collection.

* Derive `Arbitrary` for `FieldNotPresent`

This is just to make the code that generates arbitrary anchors a bit
simpler.

* Test if conflicting transactions are rejected

Generate two transactions (either V4 or V5) and insert a conflicting
spend, which can be either a transparent UTXO, or a nullifier for one of
the shielded pools. Check that any attempt to insert both transactions
causes one to be accepted and the other to be rejected.

* Delete a TODO comment that we decided not to do

Co-authored-by: teor <teor@riseup.net>
2021-09-28 01:03:08 +00:00
Conrado Gouvea b42ab67a4b
Add tests for mempool Request::Queue (#2770)
* Add tests for mempool Request::Queue

* Update test to work after refactoring

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
2021-09-23 21:13:52 +00:00
Marek 30c9618207
Clear mempool at a network upgrade (#2773)
* Update the expiry TODO

* Clear the mempool at a chain tip reset

* Clear the mempool by using a sync method (#2777)

* Clear the mempool by using a sync method

* Update docs

* Apply suggestions from code review

Co-authored-by: teor <teor@riseup.net>

* Refactor last_tip_change()

* Apply suggestions from code review

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>

* Fix brackets

* Use best_tip_block instead of manual borrowing

Co-authored-by: teor <teor@riseup.net>

Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>

Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-09-23 19:09:44 +00:00
Janito Vaqueiro Ferreira Filho 11b77afec7
Refactor mempool tests (#2771)
* Remove return of redundant vector length

An attempt to improve readability a bit by not returning a tuple with a
value that can be obtained from a single return type.

* Refactor `unmined_transactions_in_blocks`

Use a more functional style to try to make it a bit clearer.

* Use ranges in `unmined_transactions_in_blocks`

Allow a finer control over the block range to extract the transactions
from.

* Refactor mempool test code to improve clarity

It was previously not clear that only the first genesis transaction was
being used. The remaining transactions in the genesis block were
discarded and then readded later.

* Replace `oneshot` with `call`

Remove a redundant internal `ready_and` call.

* Return an `impl Iterator` instead of a `Vec<_>`

Remove unnecessary deserializations and heap allocations.

* Refactor `mempool_storage_basic_for_network` test

Make the separation between the transactions expected to be in the
mempool and those expected to be rejected clearer.

* Replace `Iterator` with `DoubleEndedIterator`

Allow getting the last transaction more easily.

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
Co-authored-by: Conrado Gouvea <conrado@zfnd.org>
2021-09-23 13:54:14 +00:00
Janito Vaqueiro Ferreira Filho 1f1bf2ec4d
Replace `mock_peer_set` function with `MockService` (#2790)
* Use `MockService` in inbound test

Refactor the `mempool_requsets_for_transactions` test so that it uses a
`MockService` instead of the `mock_peer_set` function.

* Use `MockService` in the basic mempool test

Refactor the `mempool_service_basic` test so that it uses a
`MockService` instead of the `mock_peer_set` helper function.

* Remove the `mock_peer_set` helper function

It is not used anymore, since the usages were replaced with
`MockService`s.

Co-authored-by: Conrado Gouvea <conrado@zfnd.org>
2021-09-22 15:44:13 +00:00
Janito Vaqueiro Ferreira Filho b714b2b3b6
Create a helper `MockService` type to help with writing tests that use mock `tower::Service`s (#2748)
* Implement initial service mocking helpers

Adds a [`MockService`] type, which can be configured and built for usage
in unit tests or proptests. The mocked service can then be used to
intercept requests and respond indivdiually to them.

* Use `MockService in the `mempool::Crawler` test

Refactor it to remove the helper mock function, and use the new
`MockService` helper type.

* Use `MockService` in `CandidateSet` test vectors

Refactor to remove the manual mocking of the peer set service.

* Panic if a response is not sent by `MockService`

Change the current semantics to require all `MockService` usages to
respond to every intercepted request.

A `must_use` attribute was added to the `ResponseSender` so that the
compiler can warn when this doesn't happen.

* Allow generic error types in `MockService`

Replace the hard-coded `BoxError` as the `Service`'s error type with a
generic type parameter. This allows mocking services in locations that
require specific error types.

* Add a `ResponseSender::request` getter

Allow inspecting the request again before responding, and using
information from the request in the response.

Co-authored-by: Conrado Gouvea <conrado@zfnd.org>
2021-09-21 17:44:59 +00:00
Marek 061ad55144
Sneak chain_tip_change into mempool (#2785)
* Pass ChainTipChange to the mempool

* Fix nits
2021-09-21 17:06:52 +00:00
Conrado Gouvea 957e12e4ca
Pass sync_status to mempool (#2754)
* Pass sync_status to mempool

* Update zebrad/src/components/mempool.rs

Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>

* Remove enabled flag for now; will be handled in #2723

Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
2021-09-15 22:13:29 +00:00
Deirdre Connolly ec74f7a821 Update mempool::Storage tests to not use Clone 2021-09-14 09:06:31 -04:00
Deirdre Connolly ccbbb36f7f Remove explicit return
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
2021-09-14 09:06:31 -04:00
Deirdre Connolly c03d1c156b Add mempool::Storage::remove()
Resolves #2738
2021-09-14 09:06:31 -04:00
Deirdre Connolly eff4f8720c Allow removing a tx from mempool storage 2021-09-14 09:06:31 -04:00
Conrado Gouvea 34876113c2 Remove Clone from mempool Storage 2021-09-13 18:49:46 -04:00
Conrado Gouvea 8825a52bb8
Move transaction download and verify stream into the mempool service (#2741)
* Move transaction dowloader and verifier into the mempool service

* add test for `Storage::contains_rejected()`

* Rename DownloadAndVerify->Queue; move should_download_or_verify() to previous impl

* GossipedTx -> Gossip

* Revamp error handling

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
2021-09-13 16:28:07 -04:00
Conrado Gouvea f3ee76f202
Verify inbound PushTransactions (#2727)
* Verify inbound PushTransactions

* Add GossipedTx and refactor downloader to use it

* remove grafana changes

* remove TODOs

* Tidy the transaction fetching in mempool downloader

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
Co-authored-by: Deirdre Connolly <durumcrustulum@gmail.com>
2021-09-09 10:04:44 -03:00
Conrado Gouvea a2993e8df0
Skip download and verification if the transaction is already in the mempool or state (#2718)
* Check if tx already exists in mempool or state before downloading

* Reorder checks

* Add rejected test; refactor into separate function

* Wrap mempool in buffered service

* Rename RejectedTransactionsById -> RejectedTransactionsIds

* Add RejectedTransactionIds response; fix request name

* Organize imports

* add a test for Storage::rejected_transactions

* add test for mempool `Request::RejectedTransactionIds`

* change buffer size to 1 in the test

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
2021-09-08 18:51:17 +00:00
Janito Vaqueiro Ferreira Filho be59dd2b93
Replace `Mutex` with usage of `&mut self` (#2742)
Using `&mut self` as the receiver in the method signatures allows Rust
to infer that the type is properly `Sync`, and therefore `Send`. This
allows removing the `Mutex` work-around.
2021-09-08 09:50:46 -03:00
Conrado Gouvea 1ccb2de7c7
Add transaction downloader and verifier (#2679)
* Add transaction downloader

* Changed mempool downloader to be like inbound

* Verifier working (logs result)

* Apply suggestions from code review

Co-authored-by: teor <teor@riseup.net>

* Apply suggestions from code review

Co-authored-by: teor <teor@riseup.net>

* Fix coinbase check for mempool, improve is_coinbase() docs

* Change other downloads.rs docs to reflect the mempool downloads.rs changes

* Change TIMEOUTs to downloads.rs; add docs

* Renamed is_coinbase() to has_valid_coinbase_transaction_inputs() and contains_coinbase_input() to has_any_coinbase_inputs(); reorder checks

* Validate network upgrade for V4 transactions; check before computing sighash (for V5 too)

* Add block_ prefix to downloads and verifier

* Update zebra-consensus/src/transaction.rs

Co-authored-by: teor <teor@riseup.net>

* Add consensus doc; add more Block prefixes

Co-authored-by: teor <teor@riseup.net>
2021-09-02 00:06:20 +00:00
Janito Vaqueiro Ferreira Filho 8bff71e857
Only enable the mempool crawler after synchronization reaches the chain tip (#2667)
* Store a `SyncStatus` handle in the `Crawler`

The helper type will make it easier to determine if the crawler is
enabled or not.

* Pause crawler if mempool is disabled

Implement waiting until the mempool becomes enabled, so that the crawler
does not run while the mempool is disabled.

If the `MempoolStatus` helper is unable to determine if the mempool is
enabled, stop the crawler task entirely.

* Update test to consider when crawler is paused

Change the mempool crawler test so that it's a proptest that tests
different chain sync. lengths. This leads to different scenarios with
the crawler pausing and resuming.

Co-authored-by: teor <teor@riseup.net>
2021-08-31 10:42:25 +00:00
teor 6a9a8dfc38
Stop concurrently obtaining peer set readiness (#2672)
This avoids peer set contention when most peers are busy.

Also exit the task if the peer service returns a readiness error,
because that means it's permanently unusable.

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-08-27 15:49:41 +00:00
Deirdre Connolly 8d3f6dc026
Mempool component and storage (#2651)
* First pass at a Mempool Service, incl. a storage layer underneath

* Fixed up Mempool service and storage

* allow dead code where needed

* clippy

* typo

* only drain if the mempool is full

* add a basic storage test

* remove space

* fix test for when MEMPOOL_SIZE change

* group some imports

* add a basic mempool service test

* add clippy suggestions

* remove not needed allow dead code

* Apply suggestions from code review

Co-authored-by: teor <teor@riseup.net>

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
Co-authored-by: teor <teor@riseup.net>
2021-08-27 14:36:17 +00:00
Marek 1c232ff5ea
Enumerate mempool errors (#2615)
* Enumerate mempool errors.

* Update code formatting

* Allow dead code

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>

* Allow dead code

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>

* Add a new error

* Update error formatting

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>

* Remove TransactionQueryError

* Derive Copy and Clone

Co-authored-by: teor <teor@riseup.net>

* Remove the Copy trait

Co-authored-by: teor <teor@riseup.net>

* Rename enum variants

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
2021-08-25 18:39:27 +00:00
Janito Vaqueiro Ferreira Filho 069f7716db
Create initial transaction crawler for the mempool (#2646)
* Create initial `mempool::Crawler` type

The mempool crawler is responsible for periodically asking peers for
transactions to insert into the local mempool. This initial
implementation will periodically ask for transactions, but won't do
anything with them yet.

Also, the crawler is currently configured to be always enabled, but this
should be fixed to avoid crawling while Zebra is still syncing the
chain.

* Add a timeout to peer responses

Prevent the crawler from getting stuck if there's communication with a
peer that takes too long to respond.

* Run the mempool crawler in Zebra

Spawn a task for the crawler when Zebra starts.

* Test if the crawler is sending requests

Create a mock for the `PeerSet` service to intercept requests and verify
that the transaction requests are sent periodically.

* Use `full` Tokio features when testing

Make it simpler to select the features for test builds.

Co-authored-by: teor <teor@riseup.net>

* Link to the issue for crawler activation

Make it easy to navigate from the `TODO` comment to the current project
planning.

Co-authored-by: teor <teor@riseup.net>

* Link to the issue for downloading transactions

Make it easy to navigate from the `TODO` comment to the current project
planning.

Co-authored-by: teor <teor@riseup.net>

Co-authored-by: teor <teor@riseup.net>
2021-08-24 11:23:53 -03:00