Commit Graph

13 Commits

Author SHA1 Message Date
teor b0d9471214
fix(log): Stop logging peer IP addresses, to protect user privacy (#6662)
* Add a PeerSocketAddr type which hides its IP address, but shows the port

* Manually replace SocketAddr with PeerSocketAddr where needed

```sh
fastmod SocketAddr PeerSocketAddr zebra-network
```

* Add missing imports

* Make converting into PeerSocketAddr easier

* Fix some unused imports

* Add a canonical_peer_addr() function

* Fix connection handling for PeerSocketAddr

* Fix serialization for PeerSocketAddr

* Fix tests for PeerSocketAddr

* Remove some unused imports

* Fix address book listener handling

* Remove redundant imports and conversions

* Update outdated IPv4-mapped IPv6 address code

* Make addresses canonical when deserializing

* Stop logging peer addresses in RPC code

* Update zebrad tests with new PeerSocketAddr type

* Update zebra-rpc tests with new PeerSocketAddr type

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
2023-05-14 15:06:07 +00:00
Deirdre Connolly 47cf0f475f
Halborn 2023 02 20 (#6297)
* Limit version user agents to 256 bytes, rather than 2MB, needs failure tests

* Limit all inv messages to 50,000 entries, existing tests cover this

* Limit reject message strings based on network protocol, needs success and failure tests

* Catch up as fast as possible if inventory rotation is delayed, existing tests cover this

* Truncate inv channel hashes to 1000, needs success and failure tests

* Limit inv registry size to 4 MB, needs over-limit tests for inv and addr

* Test size constraints on version user agent, reject command, and reject reason (#13)

* Test inventory registry memory limits for hashes and peers (#14)

Co-authored-by: Deirdre Connolly <durumcrustulum@gmail.com>

---------

Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Arya <aryasolhi@gmail.com>
2023-03-13 18:10:15 +00:00
teor 09836d2800
fix(clippy): Put Rust format variables inline (#5783)
* cargo clippy --fix --all-features --all-targets

With rustc 1.67.0-nightly (234151769 2022-12-03)

* cargo fmt --all
2022-12-08 01:05:57 +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
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
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
teor d482900e7f Remove a redundant pattern match
Identified by clippy's redundant_pattern_match lint.
2020-12-13 22:10:05 -05:00
Henry de Valence add94c1c45 deps: move to tokio 0.3, tower 0.4
This change is mostly mechanical, with the exception of the changes to the
`tower-batch` middleware.  This middleware was adapted from `tower::buffer`,
and the `tower::buffer` code was changed to implement its own bounded queue,
because Tokio 0.3 removed the `mpsc::Sender::poll_send` method.  See

ddc64e8d4d

for more context on the Tower changes.  To match Tower as closely as possible
in order to be able to upstream `tower-batch`, those changes are copied from
`tower::Buffer` to `tower-batch`.
2020-11-20 10:08:16 -08:00
Henry de Valence 1d3892e1dc network: rename alias to BoxError
This is shorter and consistent with Tower (which is why we use it in the
first place).
2020-09-18 18:34:25 -07:00
Henry de Valence cad38415b2
network: fix bug in inventory advertisement handling (#1022)
* network: fix bug in inventory advertisement handling

The RFC https://zebra.zfnd.org/dev/rfcs/0003-inventory-tracking.html described
the use of a `broadcast` channel in place of an `mpsc` channel to get
ring-buffer behavior, keeping a bound on the size of the channel but dropping
old entries when the channel is full.

However, it didn't explicitly describe how this works (the `broadcast` channel
returns a `RecvError::Lagged(u64)` to inform receivers that they lost
messages), so the lag-handling wasn't implemented and I didn't notice in
review.

Instead, the ? operator bubbled the lag error all the way up from
`InventoryRegistry::poll_inventory` through `<PeerSet as Service>::poll_ready`
through various Tower wrappers to users of the peer set.  The error propagation
is bad enough, because it caused client errors that shouldn't have happened,
but there's a worse interaction.

The `Service` contract distinguishes between request errors (from
`Service::call`, scoped to the request) and service errors (from
`Service::poll_ready`, scoped to the service).  The `Service` contract
specifies that once a service returns an error from `poll_ready`, the service
can be assumed to be failed permanently.

I believe (but haven't tested or carefully worked through the details) that
this caused various tower middleware to report the entire peer set service as
permanently failed due to a transient inventory "error" (more of an indicator),
and I suspect that this is the cause of #1003, where all of the sync
component's requests end up failing because the peer set reported that it
failed permanently.  I am able to reproduce #1003 locally before this change
and unable to reproduce it locally after this change, though I have not tested
exhaustively.

* network: add metric for dropped inventory advertisements

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

Co-authored-by: teor <teor@riseup.net>
2020-09-07 21:24:31 -07:00
Jane Lusby 88557ddd0a address more comments 2020-09-01 21:01:38 -04:00
Jane Lusby 96c8809348
Implement Inventory Tracking RFC (#963)
* Add .cargo to the gitignore file

* Implement Inventory Tracking RFC

* checkpoint

* wire together the inventory registry

* add comment documenting condition

* make inventory registry optional
2020-09-01 14:28:54 -07:00