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.
This commit is contained in:
parent
8e677a9d90
commit
82dc3fe20c
|
|
@ -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
|
||||
```
|
||||
|
|
|
|||
|
|
@ -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)"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue