mab122
/
radicle-heartwood-lfs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

radicle/sigrefs: Find first non-replayed commit 

The scenario A₁ ← B₁ ← A₂ ← B₂ would previously have resulted in A₁
being loaded. This is undesriable, since the latest non-replayed commit
is B₁.

The information regarding the order of duplicates, which is preserved by
`seen`, can be used to recover the first commit.
This commit is contained in:
Lorenz Leutgeb 2026-03-21 00:24:52 +01:00 committed by Fintan Halpenny
parent b5dc3486db
commit 9a4539fe82
3 changed files with 84 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
CHANGELOG.md
View File

@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
## [Unreleased]
## Fixed Bugs
- Fix the signed references reading process by correctly choosing the first,
non-replayed commit. This only occurs if duplicate signatures are found and
the process needs to find the first legitimate commit of the namespace.
## 1.7.1
## Fixed Bugs

43
crates/radicle/src/storage/refs/sigrefs/read.rs
View File

@ -101,16 +101,17 @@ where
}
}
/// Read a [`VerifiedCommit`] using the [`SignedRefsReader`].
/// Read a [`VerifiedCommit`] using the [`SignedRefsReader`], from a
/// linear history.
///
/// The [`VerifiedCommit`] will be the first commit, if the commit verifies
/// The [`VerifiedCommit`] will be the latest commit, if the commit verifies
/// and contains its parent in its [`Refs`] entry.
///
/// If the commit does not contain a parent, but its signature is not
/// repeated, then it is still returned.
/// Otherwise, the commit that is returned is either:
/// - The first commit which has no repeated signatures, i.e. it has no replay attacks.
/// - The first commit which is not a replay commit, i.e. the commit that
/// replay attacks are based on.
///
/// Otherwise, the latest commit that has no duplicate signatures in its
/// ancestry is returned.
///
/// # Replay Attacks
///
@ -145,6 +146,10 @@ where
return Ok(head);
}
// Note that for all sets of commits which share the same signature,
// the `NonEmpty` in `seen` will be in reverse order of the walk.
// That is, the latest commit in the set will be at the first position,
// and the earliest commit will be at the last position.
let seen = iter::Walk::new(*head.id(), self.repository).try_fold(
HashMap::<crypto::Signature, NonEmpty<Oid>>::new(),
|mut seen, commit| {
@ -181,7 +186,6 @@ where
// The second walk can start from the parent of head. We do not need to
// verify head twice, and we already know that the parent exists.
let mut last = None;
for commit in iter::Walk::new(parent, self.repository) {
let commit = commit
.map_err(error::Read::Commit)?
@ -192,19 +196,26 @@ where
if commits.len_nonzero() == ONE {
return Ok(commit);
} else {
}
let id = commit.id();
if id == commits.last() {
// If this commit is the last element of `commits`,
// then this means it is the earliest of all that share
// its signature. It thus cannot have been replayed.
return Ok(commit);
}
if id == commits.first() {
// We only log one warning per set of duplicates, and that is
// when we reach the first element of `commits`, which is the
// latest in the history.
log::warn!("Duplicate sigrefs found in commits {commits:?}");
last = Some(commit);
}
}
// In the extreme case where all commits in the walk contain duplicate
// signatures, return the oldest commit reached — the last one visited
// by the walk, which follows the chain from newest to oldest.
// `last` is always `Some` here because `parent` is guaranteed to exist
// (head had a duplicate signature, so it must have a parent), meaning
// the walk yields at least one commit.
Ok(last.unwrap_or(head))
unreachable!()
}
fn resolve_tip(&self) -> Result<Oid, error::Read> {

112
crates/radicle/src/storage/refs/sigrefs/read/test/signed_refs_reader.rs
View File

@ -165,77 +165,63 @@ fn two_commits() {
assert_eq!(*vc.id(), head);
}
/// Commit chain:
/// `A <- B <- A`
/// Where A is replayed
/// We test a handful scenarios with replayed commits (or rather, references
/// and signatures within commits).
///
/// Expected Result: B
#[test]
fn head_replays_root() {
const SIGNATURE_A: u8 = 1;
const SIGNATURE_B: u8 = 2;
/// For every test we define:
/// - A history, which is a linear history of commits,
/// where the earliest and leftmost commit is a root commit.
/// - Which commit we expect to be loaded, as a zero based index in the
/// history.
mod replay {
use super::*;
let a1 = mock::oid(1);
let b = mock::oid(2);
let a2 = mock::oid(3);
let repo = mock::setup_chain([
(a1, SIGNATURE_A, refs_without_parent(mock::oid(10))),
(b, SIGNATURE_B, refs_without_parent(mock::oid(10))),
(a2, SIGNATURE_A, refs_without_parent(mock::oid(10))),
]);
/// Mocks a chain of commits, where their OID is their zero-based index
/// in `chain` (note that since this is only mocked, it is not an issue
/// that the first commit in the chain, at index zero, is identified by
/// the zero OID).
///
/// Asserts that the result of [`read`] on the chain is `expected`.
fn replay(chain: impl IntoIterator<Item = u8>, expected: u8) {
let refs = refs_without_parent(mock::oid(10));
let vc = read(a2, repo).unwrap();
assert_eq!(*vc.id(), b);
}
let chain: Vec<_> = chain.into_iter().collect();
let mut repo = MockRepository::new();
let mut parent = None;
for (i, signature) in chain.iter().enumerate() {
let i = mock::oid(i as u8);
repo = repo
.with_commit(i, mock::commit_data(parent))
.with_refs(i, refs.clone())
.with_signature(i, *signature);
parent = Some(i);
}
/// Commit chain:
/// `A <- A <- A`
/// Where A is replayed twice
///
/// Expected Result: first A
#[test]
fn replay_chain() {
const SIGNATURE_A: u8 = 1;
let a1 = mock::oid(1);
let a2 = mock::oid(2);
let a3 = mock::oid(3);
let repo = mock::setup_chain([
(a1, SIGNATURE_A, refs_without_parent(mock::oid(10))),
(a2, SIGNATURE_A, refs_without_parent(mock::oid(10))),
(a3, SIGNATURE_A, refs_without_parent(mock::oid(10))),
]);
assert_eq!(
*read(mock::oid((chain.len() - 1) as u8), repo).unwrap().id(),
mock::oid(expected)
)
}
let vc = read(a3, repo).unwrap();
assert_eq!(*vc.id(), a1);
}
#[test]
fn root_at_head() {
replay([1, 2, 1], 1)
}
/// Commit chain:
/// `C <- C <- B <- A <- A`
/// Where C and A are replayed twice
///
/// Expected Result: first A
#[test]
fn multiple_replays() {
const SIGNATURE_A: u8 = 3;
const SIGNATURE_B: u8 = 2;
const SIGNATURE_C: u8 = 1;
#[test]
fn chain() {
replay([1, 1, 1], 0)
}
let c1 = mock::oid(1);
let c2 = mock::oid(2);
let b = mock::oid(3);
let a1 = mock::oid(4);
let a2 = mock::oid(5);
let r = refs_without_parent(mock::oid(10));
let repo = mock::setup_chain([
(a2, SIGNATURE_C, r.clone()),
(a1, SIGNATURE_C, r.clone()),
(b, SIGNATURE_B, r.clone()),
(c2, SIGNATURE_A, r.clone()),
(c1, SIGNATURE_A, r),
]);
#[test]
fn multiple() {
replay([1, 1, 2, 3, 3], 3)
}
let vc = read(c1, repo).unwrap();
assert_eq!(*vc.id(), b);
#[test]
fn alternating() {
replay([1, 2, 1, 2], 1)
}
}
#[test]