diff --git a/radicle-fetch/src/git/repository.rs b/radicle-fetch/src/git/repository.rs index bdc6f83f..a219a820 100644 --- a/radicle-fetch/src/git/repository.rs +++ b/radicle-fetch/src/git/repository.rs @@ -1,15 +1,36 @@ pub mod error; -use either::{ - Either, - Either::{Left, Right}, -}; +use either::Either; use radicle::git::{Namespaced, Oid, Qualified}; use radicle::storage::git::Repository; -use radicle::storage::ReadRepository; use super::refs::{Applied, Policy, RefUpdate, Update}; +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Ancestry { + Equal, + Ahead, + Behind, + Diverged, +} + +pub enum Updated<'a> { + Accepted(RefUpdate), + Rejected(Update<'a>), +} + +impl<'a> From for Updated<'a> { + fn from(up: RefUpdate) -> Self { + Updated::Accepted(up) + } +} + +impl<'a> From> for Updated<'a> { + fn from(up: Update<'a>) -> Self { + Updated::Rejected(up) + } +} + pub fn contains(repo: &Repository, oid: Oid) -> Result { repo.backend .odb() @@ -17,17 +38,27 @@ pub fn contains(repo: &Repository, oid: Oid) -> Result { .map_err(error::Contains) } -pub fn is_in_ancestry_path(repo: &Repository, old: Oid, new: Oid) -> Result { +pub fn ancestry(repo: &Repository, old: Oid, new: Oid) -> Result { if !contains(repo, old)? || !contains(repo, new)? { - return Ok(false); + return Err(error::Ancestry::Missing { a: old, b: new }); } if old == new { - return Ok(true); + return Ok(Ancestry::Equal); } - repo.is_ancestor_of(old, new) - .map_err(|err| error::Ancestry::Check { old, new, err }) + let (ahead, behind) = repo + .backend + .graph_ahead_behind(new.into(), old.into()) + .map_err(|err| error::Ancestry::Check { old, new, err })?; + + if ahead > 0 && behind == 0 { + Ok(Ancestry::Ahead) + } else if ahead == 0 && behind > 0 { + Ok(Ancestry::Behind) + } else { + Ok(Ancestry::Diverged) + } } pub fn refname_to_id<'a, N>(repo: &Repository, refname: N) -> Result, error::Resolve> @@ -59,12 +90,12 @@ where target, no_ff, } => match direct(repo, name, target, no_ff)? { - Left(r) => applied.rejected.push(r), - Right(u) => applied.updated.push(u), + Updated::Rejected(r) => applied.rejected.push(r), + Updated::Accepted(u) => applied.updated.push(u), }, Update::Prune { name, prev } => match prune(repo, name, prev)? { - Left(r) => applied.rejected.push(r), - Right(u) => applied.updated.push(u), + Updated::Rejected(r) => applied.rejected.push(r), + Updated::Accepted(u) => applied.updated.push(u), }, } } @@ -77,49 +108,62 @@ fn direct<'a>( name: Namespaced<'a>, target: Oid, no_ff: Policy, -) -> Result, RefUpdate>, error::Update> { +) -> Result, error::Update> { let tip = refname_to_id(repo, name.clone())?; match tip { Some(prev) => { - let is_ff = is_in_ancestry_path(repo, prev, target)?; - if !is_ff { - match no_ff { - Policy::Abort => { - return Err(error::Update::NonFF { - name: name.to_owned(), - new: target, - cur: prev, - }) - } - Policy::Reject => Ok(Left(Update::Direct { - name, - target, - no_ff, - })), - Policy::Allow => { - // N.b. the update is a non-fast-forward but - // we allow it, so we pass `force: true`. - repo.backend - .reference(name.as_ref(), target.into(), true, "radicle: update") - .map_err(|err| error::Update::Create { - name: name.to_owned(), - target, - err, - })?; - Ok(Right(RefUpdate::from(name.to_ref_string(), prev, target))) - } + let ancestry = ancestry(repo, prev, target)?; + + match ancestry { + Ancestry::Equal => Ok(RefUpdate::Skipped { + name: name.to_ref_string(), + oid: target, } - } else { - // N.b. the update is a fast-forward so we can safely - // pass `force: true`. - repo.backend - .reference(name.as_ref(), target.into(), true, "radicle: update") - .map_err(|err| error::Update::Create { + .into()), + Ancestry::Ahead => { + // N.b. the update is a fast-forward so we can safely + // pass `force: true`. + repo.backend + .reference(name.as_ref(), target.into(), true, "radicle: update") + .map_err(|err| error::Update::Create { + name: name.to_owned(), + target, + err, + })?; + Ok(RefUpdate::from(name.to_ref_string(), prev, target).into()) + } + Ancestry::Behind | Ancestry::Diverged if matches!(no_ff, Policy::Allow) => { + // N.b. the update is a non-fast-forward but + // we allow it, so we pass `force: true`. + repo.backend + .reference(name.as_ref(), target.into(), true, "radicle: update") + .map_err(|err| error::Update::Create { + name: name.to_owned(), + target, + err, + })?; + Ok(RefUpdate::from(name.to_ref_string(), prev, target).into()) + } + // N.b. if the target is behind, we simply reject the update + Ancestry::Behind => Ok(Update::Direct { + name, + target, + no_ff, + } + .into()), + Ancestry::Diverged if matches!(no_ff, Policy::Reject) => Ok(Update::Direct { + name, + target, + no_ff, + } + .into()), + Ancestry::Diverged => { + return Err(error::Update::NonFF { name: name.to_owned(), - target, - err, - })?; - Ok(Right(RefUpdate::from(name.to_ref_string(), prev, target))) + new: target, + cur: prev, + }) + } } } None => { @@ -132,10 +176,11 @@ fn direct<'a>( target, err, })?; - Ok(Right(RefUpdate::Created { + Ok(RefUpdate::Created { name: name.to_ref_string(), oid: target, - })) + } + .into()) } } } @@ -144,7 +189,7 @@ fn prune<'a>( repo: &Repository, name: Namespaced<'a>, prev: Either>, -) -> Result, RefUpdate>, error::Update> { +) -> Result, error::Update> { use radicle::git::raw::ObjectType; match find(repo, &name)? { @@ -160,12 +205,13 @@ fn prune<'a>( name: name.to_owned(), err, })?; - Ok(Right(RefUpdate::Deleted { + Ok(RefUpdate::Deleted { name: name.to_ref_string(), oid: prev, - })) + } + .into()) } - None => Ok(Left(Update::Prune { name, prev })), + None => Ok(Update::Prune { name, prev }.into()), } } diff --git a/radicle-fetch/src/git/repository/error.rs b/radicle-fetch/src/git/repository/error.rs index effb6ca9..12525ce3 100644 --- a/radicle-fetch/src/git/repository/error.rs +++ b/radicle-fetch/src/git/repository/error.rs @@ -7,6 +7,8 @@ pub struct Contains(#[source] pub raw::Error); #[derive(Debug, Error)] pub enum Ancestry { + #[error("missing one of {a} or {b} while checking ancestry")] + Missing { a: Oid, b: Oid }, #[error(transparent)] Contains(#[from] Contains), #[error("failed to check ancestry for {old} and {new}")] @@ -14,7 +16,7 @@ pub enum Ancestry { old: Oid, new: Oid, #[source] - err: ext::Error, + err: raw::Error, }, } diff --git a/radicle-fetch/src/state.rs b/radicle-fetch/src/state.rs index 874b1c08..90f95a7b 100644 --- a/radicle-fetch/src/state.rs +++ b/radicle-fetch/src/state.rs @@ -29,6 +29,8 @@ pub const DEFAULT_FETCH_DATA_REFS_LIMIT: u64 = 1024 * 1024 * 1024 * 5; pub mod error { use std::io; + use radicle::git::Oid; + use radicle::prelude::PublicKey; use thiserror::Error; use crate::{git, git::repository, handle, sigrefs, stage}; @@ -47,8 +49,16 @@ pub mod error { #[derive(Debug, Error)] pub enum Protocol { + #[error(transparent)] + Ancestry(#[from] repository::error::Ancestry), #[error(transparent)] Canonical(#[from] Canonical), + #[error("delegate '{remote}' has diverged 'rad/sigrefs': {current} -> {received}")] + Diverged { + remote: PublicKey, + current: Oid, + received: Oid, + }, #[error(transparent)] Io(#[from] io::Error), #[error("canonical 'refs/rad/id' is missing")] @@ -371,6 +381,20 @@ impl FetchState { remote, data: Some(sigrefs), } => { + if let Some(SignedRefsAt { at, .. }) = SignedRefsAt::load(remote, &handle.repo)? + { + // Prune non-delegates if they're behind or + // diverged. A diverged case is non-fatal for + // delegates. + if matches!( + repository::ancestry(&handle.repo, at, sigrefs.at)?, + repository::Ancestry::Behind | repository::Ancestry::Diverged + ) { + self.prune(&remote); + continue; + } + } + let cache = self.as_cached(handle); if let Some(warns) = sigrefs::validate(&cache, sigrefs)?.as_mut() { log::debug!( @@ -385,6 +409,21 @@ impl FetchState { remote, data: Some(sigrefs), } => { + if let Some(SignedRefsAt { at, .. }) = SignedRefsAt::load(remote, &handle.repo)? + { + let ancestry = repository::ancestry(&handle.repo, at, sigrefs.at)?; + if matches!(ancestry, repository::Ancestry::Behind) { + self.prune(&remote); + continue; + } else if matches!(ancestry, repository::Ancestry::Diverged) { + return Err(error::Protocol::Diverged { + remote, + current: at, + received: sigrefs.at, + }); + } + } + let cache = self.as_cached(handle); if let Some(fails) = sigrefs::validate(&cache, sigrefs)?.as_mut() { log::warn!(target: "fetch", "Pruning delegate {remote} tips, due to validation failures"); diff --git a/radicle-node/src/tests/e2e.rs b/radicle-node/src/tests/e2e.rs index 6c12f58b..4639d87e 100644 --- a/radicle-node/src/tests/e2e.rs +++ b/radicle-node/src/tests/e2e.rs @@ -855,7 +855,7 @@ fn test_non_fastforward_sigrefs() { // Eve has old refs. assert_matches!( alice.handle.fetch(rid, eve.id, DEFAULT_TIMEOUT).unwrap(), - FetchResult::Failed { .. } + FetchResult::Success { updated, .. } if updated.is_empty() ); } @@ -949,3 +949,93 @@ fn test_outdated_sigrefs() { assert_ne!(eves_refs, old_refs); assert_eq!(eves_refs_expected, eves_refs); } + +#[test] +fn test_outdated_delegate_sigrefs() { + logger::init(log::Level::Debug); + + let tmp = tempfile::tempdir().unwrap(); + + let mut alice = Node::init(tmp.path(), Config::test(Alias::new("alice"))); + let bob = Node::init(tmp.path(), Config::test(Alias::new("bob"))); + let eve = Node::init(tmp.path(), Config::test(Alias::new("eve"))); + + let rid = alice.project("acme", ""); + + let mut alice = alice.spawn(); + let mut bob = bob.spawn(); + let mut eve = eve.spawn(); + + bob.handle.track_repo(rid, Scope::All).unwrap(); + eve.handle.track_repo(rid, Scope::All).unwrap(); + alice.connect(&bob); + bob.connect(&eve); + eve.connect(&alice); + converge([&alice, &bob, &eve]); + + bob.handle.fetch(rid, alice.id, DEFAULT_TIMEOUT).unwrap(); + assert!(bob.storage.contains(&rid).unwrap()); + rad::fork(rid, &bob.signer, &bob.storage).unwrap(); + + eve.handle.fetch(rid, alice.id, DEFAULT_TIMEOUT).unwrap(); + assert!(eve.storage.contains(&rid).unwrap()); + rad::fork(rid, &eve.signer, &eve.storage).unwrap(); + + alice + .handle + .track_node(eve.id, Some(Alias::new("eve"))) + .unwrap(); + alice.handle.fetch(rid, eve.id, DEFAULT_TIMEOUT).unwrap(); + let repo = alice.storage.repository(rid).unwrap(); + assert!(repo.remote(&eve.id).is_ok()); + + log::debug!(target: "test", "Bob fetches from Eve.."); + assert_matches!( + bob.handle.fetch(rid, eve.id, DEFAULT_TIMEOUT).unwrap(), + FetchResult::Success { .. } + ); + let repo = bob.storage.repository(rid).unwrap(); + let alice_remote = repo.remote(&alice.id).unwrap(); + let old_refs = alice_remote.refs; + + // At this stage, Alice and Bob have Eve's fork and Eve does not + // have Bob's fork + + alice.issue( + rid, + "Outdated Sigrefs", + "Outdated sigrefs are harshing my vibes", + ); + let repo = alice.storage.repository(rid).unwrap(); + let alice_refs = repo.remote(&alice.id).unwrap().refs; + + // Get the current state of eve's refs in alice's storage + log::debug!(target: "test", "Alice fetches from Eve.."); + assert_matches!( + eve.handle.fetch(rid, alice.id, DEFAULT_TIMEOUT).unwrap(), + FetchResult::Success { .. } + ); + let repo = eve.storage.repository(rid).unwrap(); + let alice_remote = repo.remote(&alice.id).unwrap(); + let alice_refs_expected = alice_remote.refs; + assert_ne!(alice_refs_expected, old_refs); + assert_eq!(alice_refs_expected, alice_refs); + + log::debug!(target: "test", "Alice fetches from Bob.."); + + eve.handle + .track_node(bob.id, Some(Alias::new("bob"))) + .unwrap(); + assert_matches!( + eve.handle.fetch(rid, bob.id, DEFAULT_TIMEOUT).unwrap(), + FetchResult::Success { .. } + ); + + // Ensure that Eve's refs have not changed after fetching the old refs from Bob. + let repo = eve.storage.repository(rid).unwrap(); + let alice_remote = repo.remote(&alice.id).unwrap(); + let alice_refs = alice_remote.refs; + + assert_ne!(alice_refs, old_refs); + assert_eq!(alice_refs_expected, alice_refs); +}