Whenever the node fetches new updates, it checks if canonical references can be
updated. The node has learned how to return these results and emit them as node
events. This is a breaking change since it adds a new variant the `Event` type,
which is not forwards-compatible.
I was greeted by `rad patch redact` with
called `Result::unwrap()` on an `Err` value:
Error { code: None, message: Some("failed to convert") }
stack backtrace:
[…]
3: core::result::Result<T,E>::unwrap
at …/rust-1.88.0/lib/rustlib/src/rust/library/core/src/result.rs:1137:23
4: sqlite::cursor::Row::read
at …/index.crates.io-1949cf8c6b5b557f/sqlite-0.32.0/src/cursor.rs:136:9
5: radicle::cob::patch::cache::query::find_by_revision
at ./crates/radicle/src/cob/patch/cache.rs:624:65
6: <… as radicle::cob::patch::cache::Patches>::find_by_revision
at ./crates/radicle/src/cob/patch/cache.rs:553:9
7: radicle_cli::commands::rad_patch::redact::run
at ./crates/radicle-cli/src/commands/patch/redact.rs:23:9
8: radicle_cli::commands::rad_patch::run
at ./crates/radicle-cli/src/commands/patch.rs:1026:13
[…]
It turns out that `sqlite::cursor::Row::read` is the panicky version of
`sqlite::cursor::Row::try_read` (which returns a `Result`).
While it is somewhat rare that SQLite reads fail, it is not unheard of.
In `radicle-cli` it might not be critical, but also `radicle-protocol`
and `radicle-fetch` are affected, and they could potentially panic a
`radicle-node` process.
Use `try_read` instead, and propagate down the error handling.
Ensures that preferred seeds that are not synced are made part of the `to_sync`
set, and the Announcer is constructed without an error.
As well as this, ensure that the `synced` and `to_sync` sets do not overlap by
constructing the `to_sync` and removing any `synced` elements from it.
Making a note of that `Announcer::can_continue` might never be able to hit the
`Break` case. It could be defensive programming, so I'm going to leave it in for
the time being.
Adds a test case to show that the Announcer is allowed to mark an "unknown" node
as synced.
There may be a case where gossip reached a node that the current node was not
aware of. This should be allowed and should count towards the final replication
factor.
The `Announcer` logic was subtly different to the fetching logic, where the
preferred seeds need to be announced to *and* the replication factor had to be
reached.
This patch relaxes that constraint, and allows it to hit either targets before
exiting.
This also surfaced an issue with the with the `Progress` calculation, where
calculating the `unsynced` used the `synced` value to subtract. However, this
was not required because `to_sync` is mutated, so the value is simply
`to_sync.len()`. A test case was added to ensure this is correct by checking
that invariant that the total between synced and unsynced nodes always matches
the total of synced and unsynced progress.
While running `cargo-semver-checks`, there was an issue that was surfaced
regarding `serde(into)`, `serde(try_from)` and `schemars(transparent)` being
used, and they are incompatible.
This appeared to be a semver issue, on the side of `schemars`, since the issue
would only appear when adding `radicle` as a fresh dependency. The intuition for
this is that once `schemars` was updated locally, the same error appeared when
attempting to build the `radicle` crate.
To avoid this, `schemars` provides a `!` operator for the macro[^0]:
> You can also “unset” serde attributes by including them with a ! prefix in a
> schemars attribute, which will make schemars ignore the corresponding serde
> attribute item
[^0]: https://docs.rs/schemars_derive/latest/schemars_derive/derive.JsonSchema.html#attributes
Fintan Halpenny
Lorenz Leutgeb