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`).
Copies the template script into the git hooks DIR to prevent a potential
attacker overwriting its contents. Also introduces a check before
running the hook against sensitive files from `master`, if there are
changes between the branch and `master`, asks user to confirm
continuation of hook execution.
Introduce the `simulation` directory in anticipation of migrating the
`radicle simulation network` repository. Introduce a loose plan and
explainer of how it will work.
A new module to model the domain of protocol lines and commands
being exchanged is introduced.
This is to increase readability and to pave the way for a
future sans I/O version of the binary crate.
Co-authored-by: Lorenz Leutgeb <lorenz.leutgeb@radicle.xyz>
The service learns to consult the policies database to check if a
`NodeId` is blocked.
When a node is blocked the service will prevent any inbound or
outbound connection.
In the case of an inbound connection, a disconnect must be submitted
to the reactor.
In the case of an outbound connection, nodes are filtered out from any
connection lists, and prevented from submitting a connect to the
reactor.
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.
To prepare for future changes, warn users if their config file currently
does not explicitly specify a default seeding policy.
Because there are now potentially multiple warnings generated that all
relate to the configuration file, group these together and adjust the
wording to be more uniform.
The `SeedingPolicy` type now requires a `Scope`, however existing
configurations allow it to not be specified.
Introduce a `radicle::node::config::Scope` type that allows for an
optional `policy::Scope`. The default value resolves to
`policy::Scope::All`. This preserves backwards compatibility for
existing configurations.
As part of the work to change the default scope, one of the tests
surfaced an issue where the topology was Alice <--> Bob <--> Eve,
however because of the previous 'all' scope Alice could receive Eve
references via Bob, but since the scope was changed to 'follow' Alice
would need to have a direct connection to Eve. TODO left here for future
consideration.
The previous implementation used 'all' as the default for scope, this
could lead to surprising behaviour where a user would fetch all
references for cloned and seeded repositories. Instead have a progressive,
safe by default value - where it fetches only 'followed' references. Later
a user can decide to set the scope to 'all'. NOTE: the default policy
scope was not changed from 'all' and is intended to be changed at a
later date.
Previous implementation used the default scope when a scope wasn't
present. Scope is not present when the policy is block, because block
doesn't have a scope. Instead use an empty string when printing the
block policy.
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.
When a non-delegate attempts to update the identity document, their
action will be rejected.
Provide a better error, along with a hint, to the non-delegate.
Changes the `Error::WithHint::hint` field to use a `String` type instead of
`&'static str`. This allows the caller to provide extra hint
information.
To make it easier to use an `Error::with_hint` constructor is added.
The previous error `UnexpectedState` was opaque, and ended up in a
confusing message when a user would try to edit the identity document
while not being a deleagte themselves.
Instead, provide a more specific error that mentions the `Did`
performing the action, and the action itself.
Note that the `UnexpectedState` variant was matched on during
evaluation, meaning that operations would still appear in the timeline
of the COB. With the change to the other variant, the timeline does
not record the operation anymore.
Instead of changing `impl From<SystemTime> for LocalTime` to `TryFrom`,
the implementation was removed, as there are not many callsites, and
they are well served by `LocalTime::now`.
radicle-localtime: drop TryFrom SystemTime
radicle-node: switch SystemTime usage with LocalTime
The test 'test_concurrent_fetches' is broken. It asserts over empty
sets nullifying its utility. Correct the test by changing it so that
sets to be asserted against are cloned.
Adrian Duke