fetch: allow missing default branch

When fetching, the validation logic would prune a delegate if it was missing the
default branch. Originally, this was done to prevent any broken state, but later
it was realised that this can be too restrictive and would not allow nodes to
recover into an improved state.

Remove this check so thtat nodes can still fetch from each other when the
default branch is missing.

An e2e test is added to show that nodes can still fetch from each other and
still receive other references, even when a delegate is missing their default
branch.
This commit is contained in:
Fintan Halpenny 2024-11-04 15:52:14 +00:00 committed by cloudhead
parent f6aa46a283
commit eb095c109b
No known key found for this signature in database
2 changed files with 119 additions and 27 deletions

View File

@ -8,7 +8,7 @@ use radicle::identity::{Did, Doc, DocError};
use radicle::prelude::Verified;
use radicle::storage;
use radicle::storage::refs::{RefsAt, SignedRefs};
use radicle::storage::refs::RefsAt;
use radicle::storage::{
git::Validation, Remote, RemoteId, RemoteRepository, Remotes, ValidateRepository, Validations,
};
@ -547,15 +547,8 @@ impl FetchState {
}
let cache = self.as_cached(handle);
let mut fails = Validations::default();
// N.b. we only validate the existence of the
// default branch for delegates, since it safe for
// non-delegates to not have this branch.
let branch_validation =
validate_project_default_branch(&anchor, &sigrefs.sigrefs);
fails.extend(branch_validation.into_iter());
let validations = sigrefs::validate(&cache, sigrefs)?;
fails.extend(validations.into_iter().flatten());
let mut fails =
sigrefs::validate(&cache, sigrefs)?.unwrap_or(Validations::default());
if !fails.is_empty() {
log::warn!(target: "fetch", "Pruning delegate {remote} tips, due to validation failures");
self.prune(&remote);
@ -729,20 +722,3 @@ impl<'a, S> ValidateRepository for Cached<'a, S> {
Ok(validations)
}
}
/// If the repository has a project payload, in `anchor`, then
/// validate that the `sigrefs` contains the listed default branch.
///
/// N.b. if the repository does not have the project payload or a
/// deserialization error occurs, then this will return `None`.
fn validate_project_default_branch(
anchor: &Doc,
sigrefs: &SignedRefs<Verified>,
) -> Option<Validation> {
let proj = anchor.project().ok()?;
let branch = radicle::git::refs::branch(proj.default_branch()).to_ref_string();
(!sigrefs.contains_key(&branch)).then_some(Validation::MissingRef {
remote: sigrefs.id,
refname: branch,
})
}

View File

@ -1209,6 +1209,122 @@ fn missing_default_branch() {
);
}
#[test]
fn missing_delegate_default_branch() {
use radicle::git::raw;
use radicle::identity::Identity;
use radicle::storage::git::Repository;
logger::init(log::Level::Debug);
let tmp = tempfile::tempdir().unwrap();
let mut alice = Node::init(tmp.path(), config::relay("alice"));
let bob = Node::init(tmp.path(), config::relay("bob"));
let seed = Node::init(tmp.path(), config::relay("seed"));
let rid = alice.project("acme", "");
let mut alice = alice.spawn();
let mut bob = bob.spawn();
let mut seed = seed.spawn();
alice.handle.seed(rid, Scope::All).unwrap();
bob.handle.seed(rid, Scope::All).unwrap();
seed.handle.seed(rid, Scope::All).unwrap();
alice.connect(&seed);
converge([&seed]);
bob.connect(&seed);
bob.handle.fetch(rid, seed.id, DEFAULT_TIMEOUT).unwrap();
assert!(bob.storage.contains(&rid).unwrap());
// Helper to assert that Bob's default branch is not in storage
let assert_bobs_default_is_missing = |repo: &Repository| {
let doc = repo.identity_doc().unwrap();
let project = doc.project().unwrap();
let default_branch = repo.reference(
bob.signer.public_key(),
&radicle::git::refs::branch(project.default_branch()),
);
assert!(matches!(
default_branch,
Err(radicle::git::Error::Git(e)) if e.code() == raw::ErrorCode::NotFound
));
};
// Add Bob as a delegate to the identity document
{
let repo = alice.storage.repository(rid).unwrap();
let mut identity = Identity::load_mut(&repo).unwrap();
let doc = repo
.identity_doc()
.unwrap()
.doc
.with_edits(|doc| {
doc.delegate(bob.signer.public_key().into());
})
.unwrap();
let rev = identity.update("Add Bob", "", &doc, &alice.signer).unwrap();
repo.set_identity_head_to(rev).unwrap();
let new = repo.identity_doc().unwrap().doc;
assert!(
new.is_delegate(&bob.signer.public_key().into()),
"Bob must be a delegate after the update"
);
}
// We ensure that Bob does not have the default branch
let repo = bob.storage.repository(rid).unwrap();
assert_bobs_default_is_missing(&repo);
// Create an issue to ensure there are new refs to fetch
let issue = bob.issue(
rid,
"Delegate Issue",
"Further investigation into delegates",
);
let assert_bobs_issue_exists = |repo: &Repository| {
let issue_ref = radicle::git::refs::storage::cob(
bob.signer.public_key(),
&radicle::cob::issue::TYPENAME,
&issue,
);
assert!(repo.backend.find_reference(issue_ref.as_str()).is_ok());
};
// The seed fetches from Bob and checks that:
// a) Bob's default branch is still missing
// b) Bob's issue is there
assert_matches!(
seed.handle.fetch(rid, bob.id, DEFAULT_TIMEOUT).unwrap(),
FetchResult::Success { .. }
);
{
let repo = seed.storage.repository(rid).unwrap();
assert_bobs_default_is_missing(&repo);
assert_bobs_issue_exists(&repo);
}
// Do the same for Alice
assert_matches!(
alice.handle.fetch(rid, seed.id, DEFAULT_TIMEOUT).unwrap(),
FetchResult::Success { .. }
);
{
let repo = alice.storage.repository(rid).unwrap();
assert_bobs_default_is_missing(&repo);
assert_bobs_issue_exists(&repo);
}
// Check that Bob can still fetch from the seed
assert_matches!(
bob.handle.fetch(rid, seed.id, DEFAULT_TIMEOUT).unwrap(),
FetchResult::Success { .. }
);
}
#[test]
fn test_background_foreground_fetch() {
logger::init(log::Level::Debug);