From 5ee1f42b6a04f372147a24e233471d9ddc6e0096 Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Sun, 1 Jan 2023 18:26:40 +0100 Subject: [PATCH] cob: Have clock value for empty COB state By using `0` as the clock value for an empty state (without operations), we simplify transaction code. Previously, there was no clock value for empty states, which meant that we had to sometimes use an `Option`. --- radicle-cob/src/change_graph/evaluation.rs | 4 +-- radicle-cob/src/history.rs | 2 +- radicle-cob/src/history/entry.rs | 6 ++--- radicle/src/cob/issue.rs | 10 +++++--- radicle/src/cob/op.rs | 7 ++++-- radicle/src/cob/patch.rs | 10 ++++---- radicle/src/cob/store.rs | 29 +++++++--------------- 7 files changed, 31 insertions(+), 37 deletions(-) diff --git a/radicle-cob/src/change_graph/evaluation.rs b/radicle-cob/src/change_graph/evaluation.rs index 4b9ed7c8..6b006fd0 100644 --- a/radicle-cob/src/change_graph/evaluation.rs +++ b/radicle-cob/src/change_graph/evaluation.rs @@ -48,8 +48,8 @@ pub fn evaluate(root: Oid, graph: &Dag, rng: fastrand::Rng) -> hist clock + entry.contents().len() as Clock - 1 }) .max() - .map(|n| n + 1) - .unwrap_or_default(); + .unwrap_or_default() // When there are no operations, the clock is zero. + + 1; log::trace!("change '{}' accepted", c.change.id()); entries.insert(*entry.id(), EntryWithClock { entry, clock }); diff --git a/radicle-cob/src/history.rs b/radicle-cob/src/history.rs index 35252fa1..4e534828 100644 --- a/radicle-cob/src/history.rs +++ b/radicle-cob/src/history.rs @@ -58,7 +58,7 @@ impl History { timestamp, }; let mut entries = HashMap::new(); - entries.insert(id, EntryWithClock::from(root_entry)); + entries.insert(id, EntryWithClock::root(root_entry)); create_dag(&id, &entries) } diff --git a/radicle-cob/src/history/entry.rs b/radicle-cob/src/history/entry.rs index 94bb2b10..78cc4ea2 100644 --- a/radicle-cob/src/history/entry.rs +++ b/radicle-cob/src/history/entry.rs @@ -131,11 +131,11 @@ pub struct EntryWithClock { pub clock: Clock, } -impl From for EntryWithClock { - fn from(entry: Entry) -> Self { +impl EntryWithClock { + pub fn root(entry: Entry) -> Self { Self { entry, - clock: Clock::default(), + clock: 1 as Clock, // The root entry has a clock value of `1`. } } } diff --git a/radicle/src/cob/issue.rs b/radicle/src/cob/issue.rs index 9bc673a5..3e565104 100644 --- a/radicle/src/cob/issue.rs +++ b/radicle/src/cob/issue.rs @@ -433,7 +433,7 @@ impl<'a> Issues<'a> { tx.tag(tags.to_owned(), []); })?; // Just a sanity check that our clock is advancing as expected. - debug_assert_eq!(clock.get(), 2); + debug_assert_eq!(clock.get(), 3); Ok(IssueMut { id, @@ -549,6 +549,8 @@ mod test { let created = issues .create("My first issue", "Blah blah blah.", &[], &signer) .unwrap(); + assert_eq!(created.clock().get(), 3); + let (id, created) = (created.id, created.issue); let issue = issues.get(&id).unwrap().unwrap(); @@ -645,7 +647,7 @@ mod test { let mut issue = issues .create("My first issue", "Blah blah blah.", &[], &signer) .unwrap(); - let root = OpId::initial(author); + let root = OpId::root(author); let c1 = issue.comment("Hi hi hi.", root, &signer).unwrap(); let c2 = issue.comment("Ha ha ha.", root, &signer).unwrap(); @@ -706,8 +708,8 @@ mod test { .create("My first issue", "Blah blah blah.", &[], &signer) .unwrap(); - // The initial thread op id is always the same. - let c0 = OpId::initial(author); + // The root thread op id is always the same. + let c0 = OpId::root(author); issue.comment("Ho ho ho.", c0, &signer).unwrap(); issue.comment("Ha ha ha.", c0, &signer).unwrap(); diff --git a/radicle/src/cob/op.rs b/radicle/src/cob/op.rs index 6295c769..67c044da 100644 --- a/radicle/src/cob/op.rs +++ b/radicle/src/cob/op.rs @@ -24,6 +24,10 @@ impl OpId { Self(Lamport::initial(), actor) } + pub fn root(actor: ActorId) -> Self { + Self(Lamport::initial().tick(), actor) + } + /// Get operation id clock. pub fn clock(&self) -> Lamport { self.0 @@ -153,7 +157,7 @@ impl Actor { /// Create a new operation. pub fn op(&mut self, action: A) -> Op { let author = *self.signer.public_key(); - let clock = self.clock; + let clock = self.clock.tick(); let timestamp = clock::Physical::now(); let op = Op { action, @@ -162,7 +166,6 @@ impl Actor { timestamp, }; self.ops.insert((self.clock, author), op.clone()); - self.clock.tick(); op } diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index 7aa42783..f3559eec 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -787,7 +787,7 @@ impl<'a> Patches<'a> { tx.tag(tags.to_owned(), []); })?; // Just a sanity check that our clock is advancing as expected. - debug_assert_eq!(clock.get(), 2); + debug_assert_eq!(clock.get(), 3); Ok(PatchMut::new(id, patch, clock, self)) } @@ -1023,7 +1023,7 @@ mod test { ) .unwrap(); - assert_eq!(patch.clock, clock::Lamport::from(2)); + assert_eq!(patch.clock.get(), 3); let id = patch.id; let patch = patches.get(&id).unwrap().unwrap(); @@ -1238,15 +1238,15 @@ mod test { ) .unwrap(); - assert_eq!(patch.clock.get(), 2); + assert_eq!(patch.clock.get(), 3); assert_eq!(patch.description(), Some("Blah blah blah.")); assert_eq!(patch.version(), 0); let (r1, t1) = patch .update("I've made changes.", base, rev1_oid, &signer) .unwrap(); - assert_eq!(r1.clock().get(), 3); - assert_eq!(t1.clock().get(), 4); + assert_eq!(r1.clock().get(), 4); + assert_eq!(t1.clock().get(), 5); let id = patch.id; let patch = patches.get(&id).unwrap().unwrap(); diff --git a/radicle/src/cob/store.rs b/radicle/src/cob/store.rs index 02192421..cc1df74f 100644 --- a/radicle/src/cob/store.rs +++ b/radicle/src/cob/store.rs @@ -194,17 +194,19 @@ where pub struct Transaction { actor: ActorId, start: Lamport, - clock: Option, + clock: Lamport, actions: Vec, } impl Transaction { /// Create a new transaction. pub fn new(actor: ActorId, clock: Lamport) -> Self { + let start = clock; + Self { actor, - start: clock, - clock: Some(clock), + start, + clock, actions: Vec::new(), } } @@ -225,7 +227,7 @@ impl Transaction { let mut tx = Transaction { actor, start: Lamport::initial(), - clock: None, + clock: Lamport::initial(), actions: Vec::new(), }; operations(&mut tx); @@ -235,7 +237,7 @@ impl Transaction { let (id, cob, clock) = store.create(message, actions, signer)?; // The history clock should be in sync with the tx clock. - assert_eq!(Some(clock), tx.clock); + assert_eq!(clock, tx.clock); Ok((id, cob, clock)) } @@ -243,20 +245,7 @@ impl Transaction { /// Add an operation to this transaction. pub fn push(&mut self, action: T::Action) -> cob::OpId { self.actions.push(action); - - // If our clock already had a value, it means this isn't the first operation - // of this COB. In that case we 'tick' the clock and return the new clock - // value. - // - // Otherwise, it means it was the first operation of our COB. In that case - // we set our clock to the initial clock value (0), and return that. - if let Some(ref mut clock) = self.clock { - OpId::new(clock.tick(), self.actor) - } else { - self.clock = Some(Lamport::initial()); - - OpId::initial(self.actor) - } + OpId::new(self.clock.tick(), self.actor) } /// Commit transaction. @@ -279,7 +268,7 @@ impl Transaction { let timestamp = cob.history().timestamp().into(); // The history clock should be in sync with the tx clock. - assert_eq!(Some(cob.history().clock()), self.clock.map(|c| c.get())); + assert_eq!(cob.history().clock(), self.clock.get()); // Start the clock from where the transcation clock started. let mut clock = self.start;