The re-exports from other crates got quite confusing.
Remove all `pub use` from `radicle-node`, and rewrite all `use`s
accordingly.
The result is a slight increase in the number of `use` lines, but with
improved clarity on which type is actually imported.
It appears that `radicle_node::PROTOCOL_VERSION` (which is a re-export
of `radicle::node::PROTOCOL_VERSION` is the only symbol exported from
`radicle-node` that is used in `radicle-cli` (except tests).
Since `radicle-cli` depends on `radicle` anyway, it is questionable how
much sense that re-export and use from a binary crate (!) makes.
Remove the re-export.
Reasons for the rewrite:
- Migrate away from `ec25519` as our implementation of Ed25519,
because that crate is unmaintained.
Instead of `ec25519` we use `ed25519-dalek` which is the most popular
implementation of Ed25519 in Rust.
- Migrate away from `amplify`, because we make very little use of its
features. Here, we only use `amplify::Bytes32`, which is just a thin
wrapper of `[u8; 32]`.
Instead of `amplify::Bytes32` we use `type PublicKeyBytes = [u8; 32]`.
The realization that `ExtendedSignature` is not really tied to SSH, but
is merely a representation of a key together with a signature, allows to
relax the bounds on traits in `radicle-cob`.
Separating out uses of "public key" (a rather simple model of a key,
just 32 bytes) and "verifying key" (an actual point on the Edwards
curve, ready to verify signatures, using more memory than just 32 bytes)
allows a more sensible definition of `crypto::Signer`, which propagates
to `radicle-cob`, `radicle`, `radicle-cli`, `radicle-node` and
simplifies trait bounds.
In `radicle-node`, make the fact clearer that actually the secret key
itself is required (for our implementation of Diffie-Hellman) not
"just" signing. This results in removal of the generic argument
(usually called `G`) of multiple types.
Concretizing the types in `radicle-node` this way, i.e. by (morally)
setting `G = radicle_crypto::SigningKey`, leads to removal of
`MemorySigner` and `MockSigner`, as they really just wrap a signing
key.
Re-exporting `ed25519::Signature` (via re-export in `ed25519-dalek`)
is hopefully more future-proof than using our own type for signatures.
However, the fact that `ed25519::Signature` is not serializable and
does not implement `std::hash::Hash` means that we have to resort to
ad-hoc solutions in `radicle`:
1. The Identity COB requires serialization of signatures. These are
generally questionable, since the security of the Identity COB
should rely on the security of Signed References. In the medium
term we might even want to consider ignoring signatures in actions.
2. When reading Signed References, detection of Replay Attacks uses
a `HashMap` keyed by signatures. This detection only exists for
backwards compatibility.
These two use cases are exotic enough to warrant ad-hoc implementations.
By not wrapping `ed25519::Signature`, we encourage writing more widely
compatible code in the future.
Co-authored-by: Fintan Halpenny <fintan.halpenny@gmail.com>
In preparation of a rewrite of `radicle-crypto`, move the
responsibility for generating a cryptographically secure
seed from `radicle` (transitively via dependency on
`radicle-crypto` and `ec25519`) to the dependent of `radicle`.
Applications should decide how to obtain randomness, not `radicle` via
some far-removed dependency.
Within the workspace, the only dependent using `Profile::seed` is
`radicle-cli`, in particular for the `rad auth` subcommand. To obtain
random bytes, `radicle-cli` depends on `getrandom` and uses
`getrandom::fill`.
This introduces a `fetch_timeout` setting to the node configuration,
replacing the hardcoded constant. It defines the duration a fetch stream
is allowed to stall before being aborted. The default remains 30s, but
users on anonymizing transports like Tor can now increase it to avoid
spurious timeouts during network congestion.
Clippy reports dead code on all variants except `Hours` on `Expiry`,
since the type is defined in a binary crate.
Allow dead code here, just in case we want to use the other variants in
the future. They are not a maintenance burden, as the implementation is
simple and does not see much change.
Slightly refactor:
- `impl std::fmt::Display for Expiry` to look cleaner.
- `fn collect` to borrow `RepoId`, since it only needs
a borrow, and a a borrow of `Expiry` to spare a copy
at the only call site.
- The constant for the default value of `Expiry` to be
associated with the type `Expiry`.
Remove a remark about the default value of `Expiry` from `FetchConfig`,
as the default value is not a concern there, but rather wherever
`Expiry` is initialized.
For most use-cases, i.e., except some in `radicle-node`, the channels
in `std::sync::mpsc` suffice, allowing us to get rid of the dependency
`crossbeam-channel`.
Crossbeam is sometimes mentioned to offer higher performance compared
to `std::sync::mpsc`.
For usage in `radicle-signals` (and the counterparts in `radicle-node`
and `radicle-term`) performance is irrelevant, as these channels are
used to send a singular message (to the effect of shutting down the
node or stopping the spinner). These could potentially use
`std::sync::oneshot` in the future, which is nightly-only as of
2026-06-17, see <https://github.com/rust-lang/rust/issues/143674>.
Usage in `radicle-protocol` and `radicle`, is a heavier duty.
The channels pass commands to the service (arriving e.g. via the
control socket in `radicle-node`), events from the service to other
listeners (e.g. the control socket), and fetch results from workers to
the service. For user interaction, performance is irrelevant, since
the messages are in the tens or hundreds per second. Even for delivering
events and programmatic usage of commands, thousands to hundreds of
thousands of messages per second are plenty.
Note that these channel implementations are benchmarked in the millions
of operations per second, see e.g.:
- <https://github.com/crossbeam-rs/crossbeam/tree/b23b7e8eca2efdad9bdc1ceb1aee1207a852c03b/crossbeam-channel/benchmarks#results>
- <https://github.com/fereidani/rust-channel-benchmarks/blob/b4c979a94482e5012734657c2ee153b528bc56d6/README.md>
Also, `crossbeam-channel` was merged into `std` in 2022-11, and we do
not believe that performance of the implementations has siginifcantly
drifted since then, see <https://github.com/rust-lang/rust/pull/93563>.
One minor downside is that the channels in the standard library do not
expose the number of pending messages.
The update to `flake.lock` is a simple
nix flake update rust-overlay
to be able to reach 1.97.1.
The changes to `**/*.rs` files are improvements suggested by Clippy.
The `amplify` crate is only used in a handful places to derive `impl`s
that are not very complex.
Provide the required `impl`s directly, and remove the dependency.
The tests in `crates/radicle-node/src/tests/.rs` repeat code quite a
bit, for example, construction of IP addresses. Also, `Peer` is
unnecessarily complex (too many struct members, state indicated via a
boolean, type parameter that is always instantiated for the same type).
Clean up `Peer` by removing multiple struct members, remove the
unnecessary type parameter, and provide convenience functions that make
statement of the actual tests a little less repetitive and more concise.
For test networks, five standard nodes (Amy, Bob, Cid, Dan, Eve) are all
easy to construct. Tests are refactored to make use of these nodes in
lexicgraphic oreder. The order of initialization may differ.
Modification of configuration is done by passing `FnMut`.
In the future, using the builder pattern could be explored.
The functions
- `Session::run_handshake`
- `Session::stream`
- `Session::disconnect`
are unused.
The associated type `Session::Inner` is unused.
The function `Session::display` is of limited use with little benefit.
`Transport::link_direction` is only used in implementations of
`Debug` and `Display`, but is not helpful otherwise.
Small improvements:
- Remove confusing comments in test
- Rename `has_active_sibling_accept`
to `has_accepted_active_sibling`
- Small refactoring to avoid `… } else { None }`
Fetching a large repository looped with
Failed to consume the pack sent by the remote
even though the pack appears valid (Git indexes it in full).
The cause appears to be a bug in zlib-rs 0.6.3 according to
<https://github.com/trifectatechfoundation/zlib-rs/pull/504>.
When a delegate creates a new `Revision`, their signature is implicitly added to the revision.
Prevent the delegate from an accept verdict on two active, sibling revisions by:
1. Checking that the `state` is `Active`, and then checking `has_active_sibling_accept`
2. If there is an existing verdict, then it the verdict from the new revision is stripped.
The creation of the `Revision` must still happen, as opposed to throwing an
error, in case of existing histories.
On `Action::RevisionAccept` use `has_active_sibling_accept` to check for a sibling accept.
If one is found, the `RevisionAccept` is logged and skipped.
This preserves the invariant of a single accept per delegate on active, sibling
revisions without throwing an error on existing histories.
Verifies that when Alice redacts a revision and Bob concurrently
accepts it, both nodes converge after sync. The CRDT evaluation
order (timestamp-based) determines which operation takes effect.
The test previously had B2 as a sibling of B1 (both children of A1),
because update() always uses self.current as parent. This meant
the test was testing multi-sibling rejection, not cascading.
Now B2 is a child of B1 via a manual transaction, so when B1 is
rejected (sibling E1 accepted), B2 is rejected via RejectedBy::Parent
(cascade), which is what the test name promises.
Introduce a property testing harness for asserting a set of properties expected
of the repository identity.
The harness uses the `Network` fixture for providing four nodes that can
interact with the repository identity. The state of the harness is advanced by
providing an actor and operation on the identity document. Invariants are then
asserted about the identity.
The current invariants are:
- The `current` revision is the one and only `Accepted` revision.
- The chain of revisions is valid. That is, `Active` revisions only have a
parent that is `Accepted`, and not `Rejected` or `Redacted`.
- `Active` revisions do not contain a majority approval.
- `Accepted` revisions contain a majority approval.
- `Rejected` revisions do not contain a majority approval.
- For each revision, at most one child is `Accepted`.
- For each revision, if any of its children is `Accepted`, all other
children are `Rejected`.
- A revision which is `Rejected(RejectedBy::Parent)` has a parent that
is `Rejected`.
- A revision which is `Redacted(RedactedBy::Parent)` has a parent that
is `Redacted`.
- For each revision that is `Rejected` or `Redacted`, none of its
children is `Active`.
- A sibling or ancestor revision's rejected state applies to its sibling or
descendant.
- The repository identity documents converge when all nodes have applied all
operations.
A `BoundedVec` that implements `Arbitrary` is useful for all property testing.
Move it from the `sigrefs` module so that it can be reused by other components.
Lorenz Leutgeb