From 82dc3fe20c5328b606031ed186b9fea2dad81ddc Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Thu, 2 Jul 2026 16:08:45 +0000 Subject: [PATCH] radicle/cob/identity: Enforce sibling-accept invariant for `RevisionAccept` in `action` On `Action::RevisionAccept` use `has_active_sibling_accept` to check for a sibling accept. If one is found, the `RevisionAccept` is logged and skipped. This preserves the invariant of a single accept per delegate on active, sibling revisions without throwing an error on existing histories. --- .../radicle-cli/examples/rad-id-conflict.md | 46 ++------- crates/radicle/src/cob/identity.rs | 99 ++++++++++++++++++- 2 files changed, 101 insertions(+), 44 deletions(-) diff --git a/crates/radicle-cli/examples/rad-id-conflict.md b/crates/radicle-cli/examples/rad-id-conflict.md index b43fe591..94c06057 100644 --- a/crates/radicle-cli/examples/rad-id-conflict.md +++ b/crates/radicle-cli/examples/rad-id-conflict.md @@ -43,24 +43,24 @@ $ rad id list ╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ ``` -This isn't a problem as long as we don't try to accept both. So let's accept -Bob's: +This isn't a problem as long as we don't try to accept both. Alice decides to +go with Bob's proposal, so she redacts her own first and then accepts his: ``` ~alice +$ rad id redact 12d7300 -q $ rad id accept 89b2623 -q $ rad id list ╭────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ │ ● ID Title Author Status Created Parent │ ├────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤ │ ● 89b2623 Edit project name bob z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk accepted now 0ca42d3 │ -│ ● 12d7300 Edit project name alice (you) rejected now 0ca42d3 │ │ ● 0ca42d3 Add Bob alice (you) accepted now 0656c21 │ │ ● 0656c21 Initial revision alice (you) accepted now none │ ╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ ``` -Doing so voided the other conflicting revision, and it can no longer be -accepted now. +Alice's revision was redacted and is no longer visible. Bob syncs and sees +that the conflict is resolved: ``` ~bob $ rad sync --fetch rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji @@ -69,40 +69,6 @@ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from the network, found 1 potential s 🌱 Fetched from z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi ``` ``` ~bob (fail) -$ rad id accept 12d7300 -q -✗ Error: cannot vote on revision that is rejected -$ rad id reject 12d7300 -q -✗ Error: cannot vote on revision that is rejected -``` -``` ~bob $ rad id show 12d7300 -╭────────────────────────────────────────────────────────────────────────╮ -│ Title Edit project name │ -│ Revision 12d7300d1bbba84e4e5760c8c61999bf5fefb81a │ -│ Parent 0ca42d376bd566631083c8913cf86bec722da392 │ -│ Blob e93aa3e3c5c448bacd3537a81daf1437eccd046a │ -│ Author did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi │ -│ State rejected │ -│ Quorum no │ -├────────────────────────────────────────────────────────────────────────┤ -│ ✓ did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi alice │ -│ ? did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk bob (you) │ -╰────────────────────────────────────────────────────────────────────────╯ - -@@ -1,14 +1,14 @@ - { - "payload": { - "xyz.radicle.project": { - "defaultBranch": "master", - "description": "Radicle Heartwood Protocol & Stack", -- "name": "heartwood" -+ "name": "heart" - } - }, - "delegates": [ - "did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi", - "did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk" - ], - "threshold": 2 - } +✗ Error: revision `12d7300d1bbba84e4e5760c8c61999bf5fefb81a` not found ``` diff --git a/crates/radicle/src/cob/identity.rs b/crates/radicle/src/cob/identity.rs index 43b5090c..0e290ba9 100644 --- a/crates/radicle/src/cob/identity.rs +++ b/crates/radicle/src/cob/identity.rs @@ -522,8 +522,10 @@ impl Identity { ); } State::Active => { - let parent = revision.parent.ok_or(ApplyError::MissingParent)?; - let parent = self.revision(&parent).ok_or(ApplyError::Missing(parent))?; + let parent_id = revision.parent.ok_or(ApplyError::MissingParent)?; + let parent = self + .revision(&parent_id) + .ok_or(ApplyError::Missing(parent_id))?; if !parent.is_delegate(&did) { return Err(ApplyError::non_delegate_unauthorized(did, &action)); @@ -533,6 +535,21 @@ impl Identity { match action { Action::RevisionAccept { signature, .. } => { + // A delegate may only accept one Active child + // of a given parent. If they already accepted a + // sibling, silently skip this vote. + // This handles old histories where the + // invariant was not enforced. + if let Some(revision_id) = + self.has_active_sibling_accept(&parent_id, &did) + { + log::debug!( + "Skipping accept of {id} by {did}: \ + already accepted an active revision '{revision_id}'.", + ); + return Ok(()); + } + parent .verify_signature(&author, &signature, revision.blob) .map_err(|_source| { @@ -1486,14 +1503,15 @@ mod test { assert_eq!(bob_identity.revision(&a2).unwrap().state, State::Active); assert_eq!(bob_identity.revision(&b1).unwrap().state, State::Active); - // Now Bob accepts Alice's proposal. This voids his own. + // Bob must redact his revision, before he accepts Alice's proposal. + bob_identity.redact(b1).unwrap(); bob_identity.accept(&a2).unwrap(); assert_eq!(bob_identity.current, a2); assert_eq!(bob_identity.revision(&a1).unwrap().state, State::Accepted); assert_eq!(bob_identity.revision(&a2).unwrap().state, State::Accepted); assert_eq!( bob_identity.revision(&b1).unwrap().state, - State::Rejected(RejectedBy::Sibling(a2)) + State::Redacted(RedactedBy::Author) ); } @@ -2895,4 +2913,77 @@ mod test { None ); } + + /// Old histories may contain a delegate explicitly accepting two + /// siblings. The evaluation layer must handle this gracefully by + /// silently skipping the second accept — the vote must not be recorded. + #[test] + fn evaluation_skips_sibling_accept_in_old_history() { + let network = Network::default(); + let alice = &network.alice; + let bob = &network.bob; + + 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()); + let _a1 = alice_identity + .update( + cob::Title::new("Add Bob").unwrap(), + "", + &alice_doc.verified().unwrap(), + ) + .unwrap(); + + // Bob proposes child_a (gets implicit accept). + 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(); + + // Alice proposes child_b (sibling, gets implicit accept). + let mut alice_doc2 = alice_identity.doc().clone().edit(); + alice_doc2.visibility = Visibility::private([alice.signer.public_key().into()]); + let _child_b = alice_identity + .update( + cob::Title::new("Child B").unwrap(), + "", + &alice_doc2.verified().unwrap(), + ) + .unwrap(); + + // Alice fetches Bob's child_a and explicitly accepts it + // (simulating old history — she already has an accept on child_b). + alice.repo.fetch(bob); + alice_identity.reload().unwrap(); + + // Use transaction to bypass the API guard (simulating old history). + let sig = alice_identity + .revision(&child_a) + .unwrap() + .sign(&alice.signer) + .unwrap(); + alice_identity + .transaction("Accept child_a", |tx, _| tx.accept(child_a, sig)) + .unwrap(); + + let alice_did: Did = alice.signer.public_key().into(); + + // Alice's accept on child_a should be skipped because she already + // has an accept on the Active sibling child_b. + assert!( + !alice_identity + .revision(&child_a) + .unwrap() + .accepted() + .any(|did| did == alice_did), + "Alice's accept on child_a should have been skipped (she already accepted sibling child_b)" + ); + } }