Fixes an intermittent failure caused by a race condition with background
gossip messages.
Previously Eve's `RefsAnnounced` message could be processed by Bob
after the test updated the identity document to include Alice-Laptop
as a delegate. This caused Bob to unexpectedly fetch the issue in the
background, failing the `!has_issue` assertion.
We now explicitly wait for Bob to receive Eve's announcement before
updating the identity document.
Avoid relying on a thread sleep to wait for the nodes to connect.
Instead rely on the connection condition being established by using a polling loop.
The loop is controlled by a number of iterations, panicking if the
number of iterations is exceeded.
Addresses intermittent failures in the `test_connection_crossing` e2e test,
which were particularly prevalent on slower CI environments (such as
`rust:trixie`).
Previously, the test spawned two threads to make Alice and Bob dial each
other concurrently and strictly asserted that the "preferred" peer (the
one with the higher Node ID) would always win the `Outbound` link direction.
However, this assumption is flawed in real-world, OS-level network execution
due to two race conditions:
1. Thread Scheduling: One thread could execute and fully establish a
connection before the other thread even began processing its dial command.
2. Reactor Event Ordering: A node's reactor might wake up and process an
incoming TCP connection from its peer *before* it processes the `Connect`
command sent by the test. When it finally processes the `Connect` command,
it sees a session already exists and skips dialing entirely.
In both scenarios, a true "simultaneous crossing" never occurs. Instead, a
standard sequential connection happens, meaning the link direction is dictated
by whoever dialed first, not by the "preferred" peer logic. This caused the
strict `left: Outbound, right: Inbound` assertions to panic.
To fix this, two changes were made:
- Introduced a `std::sync::Barrier` to synchronize the two test threads.
This forces both threads to wait for each other before calling `.connect()`,
maximizing the probability of a true simultaneous dial.
- Relaxed the final assertions. Because OS-level TCP handshakes and reactor
polling can never guarantee perfect simultaneity, we no longer assert
*which* peer gets the `Outbound` link. Instead, we assert the core invariant:
that exactly one connection is established between the nodes, and that their
link directions are opposite (`s1.link != s2.link`).
The main change that needs adjustment on our side is that
`impl EcPk for ec25519::PublicKey` changed from `Compressed = [u8; 32]`
to `Compressed = amplify::Bytes32`.
Also remove `impl Deref for PublicKey` to avoid dependents of
`radicle-crypto` depend on a particular implementation, and make the
anonymous member private.
See
<f42396139a>.
The value of `RAD_SOCKET` influences the outcome of `cargo test`.
Users that set this environment variable to, e.g. point `rad` at
a socket at a custom location, were surprised that execution of
tests would interact with their node.
Split `Home::socket` into two functions. `Home::socket_default` is
"pure" and only dependent on the path represented by `Home`. On the
other hand, `Home::socket_from_env` interprets the environment.
Most users will want to use `Home::socket_from_env`.
There is only one type that can really construct canonical references,
which is the the identity document. Remove the corresponding method
from the trait and rename it to `GetRawCanonicalRefs` accordingly.
Separate module for the concern of protecting `refs/rad`.
Note that this also fixes a bug, as previously all refs
*starting with* `refs/rad`, such as `refs/radieschen` were protected.
The method `set_head` does two things:
1. Compute the canonical head and set the default branch to target.
2. Set the symbolic reference `HEAD` to target the default branch.
Split these two concerns into:
1. `set_default_branch_to_canonical_head`
2. `set_head_to_default_branch`
The main motivation behind this change is to get just a little
telemetry information from nodes on the network, namely the version
of Radicle they are running.
This is achieved by rewriting `impl Default for UserAgent` which now
uses the version information provided by the build script also used
in other crates.
Also, a new configuration option `node.userAgent` is added, which
allows users to override the user agent if they so please, or set
the value `null`, which will in turn send the user agent
`/radicle/`, which is not really helpful, and the default prior to
this commit.
Creations of `UserAgent` in the whole workspace is cleaned up. In
order to do that `UserAgent::test` is introduced.
Instead of passing the signer as an argument to many methods on `Store`,
scope the `Store` itself to a signer.
This further allows to differentiate two different access modes on the
store in `radicle::cob::store::access`: `WriteAs` (which requires signer)
and `ReadOnly` (which does not require a signer).
The caches for issues and patches in `radicle::cob::{issue,patch}::Cache`
are concretised by removing the first type parameter, since it was
specific to issues and patches anyway. This was done in this commit as
it touches very similar usage sites.
Make `Device` less prominent, and instead lean more heavily towards
traits from the `signature` crate, such as `Keypair` and `Verifier`
in addition to `Signer`. Trait bounds regarding `Signer` could be
simplified, but this is left for the future.
In `radicle-cli`, the function `term::cob::patches_mut`, which generates
errors with a hint is used instead of the lower-level `Profile::patches_mut`.
Commands `rad issue cache` and `rad patch cache` now construct a
writeable cache on top of a read-only store.
Many knock-on changes are handled as well, to arrive at a clean state.
The intent to drop outgoing connections is modeled as `Option::None`
which is brittle and easy to miss.
Extend `enum AddressConfig` with a variant that is more explicit.
Teach the `rad sync` and `rad clone` commands to accept the
`--signed-refs-feature-level` option. This option configures that
fetch to use the minimum feature level provided when fetching from
other nodes. This overrides the value of
`node.fetch.signedReferences.featureLevel.minimum`, and should only be
used in scenarios where it is necessary to downgrade the security of a
fetch for backwards compatibility.
To achieve this, the `Handle::fetch` method introduces an optional
parameter for threading through the `FeatureLevel`.
The configuration of the fetch process was only achieved at the setup
of the of the node.
This limits the ability to pass configuration options to the fetch
worker while the node service is running.
This change introduces `radicle_protocol::fetcher::state::FetchConfig`
to allow configuration options to be kept track of. This includes the
previous `timeout` option, but now also includes the
`radicle_fetch::Config`.
The service now produces the `FetchConfig` and passes it through to
the fetch worker.
Add a configuration knob `node.fetch.signedReferences.featureLevel.minimum` to
allow node operators to enforce more strict verification when fetching
Signed References.
In order to allow automatic migration to the feature level 'parent',
introduce a way to force writing signed references, even if the
user-controlled refs are unchanged.
To detect the special case of a Signed References history with a single
and root commit (which would otherwise be detected as feature level
'parent', even though the `refs/rad/sigrefs-parent` ref cannot be
written), keep information about the parent commit in `SignedRefs`.
The output of `rad inspect --sigrefs` is changed to match the more
strict interpretation of the feature level for histories with a single
and root commit.
Configuration of database connections is not performed on `open`, which
leaves room for error (e.g. to miss specifying configuration).
Methods on `Profile` automatically supply the configuration of the
profile.
In testing code, just using the default configuration suffices.
Instead of introducing our own names and aliases, directly model SQLite
pragmas with the defaults that they also take in SQLite, to avoid
confusion.
Co-authored-by: Lorenz Leutgeb <lorenz.leutgeb@radicle.xyz>
Use proxy structs to control the serialized output of `FetcherState`.
These structs convert data inside the `FetcherState` into friendlier
output for the caller of `rad node debug`.
Since the service performs further I/O (e.g. uses SQLite), it can keep
the reactor runtime thread busy for long periods. Emit a warning if that
is the case.
100 ms is quite relaxed, this is to only catch severe cases and avoid
spamming the log.
The node being connected to may be blocking the connecting node.
In this case the service will emit an event that says the connecting
node is disconnected.
Check the events for this event as well as `Event::PeerConnected`.
Introduces the ability to explicitly block a peer via the node control
socket. Previously, the node only exposed follow and unfollow commands.
While the underlying policy database schema supported a Block variant,
there was no mechanism to trigger this state via the client handle.
The new block command:
1. Updates the node's follow policy to Block.
2. Immediately disconnects the peer if a session is active.
The previous resolution would favour one `Oid` over another, due to
the call to `Option::or`.
This was found to result in scenarios where an identity document being
advertised was older than the identity document that the fetching node
already has, and would not fetch namespaces by newly added delegates.
The resolution logic is updated to handle all cases for these `Oid`s,
and when they are both present, uses a commit graph check to see which
one is the latest.
An e2e test is added to check ensure behaviour works correctly.
Adrian Duke
Lorenz Leutgeb
Fintan Halpenny
Yorgos Saslis