Commit Graph

4 Commits

Author SHA1 Message Date
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
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 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
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