From b6f07074d9198bdbf60f49af61ac6416397edb46 Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Thu, 2 Jul 2026 16:37:41 +0000 Subject: [PATCH] radicle/cob/identity: Guard `IdentityMut::accept` against sibling accepts Ensure that the `IdentityMut::accept` API returns an error when it encounters an active, sibling revision by the same delegate. --- crates/radicle-cli/src/commands/id.rs | 2 +- crates/radicle/src/cob/identity.rs | 89 +++++++++++++++++++ .../radicle/src/cob/identity/test/property.rs | 4 + 3 files changed, 94 insertions(+), 1 deletion(-) diff --git a/crates/radicle-cli/src/commands/id.rs b/crates/radicle-cli/src/commands/id.rs index 27fc3f13..5af4fb60 100644 --- a/crates/radicle-cli/src/commands/id.rs +++ b/crates/radicle-cli/src/commands/id.rs @@ -501,7 +501,7 @@ fn on_apply_err(e: &identity::ApplyError, profile: &Profile) -> anyhow::Error { | e @ radicle::cob::identity::ApplyError::MissingParent | e @ radicle::cob::identity::ApplyError::DuplicateVerdict | e @ radicle::cob::identity::ApplyError::UnexpectedState - | e @ radicle::cob::identity::ApplyError::SiblingAccepted + | e @ radicle::cob::identity::ApplyError::SiblingAccepted { .. } | e @ radicle::cob::identity::ApplyError::DocUnchanged | e @ radicle::cob::identity::ApplyError::Git(_) | e @ radicle::cob::identity::ApplyError::Doc(_) diff --git a/crates/radicle/src/cob/identity.rs b/crates/radicle/src/cob/identity.rs index aec69575..9817144d 100644 --- a/crates/radicle/src/cob/identity.rs +++ b/crates/radicle/src/cob/identity.rs @@ -1248,10 +1248,27 @@ where } /// Accept an active revision. + /// + /// # Errors + /// + /// [`SiblingAccepted`]: If the delegate has already accepted an active + /// revision that shares the same parent as the provided `revision`. + /// + /// [`SiblingAccepted`]: ApplyError::SiblingAccepted pub fn accept(&mut self, revision: &RevisionId) -> Result { let id = *revision; let revision = self.revision(revision).ok_or(Error::NotFound(id))?; + if let Some(parent_id) = revision.parent { + #[allow(deprecated)] + let did: Did = self.store.signer().verifying_key().into(); + if let Some(revision_id) = self.has_active_sibling_accept(&parent_id, &did) { + return Err(Error::Apply(ApplyError::SiblingAccepted { + revision: revision_id, + })); + } + } + #[allow(deprecated)] let signature = revision.sign(self.store.signer())?; @@ -3079,4 +3096,76 @@ mod test { State::Active ); } + + /// The `accept()` API rejects an accept if the delegate already + /// accepted an Active sibling. + #[test] + fn accept_rejects_sibling_accept() { + let network = Network::default(); + let alice = &network.alice; + let bob = &network.bob; + let eve = &network.eve; + let dave = &network.dave; + + // 4 delegates, majority = 3. This ensures Alice's accept of child_a + // doesn't immediately adopt it (Bob + Alice = 2 < 3). + let mut alice_identity = Identity::load_mut(&*alice.repo, &alice.signer).unwrap(); + let mut alice_doc = alice_identity.doc().clone().edit(); + alice_doc.delegate(bob.signer.public_key().into()); + alice_doc.delegate(eve.signer.public_key().into()); + alice_doc.delegate(dave.signer.public_key().into()); + let _a0 = alice_identity + .update( + cob::Title::new("Add delegates").unwrap(), + "", + &alice_doc.verified().unwrap(), + ) + .unwrap(); + + // Bob proposes child_a. + bob.repo.fetch(alice); + let mut bob_identity = Identity::load_mut(&*bob.repo, &bob.signer).unwrap(); + let mut bob_doc = bob_identity.doc().clone().edit(); + bob_doc.visibility = Visibility::private([]); + let child_a = bob_identity + .update( + cob::Title::new("Child A").unwrap(), + "", + &bob_doc.verified().unwrap(), + ) + .unwrap(); + + // Eve proposes child_b (sibling of child_a). + eve.repo.fetch(alice); + let mut eve_identity = Identity::load_mut(&*eve.repo, &eve.signer).unwrap(); + let mut eve_doc = eve_identity.doc().clone().edit(); + eve_doc.visibility = Visibility::private([eve.signer.public_key().into()]); + let child_b = eve_identity + .update( + cob::Title::new("Child B").unwrap(), + "", + &eve_doc.verified().unwrap(), + ) + .unwrap(); + + // Alice fetches both and accepts child_a. + // child_a stays Active (Bob + Alice = 2/3, needs 3). + alice.repo.fetch(bob); + alice.repo.fetch(eve); + alice_identity.reload().unwrap(); + alice_identity.accept(&child_a).unwrap(); + assert_eq!( + alice_identity.revision(&child_a).unwrap().state, + State::Active + ); + + // Alice tries to accept child_b — should fail. + let err = alice_identity.accept(&child_b).unwrap_err(); + assert!( + std::iter::successors(Some(&err as &dyn std::error::Error), |e| e.source()) + .filter_map(|e| e.downcast_ref::()) + .any(|e| matches!(e, ApplyError::SiblingAccepted { .. })), + "Expected SiblingAccepted error, got: {err:?}" + ); + } } diff --git a/crates/radicle/src/cob/identity/test/property.rs b/crates/radicle/src/cob/identity/test/property.rs index 15821f36..79222998 100644 --- a/crates/radicle/src/cob/identity/test/property.rs +++ b/crates/radicle/src/cob/identity/test/property.rs @@ -313,6 +313,10 @@ impl Harness { Self::has_apply_error(&e, |err| matches!( err, cob::identity::ApplyError::DuplicateVerdict + // An `actor` for the above `accept` could have + // accepted an already active, sibling revision + // which is disallowed. + | cob::identity::ApplyError::SiblingAccepted { .. } )), "Delegate accept failed with unexpected error: {e:?}" ),