The currently provided `ConnectionHandlerUpgrErr` is very hard to use. Not only does it have a long name, it also features 3 levels of nesting which results in a lot of boilerplate. Last but not least, it exposes `multistream-select` as a dependency to all protocols.
We fix all of the above by renaming the type to `StreamUpgradeError` and flattening out its interface. Unrecoverable errors during protocol selection are hidden within the `Io` variant.
Related: #3759.
Pull-Request: #3882.
This patch removes all deprecated legacy code from `libp2p-noise` and attempts to collapse all the abstraction layers into something a lot simpler.
Pull-Request: #3511.
When an inbound stream upgrade fails, there isn't a whole lot we can do about that in the handler. In fact, for several errors, we wouldn't even know which specific handler to target, for example, `NegotiationFailed`. Similiarly, in case of an IO error during the upgrade, we don't know which handler the stream was eventually meant to be for.
Pull-Request: #3605.
Previously, a protocol could be any sequence of bytes as long as it started with `/`. Now, we directly parse a protocol as `String` which enforces it to be valid UTF8.
To notify users of this change, we delete the `ProtocolName` trait. The new requirement is that users need to provide a type that implements `AsRef<str>`.
We also add a `StreamProtocol` newtype in `libp2p-swarm` which provides an easy way for users to ensure their protocol strings are compliant. The newtype enforces that protocol strings start with `/`. `StreamProtocol` also implements `AsRef<str>`, meaning users can directly use it in their upgrades.
`multistream-select` by itself only changes marginally with this patch. The only thing we enforce in the type-system is that protocols must implement `AsRef<str>`.
Resolves: #2831.
Pull-Request: #3746.
Previously, we would specify the version and path of our workspace dependencies in each of our crates. This is error prone as https://github.com/libp2p/rust-libp2p/pull/3658#discussion_r1153278072 for example shows. Problems like these happened in the past too.
There is no need for us to ever depend on a earlier version than the most current one in our crates. It thus makes sense that we manage this version in a single place.
Cargo supports a feature called "workspace inheritance" which allows us to share a dependency declaration across a workspace and inherit it with `{ workspace = true }`.
We do this for all our workspace dependencies and for the MSRV.
Resolves#3787.
Pull-Request: #3715.
Previously, we only honored the `support_floodsub` setting when the user did not specify a custom protocol for gossipsub. This patch fixes this and allows users to advertise floodsub even when they use a custom protocol.
Pull-Request: #3837.
- Move helpers to the bottom
- Restructure tests to follow arrange-act-assert
- Only set necessary options on builder for each test
Pull-Request: #3836.
The `unreachable_pub` lint makes us aware of uses of `pub` that are not actually reachable from the crate root. This is considered good because it means reading a `pub` somewhere means it is actually public API. Some of our crates are quite large and keeping their entire API surface in your head is difficult.
We should strive for most items being `pub(crate)`. This lint helps us enforce that.
Pull-Request: #3735.
Previously, we closed the entire connection upon receiving too many upgrade errors. This is unnecessarily aggressive. For example, an upgrade error may be caused by the remote dropping a stream during the initial handshake which is completely isolated from other protocols running on the same connection.
Instead of closing the connection, set `KeepAlive::No`.
Related: #3591.
Resolves: #3690.
Pull-Request: #3625.
This PR renames some method names that don't follow Rust naming conventions or behave differently from what the name suggests:
- Enforce "try" prefix on all methods that return `Result`.
- Enforce "encode" method name for methods that return encoded bytes.
- Enforce "to_bytes" method name for methods that return raw bytes.
- Enforce "decode" method name for methods that convert encoded key.
- Enforce "from_bytes" method name for methods that convert raw bytes.
Pull-Request: #3775.
Doesn't change any functionality but `pop` returns an `Option` whereas `remove` will panic on out-of-bounds. I am more comfortable with `pop` and a pattern match. Also, usage of `continue` allows us to not use an `else`.
Pull-Request: #3734.
Previously, we only mutably borrowed the `last_seq_no` in the current scope but did not modify the underlying number. This is because `u64` is copy and calling `wrapping_add` consumes `self` so the compiler just copied it. We introduce a new-type instead that is not `Copy`.
Additionally, `wrapping_add` and initializing with a random u64 might actually warp the number and thus not give us sequential numbers as intended in #3551. To solve this, we initialize with the current unix timestamp in nanoseconds. This allows a node to publish 1000000 messages a second and still not reuse sequence numbers even after a restart / re-initialization of the configuration. This is also what the go implementation does.
Resolves#3714.
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Pull-Request: #3716.
The check for fanout peer inclusion in `test_join` does not check anything since `new_peers` is always empty.
12b785e94e/protocols/gossipsub/src/behaviour/tests.rs (L611)
I assume the intention was to fill the `new_peers` with the fanout peers. In this MR I do just that.
Pull-Request: #3628.
In earlier iterations of the design for generic connection management, we removed the `ConnectionId::new` constructor because it would have allowed users to create `ConnectionId`s that are already taken, thus breaking invariants that `NetworkBehaviour`s rely on. Later, we incorporated the creation of `ConnectionId` in `DialOpts` which mitigates this risk altogether.
Thus, it is reasonably safe to introduce a public, non-deprecated constructor for `ConnectionId` that can be used for tests.
Related https://github.com/libp2p/rust-libp2p/pull/3327#issuecomment-1469870307.
Pull-Request: #3652.
This modifies the gossipsub implementation to use monotonically increasing sequence numbers for signed messages (as dictated by the specification). There is a discussion about this in #3453. This change will make rust-libp2p gossipsub align with the go-implementation when messages are signed.
Messages will however still use randomized sequence numbers when messages are unsigned for security reasons (as discussed in the issue linked).
This shouldn't change any user-level API, only the seqno behavior. It is fully backwards compatible.
Resolves#3453.
Pull-Request: #3551.
This patch-set introduces `libp2p-swarm-test`. It provides utilities for quick and safe bootstrapping of tests for `NetworkBehaviour`s. The main design features are:
- Everything has timeouts
- APIs don't get in your way
- Minimal boilerplate
Closes#2884.
Pull-Request: #2888.
Instead of relying on `protoc` and buildscripts, we generate the bindings using `pb-rs` and version them within our codebase. This makes for a better IDE integration, a faster build and an easier use of `rust-libp2p` because we don't force the `protoc` dependency onto them.
Resolves#3024.
Pull-Request: #3312.
Prefixing a variable with an underscore (`_`) in Rust indicates that it is not used. Through refactorings, it can sometimes happen that we do end up using such a variable. In this case, the underscore should be removed.
Clippy can help us with this.
Pull-Request: #3484.
Previously, a `ConnectionHandler` was immediately requested from the `NetworkBehaviour` as soon as a new dial was initiated or a new incoming connection accepted.
With this patch, we delay the creation of the handler until the connection is actually established and fully upgraded, i.e authenticated and multiplexed.
As a consequence, `NetworkBehaviour::new_handler` is now deprecated in favor of a new set of callbacks:
- `NetworkBehaviour::handle_pending_inbound_connection`
- `NetworkBehaviour::handle_pending_outbound_connection`
- `NetworkBehaviour::handle_established_inbound_connection`
- `NetworkBehaviour::handle_established_outbound_connection`
All callbacks are fallible, allowing the `NetworkBehaviour` to abort the connection either immediately or after it is fully established. All callbacks also receive a `ConnectionId` parameter which uniquely identifies the connection. For example, in case a `NetworkBehaviour` issues a dial via `NetworkBehaviourAction::Dial`, it can unambiguously detect this dial in these lifecycle callbacks via the `ConnectionId`.
Finally, `NetworkBehaviour::handle_pending_outbound_connection` also replaces `NetworkBehaviour::addresses_of_peer` by allowing the behaviour to return more addresses to be used for the dial.
Resolves#2824.
Pull-Request: #3254.
We create the `ConnectionId` for the new connection as part of `DialOpts`. This allows `NetworkBehaviour`s to accurately track state regarding their own dial attempts.
This patch is the main enabler of https://github.com/libp2p/rust-libp2p/pull/3254. Removing the `handler` field will allow us to deprecate the `NetworkBehaviour::new_handler` function in favor of four new ones that give more control over the connection lifecycle.
Most of this is trivial, apart from the rename of the `clippy::derive_hash_xor_eq` lint to `clippy::derived_hash_with_manual_eq`.
Instead of allowing that lint, we manually implement `PartialEq` and add a comment why the difference between the `PartialEq` and `Hash` implementations are okay.
Previously, we used the full reference to the `OutEvent` of the `ConnectionHandler` in all implementations of `NetworkBehaviour`. Not only is this very verbose, it is also more brittle to changes. With the current implementation plan for #2824, we will be removing the `IntoConnectionHandler` abstraction. Using a type-alias to refer to the `OutEvent` makes the migration much easier.
The gossipsub tests are calling lifecycle functions of the `NetworkBehaviour` that aren't meant to be called outside of `Swarm`. This already surfaced as a problem in https://github.com/libp2p/rust-libp2p/pull/3327 and it is coming up again in https://github.com/libp2p/rust-libp2p/pull/3254 where `new_handler` gets deprecated.
Try to mitigate that by constructing a dummy handler instead. Functionally, there is no difference as in both cases, the given handler has never seen a connection.
Instead of offering a public constructor, users are now no longer able to construct `ConnectionId`s at all. They only public API exposed are the derived traits. Internally, `ConnectionId`s are monotonically incremented using a static atomic counter, thus no two connections will ever get assigned the same ID.
It doesn't appear that https://github.com/rust-lang/rust-clippy/issues/10061 is going to be fixed any time soon. In the meantime, our CI is "red" which is misleading because we purposely don't require this CI check. It will however hit stable in ~ 2 weeks at which point our required clippy CI check will fail.
Suppress clippy lint with an `allow` to make it pass.
Trait bounds on struct declarations should be avoided as much as possible because they creep into every reference of the type. To supply default type parameters, we don't need the trait bounds.
Remove the `derive_builder` dev-dependency in gossipsub. We can manually implement the builder functionality on top of the `Default` instance of `InjectNodes`.
Resolved#3228.
Currently, we store messages to be sent to the `ConnectionHandler` in an `Arc`. However, we never actually clone these messages as we can see with this patch, hence we remove this wrapping.
Related: https://github.com/libp2p/rust-libp2p/pull/3242
As I do frequently, I corrected for the latest clippy warnings. This will make sure the CI won't complain in the future. We could automate this btw and maybe run the nightly version of clippy.
Previously, we had one callback for each kind of message that a `ConnectionHandler` would receive from either its `NetworkBehaviour` or the connection itself.
With this patch, we combine these functions, resulting in two callbacks:
- `on_behaviour_event`
- `on_connection_event`
Resolves#3080.
