Commit Graph

229 Commits

Author SHA1 Message Date
Alfredo Garcia 83827b9d03
Ignore non-verack and non-version messages in handshake (#3522)
* ignore non-verack message in handshake

* ignore non-version message in handshake

* add loops for waiting

* improve logging

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

Co-authored-by: teor <teor@riseup.net>
2022-02-16 00:54:09 +00:00
teor a4dd3b7396
4. Avoid repeated requests to peers after partial responses or errors (#3505)
* fix(network): split synthetic NotFoundRegistry from message NotFoundResponse

* docs(network): Improve `notfound` message documentation

* refactor(network): Rename MustUseOneshotSender to MustUseClientResponseSender

```
fastmod MustUseOneshotSender MustUseClientResponseSender zebra*
```

* docs(network): fix a comment typo

* refactor(network): remove generics from MustUseClientResponseSender

* refactor(network): add an inventory collector to Client, but don't use it yet

* feat(network): register missing peer responses as missing inventory

We register this missing inventory based on peer responses,
or connection errors or timeouts.

Inbound message inventory tracking requires peers to send `notfound` messages.
But `zcashd` skips `notfound` for blocks, so we can't rely on peer messages.
This missing inventory tracking works regardless of peer `notfound` messages.

* refactor(network): rename ResponseStatus to InventoryResponse

```sh
fastmod ResponseStatus InventoryResponse zebra*
```

* refactor(network): rename InventoryStatus::inner() to to_inner()

* fix(network): remove a redundant runtime.enter() in a test

* doc(network): the exact time used to filter outbound peers doesn't matter

* fix(network): handle block requests slightly more efficiently

* doc(network): fix a typo

* fmt(network): `cargo fmt` after rename ResponseStatus to InventoryResponse

* doc(test): clarify some test comments

* test(network): test synthetic notfound from connection errors and peer inventory routing

* test(network): improve inbound test diagnostics

* feat(network): add a proptest-impl feature to zebra-network

* feat(network): add a test-only connect_isolated_with_inbound function

* test(network): allow a response on the isolated peer test connection

* test(network): fix failures in test synthetic notfound

* test(network): Simplify SharedPeerError test assertions

* test(network): test synthetic notfound from partially successful requests

* test(network): MissingInventoryCollector ignores local NotFoundRegistry errors

* fix(network): decrease the inventory rotation interval

This stops us waiting 3-4 sync resets (4 minutes) before we retry a missing block.

Now we wait 1-2 sync resets (2 minutes), which is still a reasonable rate limit.
This should speed up syncing near the tip, and on testnet.

* fmt(network): cargo fmt --all

* cleanup(network): remove unnecessary allow(dead_code)

* cleanup(network): stop importing the whole sync module into tests

* doc(network): clarify syncer inventory retry constraint

* doc(network): add a TODO for a fix to ensure API behaviour remains consistent

* doc(network): fix a function doc typo

* doc(network): clarify how we handle peers that don't send `notfound`

* docs(network): clarify a test comment

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

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2022-02-15 01:44:33 +00:00
teor 9f2028feff
3. Send notfound when Zebra doesn't have a block or transaction (#3466)
* refactor(network): rename Advertised to Available

```sh
fastmod Advertised Available zebra*
fastmod advertised available zebra*
```

* refactor(network): allow different available and missing types inside an InventoryStatus

And rename it to ResponseStatus.

Split the methods between ResponseStatus and an InventoryStatus alias.

* refactor(network): add a block_hash convenience method to InventoryHash

* test(network): improve failure logs for connection tests

* fix(inbound): move address sanitization into the response future

* feat(network): send notfound when Zebra doesn't have a block or transaction

* doc(network): move module docs to the top of each module

This makes them more likely to get updated when the module changes.

* fix(network): stop sending unsupported missing inventory types to the registry

* test(network): inbound messages are forwarded to the registry

* test(inbound): test Peers requests to the inbound service, directly and via TCP

* test(network): notfound block responses are sent by the inbound service

* test(network): notfound tx responses are sent by the inbound service

* test(network): increase sync test mock service timeout

The code that these tests use hasn't actually changed much,
and they are only failing on some platforms (coverage, macOS).

So it seems like the extra concurrent inbound tests have pushed them
past their time limit.
(Perhaps due to TCP system calls, or extra serialization work.)

* doc(network): fix typo

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

* test(network): remove unnecessary multi-threaded runtime from tests

This prevents `MockService<zebra_state>` timeouts
in the `sync_block_too_high_extend_tips` test,
at the cost of reducing coverage of different execution orders.

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2022-02-14 01:51:34 +00:00
Janito Vaqueiro Ferreira Filho eb98b7a4b2
Estimate network chain tip height based on local node time and current best tip (#3492)
* Remove redundant documentation

The documentation was exactly the same as the documentation from the
trait.

* Calculate a mock time block delta for tests

Simulate a block being added to the chain with a random block time based
on the previous block time and the target spacing time.

* Add a `time` field to `ChainTipBlock`

Store the block time so that it's ready for a future chain that allows
obtaining the chain tip's block time.

* Add `ChainTip::best_tip_block_time` method

Allow obtaining the bes chain tip's block time.

* Add method to obtain both height and block time

Prevent any data races by returning both values so that they refer to
the same chain tip.

* Add `NetworkUpgrade::all_target_spacings` method

Returns all the target spacings defined for a network.

* Create a `NetworkChainTipEstimator` helper type

Isolate the code to calculate the height estimation in a new type, so
that it's easier to understand and doesn't decrease the readability of
the `chain_tip.rs` file.

* Add `ChainTip::estimate_network_chain_tip_height`

This is more of an extension method than a trait method. It uses the
`NetworkChainTipHeightEstimator` to actually perform the estimation, but
obtains the initial information from the current best chain tip.

* Fix typo in documentation

There was an extra closing bracket in the summary line.

* Refactor `MockChainTipSender` into a separate type

Prepare to allow mocking the block time of the best tip as well as the
block height.

* Allow sending mock best tip block times

Add a separate `watch` channel to send the best tip block times from a
`MockChainTipSender` to a `MockChainTip`.

The `best_tip_height_and_block_time` implementation will only return a
value if there's a height and a block time value for the best tip.

* Fix off-by-one height estimation error

Use Euclidean division to force the division result to round down
instead of rounding towards zero. This fixes an off-by-one error when
estimating a height that is lower than the current height, because the
fractionary result was being discarded, and it should have forced the
height to go one block back.

* Fix panics on local times very far in the past

Detect situations that might cause the block height estimate to
underflow, and return the genesis height instead.

* Fix another off-by-one height estimation error

The implementation of `chrono::Duration::num_seconds` adds one to the
number of seconds if it's negative. This breaks the division
calculation, so it has to be compensated for.

* Test network chain tip height estimation

Generate pairs of block heights and check that it's possible to estimate
the larger height from the smaller height and a displaced time
difference.
2022-02-11 01:27:02 +00:00
teor 294ea7ebae
fix(network): only send responded updates on handshake/ping/pong (#3463) 2022-02-08 23:00:58 +00:00
teor 9be13a4fb7
2. Route peer requests based on missing inventory (#3465)
* feat(network): send notfound messages to the inventory registry

* refactor(network): move the inventory filter into an async function

* feat(network): avoid routing requests to peers that are missing inventory

* test(network): advertised routing is independent of numeric address value

* test(network): peer set routes requests to peers not missing that inventory

* test(network): peer set fails requests if all ready peers are missing that inventory

* fix(clippy): needless-borrow in the peer set

* fix(lint): remove redundant trailing commas in macro calls

There is no clippy lint for this, maybe because some macros
are sensitive to trailing commas.
(But not the ones changed in this commit.)

* test(network): check the exact number of inventory peers

* doc(network): explain why we ignore inventory send failures

* docs(network): explain why a channel error is ignored
2022-02-08 01:16:41 +00:00
teor 98502d6181
1. Create an API for a missing inventory registry, but don't register any missing inventory yet (#3255)
* feat(network): create an API for registering missing inventory, but don't use it yet

* feat(constraint): implement AtLeastOne::iter_mut()

* refactor(network): add InventoryStatus::marker() method to remove associated data

* fix(network): prefer current inventory, and missing inventory statuses

* fix(network): if an inventory rotation is missed, delay future rotations

* fix(network): don't immediately rotate a new empty inventory registry

* fix(network): assert that only expected inventory variants are stored in the registry

* test(network): add a basic empty inventory registry test

Also adds an inventory registry update future,
which makes it easier to call from an async context.

* refactor(network): add a convenience API for new InventoryChanges

* feat(network): improve inventory registry logging and metrics

* test(network): make sure advertised and missing inventory is correctly registered

* test(network): check that missing inventory is preferred over advertised

* test(network): check that current inventory is preferred over previous

* test(network): check peer set routes inv requests to advertised peers

* refactor(network): make the InventoryChange API more flexible

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2022-02-06 23:05:52 +00:00
teor c76ff56cd1
fix(network): Add a send timeout to outbound peer messages (#3417)
* fix(network): add a send timeout to outbound peer messages

* test(network): test peer send and receive timeouts

And the equivalent success cases:
- spawn the run loop with no messages
- spawn the run loop and send and receive a message

* test(network): check for specific error types in the tests

And add an outbound error test that doesn't expect a response.

* test(network): use bounded fake peer connection channels

This lets us actually trigger send timeouts in the tests.

* refactor(network): rename some confusing types and variables

fastmod peer_inbound_tx peer_tx zebra*
fastmod peer_inbound_rx peer_rx zebra*

fastmod ClientSendTimeout ConnectionSendTimeout zebra*
fastmod ClientReceiveTimeout ConnectionReceiveTimeout zebra*

* doc(network test): explain the purpose of each peer connection test vector
2022-01-31 15:22:00 -03:00
teor 499ae89c80
T2. Add isolated Tor connection API, but don't enable it by default (#3303)
* Add arti as a zebra-network dependency

* Add a method for isolated anonymised Tor connections to a specific hostname

* Add tests for isolated tor connections

* Use a shared tor client instance for all isolated connections

* Silence a spurious tor warning in tests

* Make tor support optional, activate it via a new "tor" feature

* Extra Cargo.lock changes

* fastmod AsyncReadWrite PeerTransport zebra*

* Remove unnecessary PeerTransport generics

* Refactor common test code into a function

* Don't drop the stream until the end of the test

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2022-01-25 01:46:31 +00:00
Janito Vaqueiro Ferreira Filho ec207cfa95
Ignore unexpected block responses to fix error cascade when synchronizing blocks (#3374)
* Refactor setup of `Connection` test vectors

Add a `new_test_connection` helper function to create a `Connection`
instance that's ready for testing.

* Check that no inbound requests are sent

Return the mock inbound service from `new_test_connection` and assert
that no requests were sent to it in any test.

* Replace `&mut Vec<u8>` with an `mpsc` channel

Make it easier to run the connection task in the background, i.e.,
remove any lifetime constraints from the borrowed buffer so that
`Connection` is `'static`.

It's now also easier to assert on individual messages sent from the
`Connection` instance.

* Make `MockServiceBuilder::finish` public

Allow test functions to be generic when creating a `MockService`, so
that caller functions actually determine if the type of `MockService`
assertions.

* Move `new_test_connection` to parent module

Make it more generic so that it can be used later in property tests as
well.

* Derive `Eq` and `PartialEq` for network `Response`

Allow intercepted `Response` instances to be easily compared in tests.

* Test block request cancel causes an error cascade

This is the scenario that caused the block synchronizer to reset every
few minutes, which made it considerably slower.

* Ignore unexpected block responses

It's likely that it's just a response for a previously cancelled block
request.
2022-01-20 08:14:16 +00:00
teor 6c787dd188
T1. Fix isolated connection bugs, improve tests, upgrade dependencies (#3302)
* Make handshakes generic over AsyncRead + AsyncWrite

* Simplify connect_isolated using ServiceExt::map_err and BoxError

* Move isolated network tests to their own module

* Improve isolated TCP connection tests

* Add an in-memory connection test that uses AsyncReadWrite

* Support connect_isolated on testnet

* Add a wrapper function for isolated TCP connections to an IP address

* Run test tasks for a while, and clean up after them

* Upgrade Zebra dependencies to be compatible with arti, but don't add arti yet

* Fix deny.toml

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Conrado Gouvea <conrado@zfnd.org>
2022-01-14 19:34:59 +00:00
teor ac4ed57751
Cancel heartbeats that are waiting for a peer, rather than hanging Zebra (#3325)
* If the crawler is delayed, delay future crawl intervals by the same amount

* Cancel heartbeats that are waiting for network requests or responses
2022-01-12 19:15:07 +00:00
teor d076b999f3
Fix syncer download order and add sync tests (#3168)
* Refactor so that RetryLimit::Future is std::marker::Sync

* Make the syncer future std::marker::Send by spawning tips futures

* Download synced blocks in chain order, not HashSet order

* Improve MockService failure messages

* Add closure-based responses to the MockService API

* Move MockChainTip to zebra-chain

* Add a MockChainTipSender type alias

* Support MockChainTip in ChainSync and its downloader

* Add syncer tests for obtain tips, extend tips, and wrong block hashes

* Add block too high tests for obtain tips and extend tips

* Add syncer tests for duplicate FindBlocks response hashes

* Allow longer request delays for mocked services in syncer tests
2022-01-11 14:11:35 -03:00
teor 4c310c0671
3. Cleanup internal network request handler, fix unused request logging (#3295)
* Simplify connection internal request handling code

* Track unused inbound request messages correctly

* Use a custom enum for inbound message handler mappings

* Fix grammar

Co-authored-by: Marek <mail@marek.onl>

Co-authored-by: Marek <mail@marek.onl>
2022-01-07 08:05:24 +10:00
teor 144c532de4
Cache unsolicited address messages, and provide them to Zebra when requested (#3294) 2022-01-05 15:55:59 -05:00
teor 469fa6b917
1. Fix some address crawler timing issues (#3293)
* Stop holding completed messages until the next inbound message

* Add more info to network message block download debug logs

* Simplify address metrics logs

* Try handling inbound messages as responses, then try as a new request

* Improve address book logging

* Fix a race between the first heartbeat and getaddr requests

* Temporarily reduce the getaddr fanout to 1

* Update metrics when exiting the Connection run loop

* Downgrade some debug logs to trace
2022-01-04 18:43:30 -05:00
Janito Vaqueiro Ferreira Filho 1010773b05
Keep track of background peer tasks (#3253)
* Refactor to create heartbeat sender function

Move the code that's part of the heartbeat task into a separate helper
function.

* Move `Client` initialization down

Keep it closer to where it's actually used, and make it easier to add
new fields to `Client` for the connection and heartbeat tasks.

* Add background task handles to `Client` type

Prepare it to be able to check for panics or errors from the background
tasks.

* Add dummy background tasks to `ClientTestHarness`

Spawn simple timeout tasks as mock connection and heartbeat tasks.

* Fix `PeerSet` tests that use `ClientTestHarness`

Building a `ClientTestHarness` requires a Tokio runtime to be set up, so
the calls were moved into the `async` block.

* Refactor to create `set_task_exited_error`

Make the code reusable for both background tasks.

* Check heartbeat task for errors

Periodically poll it to check if the task has unexpectedly stopped.

* Check if connection background task has stopped

The client service should stop if the connection background task has
exited, because then it's not able to receive any replies.

* Allow aborting mocked `Client` background tasks

Wrap the background tasks in `Abortable`, so that they can be aborted
through the `ClientTestHarness`.

* Test if stopped connection task is detected

Check that stopping the background connection task is something that the
`Client` instance detects and handles correctly.

* Test if stopped heartbeat task is detected

Check that stopping the background heartbeat task is something that the
`Client` instance detects and handles correctly.

* Allow setting custom background tasks

Will be used later to create background tasks that panic.

* Test if `Client` handles panics in connection task

Use a mock background connection task that panics immediately, and check
that the `Client` handles it gracefully.

* Test if `Client` handles panics in heartbeat task

Use a mock background heartbeat task that panics immediately, and check
that the `Client` handles it gracefully.

* Change ticket referenced by `TODO`

The previously linked issue was a broad plan to improve Zebra's shutdown
behavior, while the new issue is more specific, and can be scheduled
sooner.

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

Co-authored-by: teor <teor@riseup.net>
2021-12-22 01:35:38 +00:00
Janito Vaqueiro Ferreira Filho b71833292d
Use `MockedClientHandle` in other tests (#3241)
* Move `MockedClientHandle` to `peer` module

It's more closely related to a `Client` than the `PeerSet`, and this
prepares it to be used by other tests.

* Rename `MockedClientHandle` to `ClientTestHarness`

Reduce confusion, and clarify that the client is not mocked.

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

* Add clarification to `mock_peers` documentation

Explicitly say how the generated data is returned.

* Rename method to `wants_connection_heartbeats`

The `Client` service only represents one direction of a connection, so
`is_connected` is not the exact term.

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

* Mock `Client` instead of `LoadTrackedClient`

Move where the conversion from mocked `Client` to mocked
`LoadTrackedClient` in order to make the test helper more easily used by
other tests.

* Use `ClientTestHarness` in `initialize` tests

Replace the boilerplate code to create a fake `Client` instance with
usages of the `ClientTestHarness` constructor.

* Allow receiving requests from `Client` instance

Create a helper type to wrap the result, to make it easier to assert on
specific events after trying to receive a request.

* Allow inspecting the current error in the slot

Share the `ErrorSlot` between the `Client` and the handle, so that the
handle can be used to inspect the contents of the `ErrorSlot`.

* Allow placing an error into the `ErrorSlot`

Assuming it is initially empty. If it already has an error, the code
will panic.

* Allow gracefully closing the request receiver

Close the endpoint with the appropriate call to the `close()` method.

* Allow dropping the request receiver endpoint

Forcefully closes the endpoint.

* Rename field to `client_request_receiver`

Also rename the related methods to include
`outbound_client_request_receiver` to make it more precise.

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

* Allow dropping the heartbeat shutdown receiver

Allows the `Client` to detect that the channel has been closed.

* Rename fn. to `drop_heartbeat_shutdown_receiver`

Make it clear that it affects the heartbeat task.

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

* Move `NowOrLater` into a new `now-or-later` crate

Make it easily accessible to other crates.

* Add `IsReady` extension trait for `Service`

Simplifies checking if a service is immediately ready to be called.

* Add extension method to check for readiness error

Checks if the `Service` isn't immediately ready because a call to
`ready` immediately returns an error.

* Rename method to `is_failed`

Avoid negated method names.

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

* Add a `IsReady::is_pending` extension method

Checks if a `Service` is not ready to be called.

* Use `ClientTestHarness` in `Client` test vectors

Reduce repeated code and try to improve readability.

* Create a new `ClientTestHarnessBuilder` type

A builder to create test `Client` instances using mock data which can be
tracked and manipulated through a `ClientTestHarness`.

* Allow configuring the `Client`'s mocked version

Add a `with_version` builder method.

* Use `ClientTestHarnessBuilder` in `PeerVersions`

Use the builder to set the peer version, so that the `version` parameter
can be removed from the constructor later.

* Use a default mock version where possible

Reduce noise when setting up the harness for tests that don't really
care about the remote peer version.

* Remove `Version` parameter from the `build` method

The `with_version` builder method should be used instead.

* Fix some typos and outdated info in the release checklist

* Add extra client tests for zero and multiple readiness checks (#3273)

And document existing tests.

* Replace `NowOrLater` with `futures::poll!` (#3272)

* Replace NowOrLater with the futures::poll! macro in zebrad

* Replace NowOrLater with the futures::poll! macro in zebra-test

* Remove the now-or-later crate

* remove unused imports

* rustfmt

Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
2021-12-22 06:13:26 +10:00
teor 8db0528165
Revert "Stop ignoring some completed Responses" (#3274)
* Revert "Stop ignoring some completed Responses"

This reverts commit 0383562e1098ee2b49a4b5dd1b37646e6512782f from PR #3120,
but keeps the metrics and logging changes since that commit.

* Document why the request handling needs to happen in this order
2021-12-21 11:55:20 -03:00
teor 6814525a7a
Update async correctness docs and the async in Zebra RFC (#3243)
* Justify that the ErrorSlot Mutex is deadlock-safe

* Document cancellation safety in the async RFC

* Document task starvation in the async RFC

Co-authored-by: Marek <mail@marek.onl>
2021-12-21 07:10:15 +00:00
teor d0e6de8040
Avoid deadlocks in the address book mutex (#3244)
* Tweak crawler timings so peers are more likely to be available

* Tweak min peer connection interval so we try all peers

* Let other tasks run between fanouts, so we're more likely to choose different peers

* Let other tasks run between retries, so we're more likely to choose different peers

* Let other tasks run after peer crawler DemandDrop

This makes it more likely that peers will become ready.

* Spawn the address book updater on a blocking thread

* Spawn CandidateSet address book operations on blocking threads

* Replace the PeerSet address book with a metrics watch channel

* Fix comment

* Await spawned address book tasks

* Run the address book update tasks concurrently (except for the mutex)

* Explain an internal-only method better

* Fix a typo

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

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
2021-12-20 00:44:43 +00:00
teor f176bb59a2
Stop ignoring some connection errors that could make the peer set hang (#3200)
* Drop peer services if their cancel handles are dropped

* Exit the client task if the heartbeat task exits

* Allow multiple errors on a connection without panicking

* Explain why we don't need to send an error when the request is cancelled

* Document connection fields

* Make sure connections don't hang due to spurious timer or channel usage

* Actually shut down the client when the heartbeat task exits

* Add tests for unready services

* Close all senders to peer when `Client` is dropped

* Return a Client error if the error slot has an error

* Add tests for peer Client service errors

* Make Client drop and error cleanups consistent

* Use a ClientDropped error when the Client struct is dropped

* Test channel and error state in peer Client tests

* Move all Connection cleanup into a single method

* Add tests for Connection

* fix typo in comment

Co-authored-by: Conrado Gouvea <conrado@zfnd.org>

Co-authored-by: Conrado Gouvea <conrado@zfnd.org>
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
2021-12-15 14:52:44 +00:00
teor 1835ec2c8d
Add diagnostics for peer set hangs (#3203)
* Use a named CancelHeartbeatTask unit struct for the channel type

* Prefer cancel handles in selects, if both are ready

* Fix message metrics to just show the command name

* Add metrics for internal requests and responses

* Add internal requests and responses to the messages dashboard

* Add a canceled metric, and peer addresses to request and response metrics

* Add a canceled messages graph

* Add connection state metrics for currently open connections

* Fix the connection state graph with new metrics

* Always send an error before dropping pending responses

* Move error detail logging into `fail_with`

* Delete an unused timer future

* Make error strings in metrics less verbose

* Downgrade some error logs to info

* Remove a redundant expect

* Avoid unnecessary allocations for connection state metrics

* Fix missed updates to mempool and block gossip metrics
2021-12-14 21:11:03 +00:00
Janito Vaqueiro Ferreira Filho 0ad89f2f41
Disconnect from outdated peers on network upgrade (#3108)
* Replace usage of `discover::Change` with a tuple

Remove the assumption that a `Remove` variant would never be created
with type changes that allow the compiler to guarantee that assumption.

* Add a `version` field to the `Client` type

Keep track of the peer's reported protocol version.

* Create `LoadTrackedClient` type

A `peer::Client` type wrapper that implements `Load`. This helps with
the creation of a client service that has extra peer information to be
accessed without having to send requests.

* Use `LoadTrackedClient` in `initialize`

Ensure that `PeerSet` receives `LoadTrackedClient`s so that it will be
able to query the peer's protocol version later on.

* Require `LoadTrackedClient` in `PeerSet`

Replace the generic type with a concrete `LoadTrackedClient` so that we
can query its version.

* Create `MinimumPeerVersion` helper type

A type to track the current minimum protocol version for connected
peers based on the current block height.

* Use `MinimumPeerVersion` in handshakes

Keep the code to obtain the current minimum peer protocol version in a
central place.

* Add a `MinimumPeerVersion` instance to `PeerSet`

Prepare it to be able to disconnect from outdated peers based on the
current minimum supported peer protocol version.

* Disconnect from ready services for outdated peers

When the minimum peer protocol version is detected to have changed
(because of a network upgrade), remove all ready services of peers that
became outdated.

* Cancel added unready services of outdated peers

Only add an unready service if it's for a peer that has a supported
protocol version. Otherwise, add it but drop the cancel handle so that
the `UnreadyService` can execute and detect that it was cancelled.

* Avoid adding ready services for outdated peers

If a service becomes ready but it's for a connection to an outdated
peer, drop it.

* Improve comment inside `crawl_and_dial`

Describe an edge case that is also handled but was not explicit.

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

* Test if calculated minimum peer version is correct

Given an arbitrary best chain tip height, check that the calculated
minimum peer protocol version is the expected value.

* Test if minimum version changes with chain tip

Apply an arbitrary list of chain tip height updates and check that for
each update the minimum peer version is calculated correctly.

* Test minimum peer version changed reports

Simulate a series of best chain tip height updates, and check for
minimum peer version updates at least once between them. Changes should
only be reported once.

* Create a `MockedClientHandle` helper type

Used to create and then track a mock `Client` instance.

* Add `MinimumPeerVersion::with_mock_chain_tip`

An extension method useful for tests, that contains some shared
boilerplate code.

* Bias arbitrary `Version`s to be in valid range

Give a 50% chance for an arbitrary `Version` to be in the range of
previously used values the Zcash network.

* Create a `PeerVersions` helper type

Helps with the creation of mocked client services with arbitrary
protocol versions.

* Create a `PeerSetGuard` helper type

An auxiliary type to a `PeerSet` instance created for testing. It keeps
track of any dummy endpoints of channels created and passed to the
`PeerSet` instance.

* Create a `PeerSetBuilder` helper type

Helps to reduce the code when preparing a `PeerSet` test instance.

* Test if outdated peers are rejected by `PeerSet`

Simulate a set of discovered peers being sent to the `PeerSet`. Ensure
that only up-to-date peers are kept by the `PeerSet` and that outdated
peers are dropped.

* Create `BlockHeightPairAcrossNetworkUpgrades` type

A helper type that allows the creation of arbitrary block height pairs,
where one value is before and the other is at or after the activation
height of an arbitrary network upgrade.

* Test if peers are dropped as they become outdated

Simulate a network upgrade, and check that peers that become outdated
are dropped by the `PeerSet`.

* Remove dbg! macros

Co-authored-by: teor <teor@riseup.net>
2021-12-09 02:54:29 +00:00
teor c55753d5bd
Add debug-level Zebra network message tracing (#3170)
* Add debug-level Zebra network message tracing

* Delete redundant spaces

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

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
2021-12-09 01:09:23 +00:00
teor a92c431c03
Ignore NotFound errors in the syncer (#3131) 2021-12-02 11:28:20 -03:00
teor ab471b0db0
Revert "Stop returning NotFound errors, use the response instead" (#3124)
* Revert "Stop returning NotFound errors, use the response instead"

This reverts commit 45871f6915c0b294502bf04917c42fdcd3b1075c.

* Fix clippy warnings

* Downgrade a frequent log to debug level
2021-12-01 05:09:54 +00:00
teor a358c410f5
Stop closing connections on unexpected messages, Credit: Equilibrium (#3120)
* Ignore unsupported messages from peers

* Ignore unknown message commands from peers

* Implement Display for Request, Response, Handler, connection::State

* Stop ignoring some completed `Response`s

* Stop returning NotFound errors, use the response instead

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
2021-11-30 19:26:17 +00:00
teor 7457edcb86
Stop asking users to report peer errors, fix a common peer error (#3054)
* Stop treating inv with mixed item types as a connection error

* Remove unused connection errors

* Stop asking users to create bug reports for peer errors
2021-11-15 11:32:18 -03:00
Dimitris Apostolou afb8b3d477
Fix typos (#3055)
Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-11-12 19:30:22 +00:00
teor 85b016756d
Refactor addr v1 serialization using a separate AddrV1 type (#3021)
* Implement addr v1 serialization using a separate AddrV1 type

* Remove commented-out code

* Split the address serialization code into modules

* Reorder v1 and in_version fields in serialization order

* Fix a missed search-and-replace

* Explain conversion to MetaAddr

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

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-11-10 06:47:50 +10:00
Marek d03161c63f
Add unused seed peers to the AddressBook (#2974)
* Add unused seed peers to the AddressBook

* Document a new `await`

We added an extra await on the AddressBook thread mutex.

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

* Fix a typo

* Refactor names

* Return early from `limit_initial_peers`

* Add `proptest`s regressions

* Return `MetaAddr` instead of `None`

* Test if `zebra_network::init()` deadlocks

* Remove unneeded regressions

* Rename `TimestampCollector` to `AddressBookUpdater` (#2992)

* Rename `TimestampCollector` to `AddressBookUpdater`

* Update comments

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

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

* Move `all_peers` instead of copying them

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

* Make `Duration` a const

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

* Use a timeout instead of measuring the elapsed time

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

* Copy `initial_peers` instead of moving them

* Refactor the position of `NewInitial` and `new_initial`

Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-11-04 08:34:00 -03:00
Janito Vaqueiro Ferreira Filho 0960e4fb0b
Update to Tokio 1.13.0 (#2994)
* Update `tower` to version `0.4.9`

Update to latest version to add support for Tokio version 1.

* Replace usage of `ServiceExt::ready_and`

It was deprecated in favor of `ServiceExt::ready`.

* Update Tokio dependency to version `1.13.0`

This will break the build because the code isn't ready for the update,
but future commits will fix the issues.

* Replace import of `tokio::stream::StreamExt`

Use `futures::stream::StreamExt` instead, because newer versions of
Tokio don't have the `stream` feature.

* Use `IntervalStream` in `zebra-network`

In newer versions of Tokio `Interval` doesn't implement `Stream`, so the
wrapper types from `tokio-stream` have to be used instead.

* Use `IntervalStream` in `inventory_registry`

In newer versions of Tokio the `Interval` type doesn't implement
`Stream`, so `tokio_stream::wrappers::IntervalStream` has to be used
instead.

* Use `BroadcastStream` in `inventory_registry`

In newer versions of Tokio `broadcast::Receiver` doesn't implement
`Stream`, so `tokio_stream::wrappers::BroadcastStream` instead. This
also requires changing the error type that is used.

* Handle `Semaphore::acquire` error in `tower-batch`

Newer versions of Tokio can return an error if the semaphore is closed.
This shouldn't happen in `tower-batch` because the semaphore is never
closed.

* Handle `Semaphore::acquire` error in `zebrad` test

On newer versions of Tokio `Semaphore::acquire` can return an error if
the semaphore is closed. This shouldn't happen in the test because the
semaphore is never closed.

* Update some `zebra-network` dependencies

Use versions compatible with Tokio version 1.

* Upgrade Hyper to version 0.14

Use a version that supports Tokio version 1.

* Update `metrics` dependency to version 0.17

And also update the `metrics-exporter-prometheus` to version 0.6.1.
These updates are to make sure Tokio 1 is supported.

* Use `f64` as the histogram data type

`u64` isn't supported as the histogram data type in newer versions of
`metrics`.

* Update the initialization of the metrics component

Make it compatible with the new version of `metrics`.

* Simplify build version counter

Remove all constants and use the new `metrics::incement_counter!` macro.

* Change metrics output line to match on

The snapshot string isn't included in the newer version of
`metrics-exporter-prometheus`.

* Update `sentry` to version 0.23.0

Use a version compatible with Tokio version 1.

* Remove usage of `TracingIntegration`

This seems to not be available from `sentry-tracing` anymore, so it
needs to be replaced.

* Add sentry layer to tracing initialization

This seems like the replacement for `TracingIntegration`.

* Remove unnecessary conversion

Suggested by a Clippy lint.

* Update Cargo lock file

Apply all of the updates to dependencies.

* Ban duplicate tokio dependencies

Also ban git sources for tokio dependencies.

* Stop allowing sentry-tracing git repository in `deny.toml`

* Allow remaining duplicates after the tokio upgrade

* Use C: drive for CI build output on Windows

GitHub Actions uses a Windows image with two disk drives, and the
default D: drive is smaller than the C: drive. Zebra currently uses a
lot of space to build, so it has to use the C: drive to avoid CI build
failures because of insufficient space.

Co-authored-by: teor <teor@riseup.net>
2021-11-02 18:46:57 +00:00
Alfredo Garcia 3402c1d8a2
Add user agent metrics (#2957)
* add remote peer user agent metrics

* add user agent to obsolete peers
2021-10-28 19:23:09 +00:00
teor 3e03d48799
Limit the number of outbound peer connections (#2944)
* Limit the number of outbound connections in the crawler

* Make zebra-network channel bounds depend on config.peerset_initial_target_size

* Bias Zebra towards outbound connections

And turn connection limits into `Config` methods.

* Downgrade some connection logs to debug

* Remove verbose or outdated fields in tracing logs

* Clarify connection limits

Includes:
- `fastmod OUTBOUND_PEER_BIAS_FRACTION OUTBOUND_PEER_BIAS_DENOMINATOR zebra*`
- clarify connection limit documentation

* Clarify inventory channel capacity

* Add zebra_network::initialize tests with limited numbers of peers

* Avoid cooperative async task starvation in the peer crawler and listener

If we don't yield in these loops, they can run for a long time before
tokio forces them to yield.

* Test the crawler with small connection limits

And use the multi-threaded runtime to avoid long hangs.

* Stop using the multi-threaded executor in tests where it's not needed

* Avoid starvation for every connection

Adds yields after inbound successes and initial peer connections.

* Add a crawler peer connection success test

* Add outbound connection limit tests

* Improve outbound tests
2021-10-27 21:28:51 +00:00
Janito Vaqueiro Ferreira Filho 2a1d4281c5
Manually pin `Sleep` futures (#2914)
* Wrap `Sleep` timer in a `Pin<Box<_>>`

The `Sleep` type doesn't implement `Unpin` in newer versions of Tokio.

* Wrap `Sleep` type in a `Pin<Box<_>>`

In newer Tokio versions the `Sleep` type doesn't implement `Unpin`, so
it needs to be manually pinned.
2021-10-22 16:06:03 -03:00
teor 4cdd12e2c4
Track the number of active inbound and outbound peer connections (#2912)
* Count the number of active inbound and outbound peer connections

And reduce the count when each connection fails.

* Fix a comment typo

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

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
2021-10-21 21:36:42 +00:00
teor c8ad19080a
Improve logging for initial peer connections (#2896)
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
2021-10-18 18:43:12 +00:00
teor e5f5ac9ce8
Fix or disable recent nightly clippy lints (#2817)
Co-authored-by: Conrado Gouvea <conrado@zfnd.org>
2021-10-01 15:26:06 +00:00
teor 966f52a280
Fix join errors in initial seed peer versions dashboard (#2811)
* Add metrics gauges for the most recent peer network protocol version

This gague lets us join the initial seeds to the network protocol versions,
even if the peer upgrades and reconnects with a different version.

* Ensure dashboard peer network versions are unique

Otherwise, prometheus returns an error,
and the dashboard shows no data.

* Make seeder labels more readable

- put labels to the right of the graph
- remove default ports

Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
2021-10-01 01:05:00 +00:00
teor 20b2e0549e
Add metrics for initial peer network protocol versions (#2804)
* Add tracing and metrics for seed peer DNS resolution

* Add a grafana dashboard for seed peers

Currently this just shows the initial peer count from each seed.

* Add tracing and metrics for peer network protocol versions

* Update peers dashboard with network protocol versions

* Show peer network protocol versions for each seeder in dashboard

* Add per-seed filter to dashboard

Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
2021-09-29 18:08:20 +00:00
teor b6fe816473
Add a `ChainTipChange` type to `await` chain tip changes (#2715)
* Rename ChainTipReceiver to CurrentChainTip

`fastmod ChainTipReceiver CurrentChainTip zebra*`

* Update chain tip documentation and variable names

* Basic chain tip change implementation, without resets

Also includes the following name changes:
```
fastmod CurrentChainTip LatestChainTip zebra*
fastmod chain_tip_receiver latest_chain_tip zebra*
```

* Clarify the difference between `LatestChainTip` and `ChainTipChange`
2021-09-01 22:31:16 +00:00
teor d2e14b22f9
Refactor BestTipHeight into a generic ChainTip sender and receiver (#2676)
* Rename BestTipHeight so it can be generalised to ChainTipSender

`fastmod BestTipHeight ChainTipSender zebra*`

For senders:
`fastmod best_tip_height chain_tip_sender zebra*`

For receivers:
`fastmod best_tip_height chain_tip_receiver zebra*`

* Rename best_tip_height module to chain_tip

* Wrap the chain tip watch channel in a ChainTipReceiver type

* Create a ChainTip trait to avoid tricky crate dependencies

And add convenience impls for optional and empty chain tips.

* Use the ChainTip trait in zebra-network

* Replace `Option<ChainTip>` with `NoChainTip`

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

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-08-27 11:34:33 +10:00
teor 047576273c
Stop converting `Message::Inv(TxId+)` into `Request::TransactionsById` (#2660)
`Message::Inv(TxId+)` is a transaction advertisement,
so it should be converted into `Request::AdvertiseTransactionIds`.

This is a copy-paste mistake from the original zebra-network
implementation.
2021-08-24 21:40:21 +00:00
teor c608260256
Support witnessed transaction IDs in zebra-network requests and responses (#2638)
* Rename internal network requests for wide transaction IDs

fastmod TransactionsByHash TransactionsById zebra*
fastmod AdvertiseTransactions AdvertiseTransactionIds zebra*
fastmod MempoolTransactions MempoolTransactionIds zebra*
fastmod TransactionHashes TransactionIds zebra*

* Update network transaction request/response comments

* Rename a transaction hash method for wide transaction IDs

fastmod transaction_hashes transaction_ids zebra-network

* Add UnminedTxId methods and conversions for InventoryHash

* Map WtxIds to unmined transaction network messages

Also, use UnminedTxId and UnminedTx in:
* Zebra's internal request and response format, and
* external Zcash network protocol messages.

* Enable WtxId mempool inventory tracking for peers

* Further clarify transaction IDs

* Use Witnessed rather than Wide for transaction IDs

And rename narrow to legacy when it only applies to v1-v4 transactions.
Otherwise, rename it to mined ID.

* Rename a missed binding
* Remove an incorrectly named binding

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-08-18 22:55:24 +00:00
Janito Vaqueiro Ferreira Filho 4c4dbfe7cd
Reject connections from outdated peers (#2519)
* Simplify state service initialization in test

Use the test helper function to remove redundant code.

* Create `BestTipHeight` helper type

This type abstracts away the calculation of the best tip height based on
the finalized block height and the best non-finalized chain's tip.

* Add `best_tip_height` field to `StateService`

The receiver endpoint is currently ignored.

* Return receiver endpoint from service constructor

Make it available so that the best tip height can be watched.

* Update finalized height after finalizing blocks

After blocks from the queue are finalized and committed to disk, update
the finalized block height.

* Update best non-finalized height after validation

Update the value of the best non-finalized chain tip block height after
a new block is committed to the non-finalized state.

* Update finalized height after loading from disk

When `FinalizedState` is first created, it loads the state from
persistent storage, and the finalized tip height is updated. Therefore,
the `best_tip_height` must be notified of the initial value.

* Update the finalized height on checkpoint commit

When a checkpointed block is commited, it bypasses the non-finalized
state, so there's an extra place where the finalized height has to be
updated.

* Add `best_tip_height` to `Handshake` service

It can be configured using the `Builder::with_best_tip_height`. It's
currently not used, but it will be used to determine if a connection to
a remote peer should be rejected or not based on that peer's protocol
version.

* Require best tip height to init. `zebra_network`

Without it the handshake service can't properly enforce the minimum
network protocol version from peers. Zebrad obtains the best tip height
endpoint from `zebra_state`, and the test vectors simply use a dummy
endpoint that's fixed at the genesis height.

* Pass `best_tip_height` to proto. ver. negotiation

The protocol version negotiation code will reject connections to peers
if they are using an old protocol version. An old version is determined
based on the current known best chain tip height.

* Handle an optional height in `Version`

Fallback to the genesis height in `None` is specified.

* Reject connections to peers on old proto. versions

Avoid connecting to peers that are on protocol versions that don't
recognize a network update.

* Document why peers on old versions are rejected

Describe why it's a security issue above the check.

* Test if `BestTipHeight` starts with `None`

Check if initially there is no best tip height.

* Test if best tip height is max. of latest values

After applying a list of random updates where each one either sets the
finalized height or the non-finalized height, check that the best tip
height is the maximum of the most recently set finalized height and the
most recently set non-finalized height.

* Add `queue_and_commit_finalized` method

A small refactor to make testing easier. The handling of requests for
committing non-finalized and finalized blocks is now more consistent.

* Add `assert_block_can_be_validated` helper

Refactor to move into a separate method some assertions that are done
before a block is validated. This is to allow moving these assertions
more easily to simplify testing.

* Remove redundant PoW block assertion

It's also checked in
`zebra_state::service::check::block_is_contextually_valid`, and it was
getting in the way of tests that received a gossiped block before
finalizing enough blocks.

* Create a test strategy for test vector chain

Splits a chain loaded from the test vectors in two parts, containing the
blocks to finalize and the blocks to keep in the non-finalized state.

* Test committing blocks update best tip height

Create a mock blockchain state, with a chain of finalized blocks and a
chain of non-finalized blocks. Commit all the blocks appropriately, and
verify that the best tip height is updated.

Co-authored-by: teor <teor@riseup.net>
2021-08-08 23:52:52 +00:00
teor 7586699f86
Support a minimum protocol version during initial block download (#2395)
* Support a min protocol version during initial block download

But don't actually use the state height yet.

Also rename some functions and constants.

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-06-29 10:49:03 +10:00
teor 3f7410d073
Security: stop gossiping failure and attempt times as last_seen times (#2273)
* Security: stop gossiping failure and attempt times as last_seen times

Previously, Zebra had a single time field for peer addresses, which was
updated every time a peer was attempted, sent a message, or failed.

This is a security issue, because the `last_seen` time should be
"the last time [a peer] connected to that node", so that
"nodes can use the time field to avoid relaying old 'addr' messages".
So Zebra was sending incorrect peer information to other nodes.

As part of this change, we split the `last_seen` time into the
following fields:
- untrusted_last_seen: gossiped from other peers
- last_response: time we got a response from a directly connected peer
- last_attempt: time we attempted to connect to a peer
- last_failure: time a connection with a peer failed

* Implement Arbitrary and strategies for MetaAddrChange

Also replace the MetaAddr Arbitrary impl with a derive.

* Write proptests for MetaAddr and MetaAddrChange

MetaAddr:
- the only times that get included in serialized MetaAddrs are
  the untrusted last seen and responded times

MetaAddrChange:
- the untrusted last seen time is never updated
- the services are only updated if there has been a handshake
2021-06-15 13:31:16 +10:00
teor 8ebb415e7c Clippy: remove needless borrows 2021-06-07 18:33:58 -04:00
teor a6e272bf1c
Fix a typo: BIP11 -> BIP111 (#2223) 2021-05-28 14:50:43 +02:00