From aacacf872fceb6b051a940341a4ce71de10be0e8 Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Thu, 9 Mar 2023 11:20:34 +0100 Subject: [PATCH] cob: Use `EntryId` as the operation identifier By introducing a small limitation: only allowing entries in the change graph to be addressable, instead of individual operations; we drastically simplify the CRDT implementation. There are four advantages: 1. Op ids are just regular SHA-1s 2. There's no need for relative IDs, ops never refer to other ops within the same commit 3. There's no need for a nonce, since commits can't collide, and neither can op IDs 4. `OpId` can just be an alias of `EntryId` The disadvantage of course, is that we have to be mindful of how we create op transactions, since each transaction creates an addressable unit. For example, we must not include multiple patch revisions in the same transaction. --- Cargo.lock | 1 - radicle-cli/examples/rad-id-rebase.md | 32 ++--- radicle-cli/examples/rad-id.md | 20 +-- radicle-cli/examples/rad-issue.md | 16 +-- radicle-cli/examples/rad-patch.md | 26 ++-- radicle-cob/src/backend/git/change.rs | 19 +-- radicle-cob/src/change_graph/evaluation.rs | 7 +- radicle-cob/src/history.rs | 2 +- radicle-cob/src/history/entry.rs | 53 ++++---- radicle-cob/src/lib.rs | 2 +- radicle-cob/src/object.rs | 1 + radicle-cob/src/object/collaboration.rs | 2 +- .../src/object/collaboration/update.rs | 16 ++- radicle-cob/src/tests.rs | 21 +-- radicle-httpd/src/api/error.rs | 11 +- radicle-httpd/src/api/json.rs | 4 +- radicle-httpd/src/api/v1/projects.rs | 22 ++- radicle-httpd/src/test.rs | 89 ++++++------ radicle/Cargo.toml | 1 - radicle/src/cob.rs | 6 +- radicle/src/cob/identity.rs | 73 +++++----- radicle/src/cob/issue.rs | 63 +++++---- radicle/src/cob/op.rs | 128 ++---------------- radicle/src/cob/patch.rs | 71 +++++----- radicle/src/cob/store.rs | 110 ++++----------- radicle/src/cob/test.rs | 20 +-- radicle/src/cob/thread.rs | 21 +-- 27 files changed, 343 insertions(+), 494 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 76755f7c..fbfdfb9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1807,7 +1807,6 @@ dependencies = [ "radicle-crypto", "radicle-git-ext", "radicle-ssh", - "rand 0.8.5", "serde", "serde_json", "siphasher", diff --git a/radicle-cli/examples/rad-id-rebase.md b/radicle-cli/examples/rad-id-rebase.md index 4079b1c9..bb0f7684 100644 --- a/radicle-cli/examples/rad-id-rebase.md +++ b/radicle-cli/examples/rad-id-rebase.md @@ -6,7 +6,7 @@ delegates creating proposals concurrently. ``` $ rad id edit --title "Add Alice" --description "Add Alice as a delegate" --delegates did:key:z6MkedTZGJGqgQ2py2b8kGecfxdt2yRdHWF6JpaZC47fovFn --no-confirm -✓ Identity proposal '6e4dfd0edbb0974f5a4fb0990133e0911b0992c0' created 🌱 +✓ Identity proposal '04603c0d3ea4d137487024a51c9360adfc511114' created 🌱 title: Add Alice description: Add Alice as a delegate status: ❲open❳ @@ -48,7 +48,7 @@ Quorum Reached ``` $ rad id edit --title "Add Bob" --description "Add Bob as a delegate" --delegates did:key:z6MkjchhfUsD6mmvni8mCdXHw216Xrm9bQe2mBH1P5RDjVJG --no-confirm -✓ Identity proposal '6a687267b5063c036fdde90b75cc5eb153fa25e8' created 🌱 +✓ Identity proposal '3f6ae4f8645c8b0cbcd35ea924df7b13aca52774' created 🌱 title: Add Bob description: Add Bob as a delegate status: ❲open❳ @@ -93,7 +93,7 @@ second proposal, then the identity would be out of date. So let's run through that and see what happens. ``` -$ rad id accept 6e4dfd0edbb0974f5a4fb0990133e0911b0992c0 --no-confirm +$ rad id accept 04603c0d3ea4d137487024a51c9360adfc511114 --no-confirm ✓ Accepted proposal ✓ title: Add Alice description: Add Alice as a delegate @@ -137,7 +137,7 @@ Quorum Reached ``` ``` -$ rad id commit 6e4dfd0edbb0974f5a4fb0990133e0911b0992c0 --no-confirm +$ rad id commit 04603c0d3ea4d137487024a51c9360adfc511114 --no-confirm ✓ Committed new identity '29ae4b72f5a315328f06fbd68dc1c396a2d5c45e' 🌱 title: Add Alice description: Add Alice as a delegate @@ -183,7 +183,7 @@ Quorum Reached Now, when we go to accept the second proposal: ``` -$ rad id accept 6a687267b5063c036fdde90b75cc5eb153fa25e8 --no-confirm +$ rad id accept 3f6ae4f8645c8b0cbcd35ea924df7b13aca52774 --no-confirm ! Warning: Revision is out of date ! Warning: d96f425412c9f8ad5d9a9a05c9831d0728e2338d =/= 475cdfbc8662853dd132ec564e4f5eb0f152dd7f 👉 Consider using 'rad id rebase' to update the proposal to the latest identity @@ -238,19 +238,19 @@ Note that a warning was emitted: If we attempt to commit this revision, the command will fail: ``` -$ rad id commit 6a687267b5063c036fdde90b75cc5eb153fa25e8 --no-confirm +$ rad id commit 3f6ae4f8645c8b0cbcd35ea924df7b13aca52774 --no-confirm ! Warning: Revision is out of date ! Warning: d96f425412c9f8ad5d9a9a05c9831d0728e2338d =/= 475cdfbc8662853dd132ec564e4f5eb0f152dd7f 👉 Consider using 'rad id rebase' to update the proposal to the latest identity -✗ Id failed: the identity hashes do match 'd96f425412c9f8ad5d9a9a05c9831d0728e2338d =/= 475cdfbc8662853dd132ec564e4f5eb0f152dd7f' for the revision '6877dc63e001e7f7fcb285f5f530948b3d96b488' +✗ Id failed: the identity hashes do match 'd96f425412c9f8ad5d9a9a05c9831d0728e2338d =/= 475cdfbc8662853dd132ec564e4f5eb0f152dd7f' for the revision '3f6ae4f8645c8b0cbcd35ea924df7b13aca52774' ``` So, let's fix this by running a rebase on the proposal's revision: ``` -$ rad id rebase 6a687267b5063c036fdde90b75cc5eb153fa25e8 --no-confirm -✓ Identity proposal '6a687267b5063c036fdde90b75cc5eb153fa25e8' rebased 🌱 -✓ Revision 'aaa890c3531f880c9901b162ab38016ceb559c9f' +$ rad id rebase 3f6ae4f8645c8b0cbcd35ea924df7b13aca52774 --no-confirm +✓ Identity proposal '3f6ae4f8645c8b0cbcd35ea924df7b13aca52774' rebased 🌱 +✓ Revision '42b9428df59ad349f706b1397750b75ea3b42574' title: Add Bob description: Add Bob as a delegate status: ❲open❳ @@ -293,9 +293,9 @@ Quorum Reached We can now update the proposal to have both keys in the delegates set: ``` -$ rad id update 6a687267b5063c036fdde90b75cc5eb153fa25e8 --rev aaa890c3531f880c9901b162ab38016ceb559c9f --delegates did:key:z6MkedTZGJGqgQ2py2b8kGecfxdt2yRdHWF6JpaZC47fovFn --no-confirm -✓ Identity proposal '6a687267b5063c036fdde90b75cc5eb153fa25e8' updated 🌱 -✓ Revision '24ad4a6ce84b1ce4b8cc754494c23f1079020a14' +$ rad id update 3f6ae4f8645c8b0cbcd35ea924df7b13aca52774 --rev 42b9428df59ad349f706b1397750b75ea3b42574 --delegates did:key:z6MkedTZGJGqgQ2py2b8kGecfxdt2yRdHWF6JpaZC47fovFn --no-confirm +✓ Identity proposal '3f6ae4f8645c8b0cbcd35ea924df7b13aca52774' updated 🌱 +✓ Revision '1b4ded759249e4f76d19c3e580b4736bf2a2d1c4' title: Add Bob description: Add Bob as a delegate status: ❲open❳ @@ -338,10 +338,10 @@ Quorum Reached Finally, we can accept and commit this proposal, creating the final state of our new Radicle identity: -$ rad id show 6a687267b5063c036fdde90b75cc5eb153fa25e8 --revisions +$ rad id show 3f6ae4f8645c8b0cbcd35ea924df7b13aca52774 --revisions ``` -$ rad id accept 6a687267b5063c036fdde90b75cc5eb153fa25e8 --rev 24ad4a6ce84b1ce4b8cc754494c23f1079020a14 --no-confirm +$ rad id accept 3f6ae4f8645c8b0cbcd35ea924df7b13aca52774 --rev 1b4ded759249e4f76d19c3e580b4736bf2a2d1c4 --no-confirm ✓ Accepted proposal ✓ title: Add Bob description: Add Bob as a delegate @@ -385,7 +385,7 @@ Quorum Reached ``` ``` -$ rad id commit 6a687267b5063c036fdde90b75cc5eb153fa25e8 --rev 24ad4a6ce84b1ce4b8cc754494c23f1079020a14 --no-confirm +$ rad id commit 3f6ae4f8645c8b0cbcd35ea924df7b13aca52774 --rev 1b4ded759249e4f76d19c3e580b4736bf2a2d1c4 --no-confirm ✓ Committed new identity '60de897bc24898f6908fd1272633c0b15aa4096f' 🌱 title: Add Bob description: Add Bob as a delegate diff --git a/radicle-cli/examples/rad-id.md b/radicle-cli/examples/rad-id.md index c3d979cf..6bf8e848 100644 --- a/radicle-cli/examples/rad-id.md +++ b/radicle-cli/examples/rad-id.md @@ -14,7 +14,7 @@ Let's add Bob as a delegate using their DID ``` $ rad id edit --title "Add Bob" --description "Add Bob as a delegate" --delegates did:key:z6MkedTZGJGqgQ2py2b8kGecfxdt2yRdHWF6JpaZC47fovFn --no-confirm -✓ Identity proposal 'd4ceabea6b7acc91a92c040274e4578d9158a24b' created 🌱 +✓ Identity proposal '0d396a83a5e1dda2b8929f7dc401d19dd1a79fb8' created 🌱 title: Add Bob description: Add Bob as a delegate status: ❲open❳ @@ -89,7 +89,7 @@ Finally, we can see whether the `Quorum` was reached: Let's see what happens when we reject the change: ``` -$ rad id reject d4ceabea6b7acc91a92c040274e4578d9158a24b --no-confirm +$ rad id reject 0d396a83a5e1dda2b8929f7dc401d19dd1a79fb8 --no-confirm ✓ Rejected proposal 👎 title: Add Bob description: Add Bob as a delegate @@ -145,7 +145,7 @@ increased to `1`. Instead, let's accept the proposal: ``` -$ rad id accept d4ceabea6b7acc91a92c040274e4578d9158a24b --no-confirm +$ rad id accept 0d396a83a5e1dda2b8929f7dc401d19dd1a79fb8 --no-confirm ✓ Accepted proposal ✓ title: Add Bob description: Add Bob as a delegate @@ -207,7 +207,7 @@ As well as that, the `Quorum` has now been reached: At this point, we can commit the proposal and update the identity: ``` -$ rad id commit d4ceabea6b7acc91a92c040274e4578d9158a24b --no-confirm +$ rad id commit 0d396a83a5e1dda2b8929f7dc401d19dd1a79fb8 --no-confirm ✓ Committed new identity 'c96e764965aaeff1c6ea3e5b97e2b9828773c8b0' 🌱 title: Add Bob description: Add Bob as a delegate @@ -255,7 +255,7 @@ the `--threshold` option: ``` $ rad id edit --title "Update threshold" --description "Update to safer threshold" --threshold 2 --no-confirm -✓ Identity proposal 'b54d8c5fbe42236c9210f39c4051cd223a884b7c' created 🌱 +✓ Identity proposal 'f435d6e89c8f922ede691287c0d8b7f82afa591e' created 🌱 title: Update threshold description: Update to safer threshold status: ❲open❳ @@ -298,8 +298,8 @@ Quorum Reached But we change our minds and decide to close the proposal instead: ``` -$ rad id close b54d8c5fbe42236c9210f39c4051cd223a884b7c --no-confirm -✓ Closed identity proposal 'b54d8c5fbe42236c9210f39c4051cd223a884b7c' +$ rad id close f435d6e89c8f922ede691287c0d8b7f82afa591e --no-confirm +✓ Closed identity proposal 'f435d6e89c8f922ede691287c0d8b7f82afa591e' title: Update threshold description: Update to safer threshold status: ❲closed❳ @@ -348,15 +348,15 @@ Radicle identity, then we can use the list command: ``` $ rad id list -b54d8c5fbe42236c9210f39c4051cd223a884b7c "Update threshold" ❲closed❳ -d4ceabea6b7acc91a92c040274e4578d9158a24b "Add Bob" ❲committed❳ +0d396a83a5e1dda2b8929f7dc401d19dd1a79fb8 "Add Bob" ❲committed❳ +f435d6e89c8f922ede691287c0d8b7f82afa591e "Update threshold" ❲closed❳ ``` And if we want to view the latest state of any proposal we can use the show command: ``` -$ rad id show b54d8c5fbe42236c9210f39c4051cd223a884b7c +$ rad id show f435d6e89c8f922ede691287c0d8b7f82afa591e title: Update threshold description: Update to safer threshold status: ❲closed❳ diff --git a/radicle-cli/examples/rad-issue.md b/radicle-cli/examples/rad-issue.md index 206b1dfb..3bafa698 100644 --- a/radicle-cli/examples/rad-issue.md +++ b/radicle-cli/examples/rad-issue.md @@ -11,7 +11,7 @@ The issue is now listed under our project. ``` $ rad issue list -e8eb9ca4afa050499b259842ddef2d41abf0fd83 "flux capacitor underpowered" +2e8c1bf3fe0532a314778357c886608a966a34bd "flux capacitor underpowered" ``` Great! Now we've documented the issue for ourselves and others. @@ -22,20 +22,20 @@ others to work on. This is to ensure work is not duplicated. Let's assign ourselves to this one. ``` -$ rad assign e8eb9ca4afa050499b259842ddef2d41abf0fd83 did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi +$ rad assign 2e8c1bf3fe0532a314778357c886608a966a34bd did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi ``` It will now show in the list of issues assigned to us. ``` $ rad issue list --assigned -e8eb9ca4afa050499b259842ddef2d41abf0fd83 "flux capacitor underpowered" did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi +2e8c1bf3fe0532a314778357c886608a966a34bd "flux capacitor underpowered" did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi ``` Note: this can always be undone with the `unassign` subcommand. ``` -$ rad unassign e8eb9ca4afa050499b259842ddef2d41abf0fd83 did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi +$ rad unassign 2e8c1bf3fe0532a314778357c886608a966a34bd did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi ``` Great, now we have communicated to the world about our car's defect. @@ -44,8 +44,8 @@ But wait! We've found an important detail about the car's power requirements. It will help whoever works on a fix. ``` -$ rad comment e8eb9ca4afa050499b259842ddef2d41abf0fd83 --message 'The flux capacitor needs 1.21 Gigawatts' -f1895792f7b1b56590aa21e34454bde74d04649a -$ rad comment e8eb9ca4afa050499b259842ddef2d41abf0fd83 --reply-to f1895792f7b1b56590aa21e34454bde74d04649a --message 'More power!' -0bf5f874c57ac0a5cc010a9895dd0fec9edc4f3d +$ rad comment 2e8c1bf3fe0532a314778357c886608a966a34bd --message 'The flux capacitor needs 1.21 Gigawatts' +9822748bd076595a2408aad02b3a0d9f94fec7e0 +$ rad comment 2e8c1bf3fe0532a314778357c886608a966a34bd --reply-to 9822748bd076595a2408aad02b3a0d9f94fec7e0 --message 'More power!' +edec8d07bf3788b98943394c1274910b8f12d35c ``` diff --git a/radicle-cli/examples/rad-patch.md b/radicle-cli/examples/rad-patch.md index f92ae7cb..4a9432c6 100644 --- a/radicle-cli/examples/rad-patch.md +++ b/radicle-cli/examples/rad-patch.md @@ -35,7 +35,7 @@ z6MknSL…StBU8Vi/master (f2de534) <- z6MknSL…StBU8Vi/flux-capacitor-power (3e 3e674d1 Define power requirements -✓ Patch d8584d098142d774211ac5cdc8d1df4a113875dd created 🌱 +✓ Patch 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 created 🌱 To publish your patch to the network, run: rad push @@ -49,15 +49,15 @@ $ rad patch ❲YOU PROPOSED❳ -Define power requirements d8584d09814 R0 3e674d1 (flux-capacitor-power) ahead 1, behind 0 +Define power requirements 191a14e520f R0 3e674d1 (flux-capacitor-power) ahead 1, behind 0 └─ * opened by did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (you) [..] -└─ * patch id d8584d098142d774211ac5cdc8d1df4a113875dd +└─ * patch id 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 ❲OTHERS PROPOSED❳ Nothing to show. -$ rad patch show d8584d098142d774211ac5cdc8d1df4a113875dd +$ rad patch show 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 Define power requirements @@ -84,34 +84,34 @@ $ git commit --message "Add README, just for the fun" [flux-capacitor-power 27857ec] Add README, just for the fun 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 README.md -$ rad patch update --message "Add README, just for the fun" d8584d098142d774211ac5cdc8d1df4a113875dd +$ rad patch update --message "Add README, just for the fun" 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 🌱 Updating patch for heartwood ✓ Pushing HEAD to storage... ✓ Analyzing remotes... -d8584d09814 R0 (3e674d1) -> R1 (27857ec) +191a14e520f R0 (3e674d1) -> R1 (27857ec) 1 commit(s) ahead, 0 commit(s) behind -✓ Patch d8584d098142d774211ac5cdc8d1df4a113875dd updated 🌱 +✓ Patch 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 updated 🌱 ``` And lets leave a quick comment for our team: ``` -$ rad comment d8584d098142d774211ac5cdc8d1df4a113875dd --message 'I cannot wait to get back to the 90s!' -84ef44764de73695cf30e6b284585d2c50d6d0e5 -$ rad comment d8584d098142d774211ac5cdc8d1df4a113875dd --message 'I cannot wait to get back to the 90s!' --reply-to 84ef44764de73695cf30e6b284585d2c50d6d0e5 -2fa3ac18d82ebdafe73484a15fa9823355c4664b +$ rad comment 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 --message 'I cannot wait to get back to the 90s!' +70fc8b18300096f6f0f919797457244e6e4b2cea +$ rad comment 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 --message 'I cannot wait to get back to the 90s!' --reply-to 70fc8b18300096f6f0f919797457244e6e4b2cea +7a9f7a6358238f4ff115d2b2a5e522ab93867d38 ``` Now, let's checkout the patch that we just created: ``` -$ rad patch checkout d8584d098142d774211ac5cdc8d1df4a113875dd +$ rad patch checkout 191a14e520f2eeff7c0e3ee0a5523c5217eecb89 ✓ Performing patch checkout... -✓ Switched to branch patch/d8584d09814 +✓ Switched to branch patch/191a14e520f ``` diff --git a/radicle-cob/src/backend/git/change.rs b/radicle-cob/src/backend/git/change.rs index 37b7addb..fca190d8 100644 --- a/radicle-cob/src/backend/git/change.rs +++ b/radicle-cob/src/backend/git/change.rs @@ -110,17 +110,8 @@ impl change::Storage for git2::Repository { history_type, }; - let (revision, blob_oids) = write_manifest(self, &manifest, &contents)?; + let revision = write_manifest(self, &manifest, &contents)?; let tree = self.find_tree(revision)?; - let contents = NonEmpty::collect(blob_oids.into_iter().zip(contents).map(|(oid, op)| { - entry::EntryBlob { - oid: oid.into(), - data: op, - } - })) - // SAFETY: We know it's not empty because the original `contents` is `NonEmpty`. - .unwrap(); - let signature = { let sig = signer.sign(revision.as_bytes()); let key = signer.public_key(); @@ -216,7 +207,7 @@ fn load_contents( let blob = entry .to_object(repo) .and_then(|object| object.peel_to_blob()) - .map(entry::EntryBlob::from) + .map(|blob| blob.content().to_owned()) .map(|b| (name, b)); Some(blob) @@ -291,7 +282,7 @@ fn write_manifest( repo: &git2::Repository, manifest: &store::Manifest, contents: &NonEmpty>, -) -> Result<(git2::Oid, Vec), git2::Error> { +) -> Result { let mut tb = repo.treebuilder(None)?; // SAFETY: we're serializing to an in memory buffer so the only source of // errors here is a programming error, which we can't recover from @@ -303,13 +294,11 @@ fn write_manifest( git2::FileMode::Blob.into(), )?; - let mut blob_oids = Vec::new(); for (ix, op) in contents.iter().enumerate() { let oid = repo.blob(op.as_ref())?; tb.insert(&ix.to_string(), oid, git2::FileMode::Blob.into())?; - blob_oids.push(oid); } let tree_oid = tb.write()?; - Ok((tree_oid, blob_oids)) + Ok(tree_oid) } diff --git a/radicle-cob/src/change_graph/evaluation.rs b/radicle-cob/src/change_graph/evaluation.rs index e494178a..67b4aa36 100644 --- a/radicle-cob/src/change_graph/evaluation.rs +++ b/radicle-cob/src/change_graph/evaluation.rs @@ -40,12 +40,7 @@ pub fn evaluate(root: Oid, graph: &Dag, rng: fastrand::Rng) -> hist let clock = graph[&c.oid] .dependencies .iter() - .map(|e| { - let entry = &entries[&EntryId::from(*e)]; - let clock = entry.range(); - - *clock.end() - }) + .map(|e| entries[&EntryId::from(*e)].clock()) .max() .unwrap_or_default() // When there are no operations, the clock is zero. + 1; diff --git a/radicle-cob/src/history.rs b/radicle-cob/src/history.rs index d2e6142f..455c6dad 100644 --- a/radicle-cob/src/history.rs +++ b/radicle-cob/src/history.rs @@ -80,7 +80,7 @@ impl History { pub fn clock(&self) -> Clock { self.graph .tips() - .map(|(_, node)| node.clock + node.entry.contents.len() as Clock - 1) + .map(|(_, node)| node.clock) .max() .unwrap_or_default() } diff --git a/radicle-cob/src/history/entry.rs b/radicle-cob/src/history/entry.rs index 7fc597da..d761627f 100644 --- a/radicle-cob/src/history/entry.rs +++ b/radicle-cob/src/history/entry.rs @@ -2,34 +2,19 @@ // // This file is part of radicle-link, distributed under the GPLv3 with Radicle // Linking Exception. For full terms see the included LICENSE file. +use std::fmt; +use std::str::FromStr; use git_ext::Oid; use nonempty::NonEmpty; use radicle_crypto::PublicKey; +use serde::{Deserialize, Serialize}; use crate::pruning_fold; -/// Blob under an entry. -#[derive(Clone, Debug, PartialEq, Eq, Hash)] -pub struct EntryBlob { - /// The OID of the blob. - pub oid: Oid, - /// The blob data. - pub data: Vec, -} - -impl<'r> From> for EntryBlob { - fn from(blob: git2::Blob) -> Self { - Self { - oid: blob.id().into(), - data: blob.content().to_vec(), - } - } -} - /// Entry contents. /// This is the change payload. -pub type Contents = NonEmpty; +pub type Contents = NonEmpty>; /// Logical clock used to track causality in change graph. pub type Clock = u64; @@ -38,9 +23,25 @@ pub type Clock = u64; pub type Timestamp = u64; /// A unique identifier for a history entry. -#[derive(Clone, Copy, Debug, PartialEq, Hash, Eq, PartialOrd, Ord)] +#[derive(Clone, Copy, Debug, PartialEq, Hash, Eq, PartialOrd, Ord, Serialize, Deserialize)] pub struct EntryId(Oid); +impl fmt::Display for EntryId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +impl FromStr for EntryId { + type Err = git_ext::Error; + + fn from_str(s: &str) -> Result { + let oid = git_ext::Oid::try_from(s)?; + + Ok(Self(oid)) + } +} + impl From for EntryId { fn from(id: git2::Oid) -> Self { Self(id.into()) @@ -165,17 +166,9 @@ impl EntryWithClock { self.clock } - /// Get the clock range. - pub fn range(&self) -> std::ops::RangeInclusive { - self.clock..=(self.clock + self.contents.tail.len() as Clock) - } - /// Iterator over the changes, including the clock. - pub fn changes(&self) -> impl Iterator { - self.contents - .iter() - .enumerate() - .map(|(ix, blob)| (self.clock + ix as u64, blob)) + pub fn changes(&self) -> impl Iterator { + self.contents.iter().map(|blob| blob.as_slice()) } } diff --git a/radicle-cob/src/lib.rs b/radicle-cob/src/lib.rs index 08e8f41c..46ec66d3 100644 --- a/radicle-cob/src/lib.rs +++ b/radicle-cob/src/lib.rs @@ -98,7 +98,7 @@ pub use type_name::TypeName; pub mod object; pub use object::{ - create, get, info, list, remove, update, CollaborativeObject, Create, ObjectId, Update, + create, get, info, list, remove, update, CollaborativeObject, Create, ObjectId, Update, Updated, }; #[cfg(test)] diff --git a/radicle-cob/src/object.rs b/radicle-cob/src/object.rs index c1add548..1dbf467c 100644 --- a/radicle-cob/src/object.rs +++ b/radicle-cob/src/object.rs @@ -13,6 +13,7 @@ use thiserror::Error; pub mod collaboration; pub use collaboration::{ create, get, info, list, parse_refstr, remove, update, CollaborativeObject, Create, Update, + Updated, }; pub mod storage; diff --git a/radicle-cob/src/object/collaboration.rs b/radicle-cob/src/object/collaboration.rs index 47d58242..e8f2c075 100644 --- a/radicle-cob/src/object/collaboration.rs +++ b/radicle-cob/src/object/collaboration.rs @@ -27,7 +27,7 @@ mod remove; pub use remove::remove; mod update; -pub use update::{update, Update}; +pub use update::{update, Update, Updated}; /// A collaborative object #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/radicle-cob/src/object/collaboration/update.rs b/radicle-cob/src/object/collaboration/update.rs index a8ac2979..1fa3fb08 100644 --- a/radicle-cob/src/object/collaboration/update.rs +++ b/radicle-cob/src/object/collaboration/update.rs @@ -10,6 +10,15 @@ use crate::{change, change_graph::ChangeGraph, CollaborativeObject, ObjectId, St use super::error; +/// Result of an `update` operation. +#[derive(Debug)] +pub struct Updated { + /// The new head commit of the DAG. + pub head: Oid, + /// The newly updated collaborative object. + pub object: CollaborativeObject, +} + /// The data required to update an object pub struct Update { /// The type of history that will be used for this object. @@ -48,7 +57,7 @@ pub fn update( resource: Oid, identifier: &S::Identifier, args: Update, -) -> Result +) -> Result where S: Store, G: crypto::Signer, @@ -93,5 +102,8 @@ where change.timestamp, ); - Ok(object) + Ok(Updated { + object, + head: change.id, + }) } diff --git a/radicle-cob/src/tests.rs b/radicle-cob/src/tests.rs index e2fe3ecd..e8226e4b 100644 --- a/radicle-cob/src/tests.rs +++ b/radicle-cob/src/tests.rs @@ -7,7 +7,8 @@ use qcheck::Arbitrary; use radicle_crypto::Signer; use crate::{ - create, get, list, object, test::arbitrary::Invalid, update, Create, ObjectId, TypeName, Update, + create, get, list, object, test::arbitrary::Invalid, update, Create, ObjectId, TypeName, + Update, Updated, }; use super::test; @@ -121,7 +122,7 @@ fn update_cob() { .unwrap() .expect("BUG: cob was missing"); - let updated = update( + let Updated { object, .. } = update( &storage, &signer, proj.project.content_id, @@ -136,12 +137,12 @@ fn update_cob() { ) .unwrap(); - let expected = get(&storage, &typename, updated.id()) + let expected = get(&storage, &typename, object.id()) .unwrap() .expect("BUG: cob was missing"); - assert_ne!(updated, not_expected); - assert_eq!(updated, expected); + assert_ne!(object, not_expected); + assert_eq!(object, expected); } #[test] @@ -183,7 +184,7 @@ fn traverse_cobs() { ) .unwrap(); - let updated = update( + let Updated { object, .. } = update( &storage, &neil_signer, neil_proj.project.content_id, @@ -199,9 +200,9 @@ fn traverse_cobs() { .unwrap(); // traverse over the history and filter by changes that were only authorized by terry - let contents = updated.history().traverse(Vec::new(), |mut acc, entry| { + let contents = object.history().traverse(Vec::new(), |mut acc, entry| { if entry.actor() == terry_signer.public_key() { - acc.push(entry.contents().head.data.clone()); + acc.push(entry.contents().head.clone()); } ControlFlow::Continue(acc) }); @@ -209,8 +210,8 @@ fn traverse_cobs() { assert_eq!(contents, vec![b"issue 1".to_vec()]); // traverse over the history and filter by changes that were only authorized by neil - let contents = updated.history().traverse(Vec::new(), |mut acc, entry| { - acc.push(entry.contents().head.data.clone()); + let contents = object.history().traverse(Vec::new(), |mut acc, entry| { + acc.push(entry.contents().head.clone()); ControlFlow::Continue(acc) }); diff --git a/radicle-httpd/src/api/error.rs b/radicle-httpd/src/api/error.rs index 5608a649..91d593a1 100644 --- a/radicle-httpd/src/api/error.rs +++ b/radicle-httpd/src/api/error.rs @@ -81,10 +81,17 @@ impl IntoResponse for Error { StatusCode::INTERNAL_SERVER_ERROR, Some(e.message().to_owned()), ), - _ => { + other => { tracing::error!("Error: {:?}", &self); - (StatusCode::INTERNAL_SERVER_ERROR, None) + if cfg!(debug_assertions) { + ( + StatusCode::INTERNAL_SERVER_ERROR, + Some(format!("{other:?}")), + ) + } else { + (StatusCode::INTERNAL_SERVER_ERROR, None) + } } }; diff --git a/radicle-httpd/src/api/json.rs b/radicle-httpd/src/api/json.rs index dd4bed70..c542f556 100644 --- a/radicle-httpd/src/api/json.rs +++ b/radicle-httpd/src/api/json.rs @@ -9,7 +9,7 @@ use serde_json::{json, Value}; use radicle::cob::issue::{Issue, IssueId}; use radicle::cob::patch::{Patch, PatchId}; use radicle::cob::thread::{self, CommentId}; -use radicle::cob::{Author, OpId, Timestamp}; +use radicle::cob::{Author, Timestamp}; use radicle_surf::blob::Blob; use radicle_surf::tree::Tree; use radicle_surf::{Commit, Stats}; @@ -132,7 +132,7 @@ fn name_in_path(path: &str) -> &str { #[derive(Serialize)] #[serde(rename_all = "camelCase")] struct Comment { - id: OpId, + id: CommentId, author: Author, body: String, reactions: [String; 0], diff --git a/radicle-httpd/src/api/v1/projects.rs b/radicle-httpd/src/api/v1/projects.rs index 11520f6c..0720b25d 100644 --- a/radicle-httpd/src/api/v1/projects.rs +++ b/radicle-httpd/src/api/v1/projects.rs @@ -1104,7 +1104,7 @@ mod routes { "assignees": [], "discussion": [ { - "id": ISSUE_DISCUSSION_ID, + "id": ISSUE_ID, "author": { "id": "did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi" }, @@ -1122,7 +1122,7 @@ mod routes { #[tokio::test] async fn test_projects_issues_create() { - const CREATED_ISSUE_ID: &str = "1b706bb9af971d43ec38718c71402e36055c5c89"; + const CREATED_ISSUE_ID: &str = "b457364fbe2ef0eac69a835a087f60ee13ccb367"; let tmp = tempfile::tempdir().unwrap(); let ctx = test::contributor(tmp.path()); @@ -1171,7 +1171,7 @@ mod routes { "status": "open", }, "discussion": [{ - "id": ISSUE_DISCUSSION_ID, + "id": CREATED_ISSUE_ID, "author": { "id": CONTRIBUTOR_PUB_KEY, }, @@ -1245,7 +1245,7 @@ mod routes { "replyTo": null, }, { - "id": "265af21e409eacc8eb150b73882ac3ada9d4aea3", + "id": "9685b141c2e939c3d60f8ca34f8c7bf01a609af1", "author": { "id": CONTRIBUTOR_PUB_KEY, }, @@ -1269,7 +1269,7 @@ mod routes { test::create_session(ctx).await; let body = serde_json::to_vec(&json!({ - "type":"thread", + "type": "thread", "action": { "type": "comment", "body": "This is a reply to the first comment", @@ -1277,9 +1277,7 @@ mod routes { }})) .unwrap(); - let response = get(&app, format!("/projects/{CONTRIBUTOR_RID}/issues")).await; - println!("{:?}", response.json().await); - + let _ = get(&app, format!("/projects/{CONTRIBUTOR_RID}/issues")).await; let response = patch( &app, format!("/projects/{CONTRIBUTOR_RID}/issues/{CONTRIBUTOR_ISSUE_ID}"), @@ -1358,7 +1356,7 @@ mod routes { "tags": [], "revisions": [ { - "id": "47878ed82515772f4c44e4796c330f4a74473559", + "id": PATCH_ID, "description": "", "reviews": [], } @@ -1389,7 +1387,7 @@ mod routes { "tags": [], "revisions": [ { - "id": "47878ed82515772f4c44e4796c330f4a74473559", + "id": PATCH_ID, "description": "", "reviews": [], } @@ -1401,7 +1399,7 @@ mod routes { #[tokio::test] async fn test_projects_create_patches() { - const CREATED_PATCH_ID: &str = "22f8fbe09f7430579dd0730e4f2394362d844647"; + const CREATED_PATCH_ID: &str = "54505091ff3561466cfbe83e7e23c21cb1bb8a17"; let tmp = tempfile::tempdir().unwrap(); let ctx = test::contributor(tmp.path()); @@ -1459,7 +1457,7 @@ mod routes { "tags": [], "revisions": [ { - "id": "73efc59cd9f787deff0ae1629f47f1d90f307282", + "id": CREATED_PATCH_ID, "description": "", "reviews": [], } diff --git a/radicle-httpd/src/test.rs b/radicle-httpd/src/test.rs index 2c5214dc..5ba34246 100644 --- a/radicle-httpd/src/test.rs +++ b/radicle-httpd/src/test.rs @@ -1,4 +1,3 @@ -use std::convert::TryInto as _; use std::path::Path; use std::str::FromStr; use std::sync::Arc; @@ -13,47 +12,77 @@ use tower::ServiceExt; use radicle::cob::issue::Issues; use radicle::cob::patch::{MergeTarget, Patches}; +use radicle::crypto::ssh::keystore::MemorySigner; +use radicle::crypto::ssh::Keystore; +use radicle::crypto::{KeyPair, Seed, Signer}; use radicle::git::{raw as git2, RefString}; +use radicle::profile::Home; use radicle::storage::ReadStorage; +use radicle::Storage; use radicle_crypto::test::signer::MockSigner; use crate::api::{auth, Context}; pub const HEAD: &str = "1e978d19f251cd9821d9d9a76d1bd436bf0690d5"; pub const HEAD_1: &str = "f604ce9fd5b7cc77b7609beda45ea8760bee78f7"; -pub const PATCH_ID: &str = "73d5187bb38835a232ce6ff41102a94bb92e5130"; -pub const ISSUE_ID: &str = "959afe03c1dbea4f47462bb7824584a78741f59c"; -pub const ISSUE_DISCUSSION_ID: &str = "e2874702515026daf62d4385cafb88fef1fad0c8"; -pub const ISSUE_COMMENT_ID: &str = "905d72c1f13f282f86cb93ce9f7eb9464a08ba79"; +pub const PATCH_ID: &str = "6ec9a764a888576abc7e582dbf82a31e23a9789d"; +pub const ISSUE_ID: &str = "5ad77fa3f476beed9a26f49b2b3b844e61bef792"; +pub const ISSUE_DISCUSSION_ID: &str = "f1dff128a22e8183a23516dd9812e72e80914c92"; +pub const ISSUE_COMMENT_ID: &str = "845218041bf9eb8155bfa4aaa8f0c91ce18e5c13"; pub const SESSION_ID: &str = "u9MGAkkfkMOv0uDDB2WeUHBT7HbsO2Dy"; pub const TIMESTAMP: u64 = 1671125284; pub const CONTRIBUTOR_RID: &str = "rad:z4XaCmN3jLSeiMvW15YTDpNbDHFhG"; pub const CONTRIBUTOR_PUB_KEY: &str = "did:key:z6Mkk7oqY4pPxhMmGEotDYsFo97vhCj85BLY1H256HrJmjN8"; -pub const CONTRIBUTOR_ISSUE_ID: &str = "b991ccfa866e164b81a4aa0a7e082357b6c75306"; +pub const CONTRIBUTOR_ISSUE_ID: &str = "f1dff128a22e8183a23516dd9812e72e80914c92"; const PASSWORD: &str = "radicle"; +/// Create a new profile. +pub fn profile(home: &Path, seed: [u8; 32]) -> radicle::Profile { + let home = Home::new(home).unwrap(); + let storage = Storage::open(home.storage()).unwrap(); + let keystore = Keystore::new(&home.keys()); + let keypair = KeyPair::from_seed(Seed::from(seed)); + + radicle::storage::git::transport::local::register(storage.clone()); + keystore + .store(keypair.clone(), "radicle", PASSWORD.to_owned()) + .unwrap(); + + radicle::Profile { + home, + storage, + keystore, + public_key: keypair.pk.into(), + } +} + pub fn seed(dir: &Path) -> Context { - seed_with_signer(dir, false) + let home = dir.join("radicle"); + let profile = profile(home.as_path(), [0xff; 32]); + let signer = Box::new(MockSigner::from_seed([0xff; 32])); + + seed_with_signer(dir, profile, &signer) } pub fn contributor(dir: &Path) -> Context { - seed_with_signer(dir, true) + let mut seed = [0xff; 32]; + *seed.last_mut().unwrap() = 0xee; + + let home = dir.join("radicle"); + let profile = profile(home.as_path(), seed); + let signer = MemorySigner::load(&profile.keystore, PASSWORD.to_owned().into()).unwrap(); + + seed_with_signer(dir, profile, &signer) } -fn seed_with_signer(dir: &Path, self_signer: bool) -> Context { +fn seed_with_signer(dir: &Path, profile: radicle::Profile, signer: &G) -> Context { let workdir = dir.join("hello-world"); - let rad_home = dir.join("radicle"); env::set_var("RAD_COMMIT_TIME", TIMESTAMP.to_string()); env::set_var("RAD_PASSPHRASE", PASSWORD); - env::set_var( - "RAD_SEED", - "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffee", - ); fs::create_dir_all(&workdir).unwrap(); - fs::create_dir_all(&rad_home).unwrap(); // add commits to workdir (repo) let repo = git2::Repository::init(&workdir).unwrap(); @@ -95,42 +124,24 @@ fn seed_with_signer(dir: &Path, self_signer: bool) -> Context { ) .unwrap(); - // eq. rad auth - let profile = - radicle::Profile::init(rad_home.try_into().unwrap(), PASSWORD.to_owned()).unwrap(); - - let mock_signer = MockSigner::from_seed([0xff; 32]); - // rad init let repo = git2::Repository::open(&workdir).unwrap(); let name = "hello-world".to_string(); let description = "Rad repository for tests".to_string(); - let signer = if self_signer { - profile.signer().unwrap() - } else { - Box::new(mock_signer) - }; let branch = RefString::try_from("master").unwrap(); - let (id, _, _) = radicle::rad::init( - &repo, - &name, - &description, - branch, - &signer, - &profile.storage, - ) - .unwrap(); + let (id, _, _) = + radicle::rad::init(&repo, &name, &description, branch, signer, &profile.storage).unwrap(); let storage = &profile.storage; let repo = storage.repository(id).unwrap(); let mut issues = Issues::open(&repo).unwrap(); - issues + let _ = issues .create( "Issue #1".to_string(), "Change 'hello world' to 'hello everyone'".to_string(), &[], &[], - &signer, + signer, ) .unwrap(); @@ -138,7 +149,7 @@ fn seed_with_signer(dir: &Path, self_signer: bool) -> Context { let mut patches = Patches::open(&repo).unwrap(); let oid = radicle::git::Oid::from_str(HEAD).unwrap(); let base = radicle::git::Oid::from_str(HEAD_1).unwrap(); - patches + let _ = patches .create( "A new `hello word`", "change `hello world` in README to something else", @@ -146,7 +157,7 @@ fn seed_with_signer(dir: &Path, self_signer: bool) -> Context { base, oid, &[], - &signer, + signer, ) .unwrap(); diff --git a/radicle/Cargo.toml b/radicle/Cargo.toml index 973f2e25..19bbf4ee 100644 --- a/radicle/Cargo.toml +++ b/radicle/Cargo.toml @@ -24,7 +24,6 @@ serde = { version = "1", features = ["derive"] } serde_json = { version = "1", features = ["preserve_order"] } siphasher = { version = "0.3.10" } radicle-git-ext = { version = "0", features = ["serde"] } -rand = { version = "0.8.5", default-features = false, features = ["std_rng"] } sqlite = { version = "0.30.3", optional = true } tempfile = { version = "3.3.0" } thiserror = { version = "1" } diff --git a/radicle/src/cob.rs b/radicle/src/cob.rs index 59eead44..05d29c7c 100644 --- a/radicle/src/cob.rs +++ b/radicle/src/cob.rs @@ -11,10 +11,10 @@ pub mod test; pub use cob::{create, get, list, remove, update}; pub use cob::{ - history::entry::EntryBlob, object::collaboration::error, CollaborativeObject, Contents, Create, - Entry, History, ObjectId, TypeName, Update, + history::EntryId, object::collaboration::error, CollaborativeObject, Contents, Create, Entry, + History, ObjectId, TypeName, Update, Updated, }; pub use common::*; -pub use op::{ActorId, Op, OpId}; +pub use op::{ActorId, Op}; use radicle_cob as cob; diff --git a/radicle/src/cob/identity.rs b/radicle/src/cob/identity.rs index a05c4e6e..34037635 100644 --- a/radicle/src/cob/identity.rs +++ b/radicle/src/cob/identity.rs @@ -22,7 +22,7 @@ use crate::{ use super::{ thread::{self, Thread}, - Author, OpId, + Author, EntryId, }; /// The logical clock we use to order operations to proposals. @@ -36,7 +36,7 @@ pub type Op = cob::Op; pub type ProposalId = ObjectId; -pub type RevisionId = OpId; +pub type RevisionId = EntryId; /// Proposal operation. #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] @@ -79,13 +79,13 @@ pub enum ApplyError { /// For example, this can occur if an operation references anothern operation /// that hasn't happened yet. #[error("causal dependency {0:?} missing")] - Missing(OpId), + Missing(EntryId), #[error("the proposal is committed")] Committed, #[error(transparent)] Commit(#[from] CommitError), #[error("the revision {0:?} is redacted")] - Redacted(OpId), + Redacted(EntryId), /// Error applying an op to the proposal thread. #[error("thread apply failed: {0}")] Thread(#[from] thread::OpError), @@ -97,21 +97,21 @@ pub enum CommitError { #[error(transparent)] Identity(#[from] IdentityError), #[error("the proposal {0} is closed")] - Closed(OpId), + Closed(EntryId), #[error("the revision {0} is missing")] - Missing(OpId), + Missing(EntryId), #[error( "the identity hashes do match '{current} =/= {expected}' for the revision '{revision}'" )] Mismatch { current: Oid, expected: Oid, - revision: OpId, + revision: EntryId, }, #[error("the revision {0} is already committed")] - Committed(OpId), + Committed(EntryId), #[error("the revision {0} is redacted")] - Redacted(OpId), + Redacted(EntryId), #[error(transparent)] Doc(#[from] DocError), #[error("signatures did not reach quorum threshold: {0}")] @@ -145,7 +145,7 @@ pub struct Proposal { /// List of revisions for this proposal. revisions: GMap>, /// Timeline of events. - timeline: GSet<(clock::Lamport, OpId)>, + timeline: GSet<(clock::Lamport, EntryId)>, } #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] @@ -359,7 +359,6 @@ impl store::FromHistory for Proposal { revision.discussion.apply([cob::Op::new( op.id, action, - op.nonce, op.author, op.timestamp, op.clock, @@ -477,14 +476,14 @@ impl store::Transaction { &mut self, revision: RevisionId, signature: Signature, - ) -> Result { + ) -> Result<(), store::Error> { self.push(Action::Accept { revision, signature, }) } - pub fn reject(&mut self, revision: RevisionId) -> Result { + pub fn reject(&mut self, revision: RevisionId) -> Result<(), store::Error> { self.push(Action::Reject { revision }) } @@ -492,22 +491,18 @@ impl store::Transaction { &mut self, title: impl ToString, description: impl ToString, - ) -> Result { + ) -> Result<(), store::Error> { self.push(Action::Edit { title: title.to_string(), description: description.to_string(), }) } - pub fn redact(&mut self, revision: RevisionId) -> Result { + pub fn redact(&mut self, revision: RevisionId) -> Result<(), store::Error> { self.push(Action::Redact { revision }) } - pub fn revision( - &mut self, - current: Oid, - proposed: Doc, - ) -> Result { + pub fn revision(&mut self, current: Oid, proposed: Doc) -> Result<(), store::Error> { self.push(Action::Revision { current, proposed }) } @@ -516,7 +511,7 @@ impl store::Transaction { &mut self, revision: RevisionId, body: S, - ) -> Result { + ) -> Result<(), store::Error> { self.push(Action::Thread { revision, action: thread::Action::Comment { @@ -532,7 +527,7 @@ impl store::Transaction { revision: RevisionId, body: S, reply_to: thread::CommentId, - ) -> Result { + ) -> Result<(), store::Error> { self.push(Action::Thread { revision, action: thread::Action::Comment { @@ -566,24 +561,24 @@ impl<'a, 'g> ProposalMut<'a, 'g> { } } - pub fn transaction( + pub fn transaction( &mut self, message: &str, signer: &G, operations: F, - ) -> Result + ) -> Result where G: Signer, - F: FnOnce(&mut Transaction) -> Result, + F: FnOnce(&mut Transaction) -> Result<(), store::Error>, { - let mut tx = Transaction::new(*signer.public_key(), self.clock, self.store.rng()); - let output = operations(&mut tx)?; - let (ops, clock) = tx.commit(message, self.id, &mut self.store.raw, signer)?; + let mut tx = Transaction::new(*signer.public_key(), self.clock); + operations(&mut tx)?; + let (ops, clock, commit) = tx.commit(message, self.id, &mut self.store.raw, signer)?; self.proposal.apply(ops)?; self.clock = clock; - Ok(output) + Ok(commit) } /// Get the internal logical clock. @@ -597,12 +592,16 @@ impl<'a, 'g> ProposalMut<'a, 'g> { revision: RevisionId, signature: Signature, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Accept", signer, |tx| tx.accept(revision, signature)) } /// Reject a proposal revision. - pub fn reject(&mut self, revision: RevisionId, signer: &G) -> Result { + pub fn reject( + &mut self, + revision: RevisionId, + signer: &G, + ) -> Result { self.transaction("Reject", signer, |tx| tx.reject(revision)) } @@ -612,17 +611,17 @@ impl<'a, 'g> ProposalMut<'a, 'g> { title: String, description: String, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Edit", signer, |tx| tx.edit(title, description)) } /// Commit a proposal. - pub fn commit(&mut self, signer: &G) -> Result { + pub fn commit(&mut self, signer: &G) -> Result { self.transaction("Commit", signer, |tx| tx.push(Action::Commit)) } /// Close a proposal. - pub fn close(&mut self, signer: &G) -> Result { + pub fn close(&mut self, signer: &G) -> Result { self.transaction("Close", signer, |tx| tx.push(Action::Close)) } @@ -633,7 +632,7 @@ impl<'a, 'g> ProposalMut<'a, 'g> { body: S, reply_to: thread::CommentId, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Comment", signer, |tx| tx.comment(revision, body, reply_to)) } @@ -643,7 +642,7 @@ impl<'a, 'g> ProposalMut<'a, 'g> { current: impl Into, proposed: Doc, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Add revision", signer, |tx| { tx.revision(current.into(), proposed) }) @@ -695,7 +694,7 @@ impl<'a> Proposals<'a> { Ok(()) })?; // Just a sanity check that our clock is advancing as expected. - debug_assert_eq!(clock.get(), 2); + debug_assert_eq!(clock.get(), 1); Ok(ProposalMut::new(id, proposal, clock, self)) } diff --git a/radicle/src/cob/issue.rs b/radicle/src/cob/issue.rs index 57f893ec..84afa93b 100644 --- a/radicle/src/cob/issue.rs +++ b/radicle/src/cob/issue.rs @@ -14,7 +14,7 @@ use crate::cob::store::FromHistory as _; use crate::cob::store::Transaction; use crate::cob::thread; use crate::cob::thread::{CommentId, Thread}; -use crate::cob::{store, ActorId, ObjectId, OpId, TypeName}; +use crate::cob::{store, ActorId, EntryId, ObjectId, TypeName}; use crate::crypto::Signer; use crate::prelude::Did; use crate::storage::git as storage; @@ -146,7 +146,6 @@ impl store::FromHistory for Issue { self.thread.apply([cob::Op::new( op.id, action, - op.nonce, op.author, op.timestamp, op.clock, @@ -214,7 +213,7 @@ impl store::Transaction { &mut self, add: impl IntoIterator, remove: impl IntoIterator, - ) -> Result { + ) -> Result<(), store::Error> { let add = add.into_iter().collect::>(); let remove = remove.into_iter().collect::>(); @@ -222,19 +221,19 @@ impl store::Transaction { } /// Set the issue title. - pub fn edit(&mut self, title: impl ToString) -> Result { + pub fn edit(&mut self, title: impl ToString) -> Result<(), store::Error> { self.push(Action::Edit { title: title.to_string(), }) } /// Lifecycle an issue. - pub fn lifecycle(&mut self, state: State) -> Result { + pub fn lifecycle(&mut self, state: State) -> Result<(), store::Error> { self.push(Action::Lifecycle { state }) } /// Create the issue thread. - pub fn thread(&mut self, body: S) -> Result { + pub fn thread(&mut self, body: S) -> Result<(), store::Error> { self.push(Action::from(thread::Action::Comment { body: body.to_string(), reply_to: None, @@ -246,7 +245,7 @@ impl store::Transaction { &mut self, body: S, reply_to: CommentId, - ) -> Result { + ) -> Result<(), store::Error> { self.push(Action::from(thread::Action::Comment { body: body.to_string(), reply_to: Some(reply_to), @@ -258,7 +257,7 @@ impl store::Transaction { &mut self, add: impl IntoIterator, remove: impl IntoIterator, - ) -> Result { + ) -> Result<(), store::Error> { let add = add.into_iter().collect::>(); let remove = remove.into_iter().collect::>(); @@ -266,7 +265,7 @@ impl store::Transaction { } /// React to an issue comment. - pub fn react(&mut self, to: CommentId, reaction: Reaction) -> Result { + pub fn react(&mut self, to: CommentId, reaction: Reaction) -> Result<(), store::Error> { self.push(Action::Thread { action: thread::Action::React { to, @@ -300,17 +299,17 @@ impl<'a, 'g> IssueMut<'a, 'g> { &mut self, assignees: impl IntoIterator, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Assign", signer, |tx| tx.assign(assignees, [])) } /// Set the issue title. - pub fn edit(&mut self, title: impl ToString, signer: &G) -> Result { + pub fn edit(&mut self, title: impl ToString, signer: &G) -> Result { self.transaction("Edit", signer, |tx| tx.edit(title)) } /// Lifecycle an issue. - pub fn lifecycle(&mut self, state: State, signer: &G) -> Result { + pub fn lifecycle(&mut self, state: State, signer: &G) -> Result { self.transaction("Lifecycle", signer, |tx| tx.lifecycle(state)) } @@ -319,7 +318,7 @@ impl<'a, 'g> IssueMut<'a, 'g> { &mut self, body: S, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Create thread", signer, |tx| tx.thread(body)) } @@ -329,8 +328,11 @@ impl<'a, 'g> IssueMut<'a, 'g> { body: S, reply_to: CommentId, signer: &G, - ) -> Result { - assert!(self.thread.comment(&reply_to).is_some()); + ) -> Result { + assert!( + self.thread.comment(&reply_to).is_some(), + "Comment {reply_to} not found" + ); self.transaction("Comment", signer, |tx| tx.comment(body, reply_to)) } @@ -340,7 +342,7 @@ impl<'a, 'g> IssueMut<'a, 'g> { add: impl IntoIterator, remove: impl IntoIterator, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Tag", signer, |tx| tx.tag(add, remove)) } @@ -350,7 +352,7 @@ impl<'a, 'g> IssueMut<'a, 'g> { to: CommentId, reaction: Reaction, signer: &G, - ) -> Result { + ) -> Result { self.transaction("React", signer, |tx| tx.react(to, reaction)) } @@ -359,30 +361,29 @@ impl<'a, 'g> IssueMut<'a, 'g> { &mut self, assignees: impl IntoIterator, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Unassign", signer, |tx| tx.assign([], assignees)) .map_err(Error::from) } - pub fn transaction( + pub fn transaction( &mut self, message: &str, signer: &G, operations: F, - ) -> Result + ) -> Result where G: Signer, - F: FnOnce(&mut Transaction) -> Result, + F: FnOnce(&mut Transaction) -> Result<(), store::Error>, { - let rng = self.store.rng(); - let mut tx = Transaction::new(*signer.public_key(), self.clock, rng); - let output = operations(&mut tx)?; - let (ops, clock) = tx.commit(message, self.id, &mut self.store.raw, signer)?; + let mut tx = Transaction::new(*signer.public_key(), self.clock); + operations(&mut tx)?; + let (ops, clock, commit) = tx.commit(message, self.id, &mut self.store.raw, signer)?; self.issue.apply(ops)?; self.clock = clock; - Ok(output) + Ok(commit) } } @@ -461,7 +462,7 @@ impl<'a> Issues<'a> { Ok(()) })?; // Just a sanity check that our clock is advancing as expected. - debug_assert_eq!(clock.get(), 4); + debug_assert_eq!(clock.get(), 1); Ok(IssueMut { id, @@ -618,7 +619,7 @@ mod test { .create("My first issue", "Blah blah blah.", &[], &[], &signer) .unwrap(); - assert_eq!(created.clock().get(), 4); + assert_eq!(created.clock().get(), 1); let (id, created) = (created.id, created.issue); let issue = issues.get(&id).unwrap().unwrap(); @@ -812,6 +813,8 @@ mod test { .create("My first issue", "Blah blah blah.", &[], &[], &signer) .unwrap(); + assert_eq!(issue.clock.get(), 1); + // The root thread op id is always the same. let (c0, _) = issue.root(); let c0 = *c0; @@ -819,6 +822,8 @@ mod test { issue.comment("Ho ho ho.", c0, &signer).unwrap(); issue.comment("Ha ha ha.", c0, &signer).unwrap(); + assert_eq!(issue.clock.get(), 3); + let id = issue.id; let issue = issues.get(&id).unwrap().unwrap(); let (_, c0) = &issue.comments().nth(0).unwrap(); @@ -888,8 +893,6 @@ mod test { ) .unwrap(); - assert_eq!(created.clock().get(), 4); - let (id, created) = (created.id, created.issue); let issue = issues.get(&id).unwrap().unwrap(); diff --git a/radicle/src/cob/op.rs b/radicle/src/cob/op.rs index 9b4cc518..60ac5982 100644 --- a/radicle/src/cob/op.rs +++ b/radicle/src/cob/op.rs @@ -1,107 +1,14 @@ -use std::fmt; -use std::str; -use std::str::FromStr; - use nonempty::NonEmpty; -use serde::{Deserialize, Serialize}; use thiserror::Error; -use radicle_cob::history::EntryWithClock; +use radicle_cob::history::{EntryId, EntryWithClock}; use radicle_crdt::clock; use radicle_crdt::clock::Lamport; use radicle_crypto::PublicKey; -use crate::git; - -/// Identifies an [`Op`] internally and within the change graph. -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] -#[serde(into = "String", try_from = "String")] -pub struct OpId(git::Oid); - -impl OpId { - /// Create a new operation id. - pub fn new(oid: git::Oid) -> Self { - Self(oid) - } -} - -impl fmt::Display for OpId { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) - } -} - -impl From for git::Oid { - fn from(value: OpId) -> Self { - value.0 - } -} - -impl From for git2::Oid { - fn from(value: OpId) -> Self { - value.0.into() - } -} - -impl From for OpId { - fn from(value: git::Oid) -> Self { - Self(value) - } -} - -impl From for OpId { - fn from(value: git2::Oid) -> Self { - Self(value.into()) - } -} - -// Used by `serde::Serialize`. -impl From for String { - fn from(value: OpId) -> Self { - value.to_string() - } -} - -// Used by `serde::Deserialize`. -impl TryFrom for OpId { - type Error = git::raw::Error; - - fn try_from(value: String) -> Result { - value.as_str().try_into() - } -} - -/// Error decoding an operation from an entry. -#[derive(Error, Debug)] -pub enum OpIdError { - #[error("cannot parse op id from empty string")] - Empty, - #[error("badly formatted op id")] - BadFormat, -} - -impl FromStr for OpId { - type Err = git::raw::Error; - - fn from_str(s: &str) -> Result { - Self::try_from(s) - } -} - -impl TryFrom<&str> for OpId { - type Error = git::raw::Error; - - fn try_from(s: &str) -> Result { - git::Oid::try_from(s).map(Self) - } -} - /// The author of an [`Op`]. pub type ActorId = PublicKey; -/// Random number used to prevent op-id collisions. -pub type Nonce = u64; - /// Error decoding an operation from an entry. #[derive(Error, Debug)] pub enum OpEncodingError { @@ -111,29 +18,16 @@ pub enum OpEncodingError { Git(#[from] git2::Error), } -/// The operation payload that is actually stored on disk as a git blob. -#[derive(Debug, Clone, Deserialize)] -pub struct OpBlob { - /// The underlying action. - pub action: A, - /// A random number used to disambiguate otherwise identical ops (actions). - /// Note that since the timestamp and author are not stored at the individual op level, - /// but instead at the commit level; individual ops can trivially collide. - pub nonce: Nonce, -} - /// The `Op` is the operation that is applied onto a state to form a CRDT. /// /// Everything that can be done in the system is represented by an `Op`. /// Operations are applied to an accumulator to yield a final state. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Op { - /// Operation id. - pub id: OpId, + /// Id of the entry under which this operation lives. + pub id: EntryId, /// The action carried out by this operation. pub action: A, - /// The nonce from the [`OpBlob`]. - pub nonce: Nonce, /// The author of the operation. pub author: ActorId, /// Lamport clock. @@ -156,9 +50,8 @@ impl Ord for Op { impl Op { pub fn new( - id: OpId, + id: EntryId, action: A, - nonce: Nonce, author: ActorId, timestamp: impl Into, clock: Lamport, @@ -166,14 +59,13 @@ impl Op { Self { id, action, - nonce, author, clock, timestamp: timestamp.into(), } } - pub fn id(&self) -> OpId { + pub fn id(&self) -> EntryId { self.id } } @@ -187,16 +79,16 @@ where type Error = OpEncodingError; fn try_from(entry: &'a EntryWithClock) -> Result { + let id = *entry.id(); let ops = entry .changes() - .map(|(clock, blob)| { - let OpBlob { action, nonce } = serde_json::from_slice(blob.data.as_slice())?; + .map(|blob| { + let action = serde_json::from_slice(blob)?; let op = Op { - id: blob.oid.into(), + id, action, - nonce, author: *entry.actor(), - clock: clock.into(), + clock: entry.clock().into(), timestamp: entry.timestamp().into(), }; Ok::<_, Self::Error>(op) diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index e2fe4437..2f152f39 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -19,7 +19,7 @@ use crate::cob::store::Transaction; use crate::cob::thread; use crate::cob::thread::CommentId; use crate::cob::thread::Thread; -use crate::cob::{store, ActorId, ObjectId, OpId, TypeName}; +use crate::cob::{store, ActorId, EntryId, ObjectId, TypeName}; use crate::crypto::{PublicKey, Signer}; use crate::git; use crate::prelude::*; @@ -39,13 +39,13 @@ pub type Op = cob::Op; pub type PatchId = ObjectId; /// Unique identifier for a patch revision. -pub type RevisionId = OpId; +pub type RevisionId = EntryId; /// Index of a revision in the revisions list. pub type RevisionIx = usize; /// Error applying an operation onto a state. -#[derive(Error, Debug)] +#[derive(Debug, Error)] pub enum ApplyError { /// Causal dependency missing. /// @@ -55,7 +55,7 @@ pub enum ApplyError { /// For example, this can occur if an operation references anothern operation /// that hasn't happened yet. #[error("causal dependency {0:?} missing")] - Missing(OpId), + Missing(EntryId), /// Error applying an op to the patch thread. #[error("thread apply failed: {0}")] Thread(#[from] thread::OpError), @@ -135,7 +135,7 @@ pub struct Patch { /// first revision. pub revisions: GMap>, /// Timeline of operations. - pub timeline: GSet<(Lamport, OpId)>, + pub timeline: GSet<(Lamport, EntryId)>, } impl Semilattice for Patch { @@ -335,9 +335,9 @@ impl store::FromHistory for Patch { // TODO(cloudhead): Make sure we can deal with redacted revisions which are added // to out of order, like in the `Merge` case. if let Some(Redactable::Present(revision)) = self.revisions.get_mut(&revision) { - revision.discussion.apply([cob::Op::new( - op.id, action, op.nonce, op.author, timestamp, op.clock, - )])?; + revision + .discussion + .apply([cob::Op::new(op.id, action, op.author, timestamp, op.clock)])?; } else { return Err(ApplyError::Missing(revision)); } @@ -561,7 +561,7 @@ impl store::Transaction { title: impl ToString, description: impl ToString, target: MergeTarget, - ) -> Result { + ) -> Result<(), store::Error> { self.push(Action::Edit { title: title.to_string(), description: description.to_string(), @@ -574,7 +574,7 @@ impl store::Transaction { &mut self, revision: RevisionId, body: S, - ) -> Result { + ) -> Result<(), store::Error> { self.push(Action::Thread { revision, action: thread::Action::Comment { @@ -590,7 +590,7 @@ impl store::Transaction { revision: RevisionId, body: S, reply_to: Option, - ) -> Result { + ) -> Result<(), store::Error> { self.push(Action::Thread { revision, action: thread::Action::Comment { @@ -607,7 +607,7 @@ impl store::Transaction { verdict: Option, comment: Option, inline: Vec, - ) -> Result { + ) -> Result<(), store::Error> { self.push(Action::Review { revision, comment, @@ -617,7 +617,7 @@ impl store::Transaction { } /// Merge a patch revision. - pub fn merge(&mut self, revision: RevisionId, commit: git::Oid) -> Result { + pub fn merge(&mut self, revision: RevisionId, commit: git::Oid) -> Result<(), store::Error> { self.push(Action::Merge { revision, commit }) } @@ -627,7 +627,7 @@ impl store::Transaction { description: impl ToString, base: impl Into, oid: impl Into, - ) -> Result { + ) -> Result<(), store::Error> { self.push(Action::Revision { description: description.to_string(), base: base.into(), @@ -640,7 +640,7 @@ impl store::Transaction { &mut self, add: impl IntoIterator, remove: impl IntoIterator, - ) -> Result { + ) -> Result<(), store::Error> { let add = add.into_iter().collect::>(); let remove = remove.into_iter().collect::>(); @@ -671,24 +671,24 @@ impl<'a, 'g> PatchMut<'a, 'g> { } } - pub fn transaction( + pub fn transaction( &mut self, message: &str, signer: &G, operations: F, - ) -> Result + ) -> Result where G: Signer, - F: FnOnce(&mut Transaction) -> Result, + F: FnOnce(&mut Transaction) -> Result<(), store::Error>, { - let mut tx = Transaction::new(*signer.public_key(), self.clock, self.store.rng()); - let output = operations(&mut tx)?; - let (ops, clock) = tx.commit(message, self.id, &mut self.store.raw, signer)?; + let mut tx = Transaction::new(*signer.public_key(), self.clock); + operations(&mut tx)?; + let (ops, clock, commit) = tx.commit(message, self.id, &mut self.store.raw, signer)?; self.patch.apply(ops)?; self.clock = clock; - Ok(output) + Ok(commit) } /// Get the internal logical clock. @@ -703,7 +703,7 @@ impl<'a, 'g> PatchMut<'a, 'g> { description: String, target: MergeTarget, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Edit", signer, |tx| tx.edit(title, description, target)) } @@ -714,7 +714,7 @@ impl<'a, 'g> PatchMut<'a, 'g> { body: S, reply_to: Option, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Comment", signer, |tx| tx.comment(revision, body, reply_to)) } @@ -726,7 +726,7 @@ impl<'a, 'g> PatchMut<'a, 'g> { comment: Option, inline: Vec, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Review", signer, |tx| { tx.review(revision, verdict, comment, inline) }) @@ -738,7 +738,7 @@ impl<'a, 'g> PatchMut<'a, 'g> { revision: RevisionId, commit: git::Oid, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Merge revision", signer, |tx| tx.merge(revision, commit)) } @@ -749,7 +749,7 @@ impl<'a, 'g> PatchMut<'a, 'g> { base: impl Into, oid: impl Into, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Add revision", signer, |tx| { tx.revision(description, base, oid) }) @@ -761,7 +761,7 @@ impl<'a, 'g> PatchMut<'a, 'g> { add: impl IntoIterator, remove: impl IntoIterator, signer: &G, - ) -> Result { + ) -> Result { self.transaction("Tag", signer, |tx| tx.tag(add, remove)) } } @@ -823,7 +823,7 @@ impl<'a> Patches<'a> { Ok(()) })?; // Just a sanity check that our clock is advancing as expected. - debug_assert_eq!(clock.get(), 3); + debug_assert_eq!(clock.get(), 1); Ok(PatchMut::new(id, patch, clock, self)) } @@ -926,7 +926,7 @@ mod test { type State = ( Actor, clock::Lamport, - Vec, + Vec, Vec, ); @@ -1085,7 +1085,7 @@ mod test { ) .unwrap(); - assert_eq!(patch.clock.get(), 3); + assert_eq!(patch.clock.get(), 1); let id = patch.id; let patch = patches.get(&id).unwrap().unwrap(); @@ -1364,18 +1364,25 @@ mod test { ) .unwrap(); - assert_eq!(patch.clock.get(), 3); + assert_eq!(patch.clock.get(), 1); assert_eq!(patch.description(), Some("Blah blah blah.")); assert_eq!(patch.version(), 0); let _ = patch .update("I've made changes.", base, rev1_oid, &signer) .unwrap(); + assert_eq!(patch.clock.get(), 2); let id = patch.id; let patch = patches.get(&id).unwrap().unwrap(); assert_eq!(patch.version(), 1); assert_eq!(patch.revisions.len(), 2); + assert_eq!(patch.revisions().count(), 2); + assert_eq!(patch.revisions().nth(0).unwrap().1.description(), ""); + assert_eq!( + patch.revisions().nth(1).unwrap().1.description(), + "I've made changes." + ); let (_, revision) = patch.latest().unwrap(); diff --git a/radicle/src/cob/store.rs b/radicle/src/cob/store.rs index 61b10948..edaf2e23 100644 --- a/radicle/src/cob/store.rs +++ b/radicle/src/cob/store.rs @@ -6,13 +6,10 @@ use std::ops::ControlFlow; use nonempty::NonEmpty; use radicle_crdt::Lamport; -use rand::rngs::StdRng; -use rand::{RngCore as _, SeedableRng}; use serde::{Deserialize, Serialize}; -use crate::cob::op::{Nonce, Op, OpId, Ops}; -use crate::cob::CollaborativeObject; -use crate::cob::{ActorId, Create, History, ObjectId, TypeName, Update}; +use crate::cob::op::{Op, Ops}; +use crate::cob::{ActorId, Create, EntryId, History, ObjectId, TypeName, Update, Updated}; use crate::git; use crate::prelude::*; use crate::storage::git as storage; @@ -91,7 +88,6 @@ pub struct Store<'a, T> { parent: git::Oid, repo: &'a storage::Repository, witness: PhantomData, - rng: StdRng, } impl<'a, T> AsRef for Store<'a, T> { @@ -103,21 +99,14 @@ impl<'a, T> AsRef for Store<'a, T> { impl<'a, T> Store<'a, T> { /// Open a new generic store. pub fn open(repo: &'a storage::Repository) -> Result { - let rng = rng::std(); let identity = repo.identity()?; Ok(Self { repo, parent: identity.head, witness: PhantomData, - rng, }) } - - /// Derive a new RNG from the existing one. - pub fn rng(&self) -> StdRng { - StdRng::from_rng(self.rng.clone()).expect("Store::rng: failed to derive RNG") - } } impl<'a, T: FromHistory> Store<'a, T> @@ -129,11 +118,11 @@ where &self, object_id: ObjectId, message: &str, - actions: impl Into>>, + actions: impl Into>, signer: &G, - ) -> Result { - let changes = actions.into(); - let obj = cob::update( + ) -> Result { + let changes = actions.into().try_map(encoding::encode)?; + let updated = cob::update( self.repo, signer, self.parent, @@ -149,17 +138,17 @@ where self.repo.sign_refs(signer).map_err(Error::SignRefs)?; - Ok(obj) + Ok(updated) } /// Create an object. pub fn create( &self, message: &str, - actions: impl Into>>, + actions: impl Into>, signer: &G, ) -> Result<(ObjectId, T, Lamport), Error> { - let contents = actions.into(); + let contents = actions.into().try_map(encoding::encode)?; let cob = cob::create( self.repo, signer, @@ -227,22 +216,16 @@ where #[derive(Debug)] pub struct Transaction { actor: ActorId, - start: Lamport, clock: Lamport, - rng: StdRng, - actions: Vec<(T::Action, OpId, Nonce, Vec)>, + actions: Vec, } impl Transaction { /// Create a new transaction. - pub fn new(actor: ActorId, clock: Lamport, rng: StdRng) -> Self { - let start = clock; - + pub fn new(actor: ActorId, clock: Lamport) -> Self { Self { actor, - start, clock, - rng, actions: Vec::new(), } } @@ -262,16 +245,14 @@ impl Transaction { let actor = *signer.public_key(); let mut tx = Transaction { actor, - start: Lamport::initial(), - clock: Lamport::initial(), - rng: store.rng(), + // Nb. The clock is never zero. + clock: Lamport::initial().tick(), actions: Vec::new(), }; operations(&mut tx)?; let actions = NonEmpty::from_vec(tx.actions) - .expect("Transaction::initial: transaction must contain at least one operation") - .map(|(_, _, _, blob)| blob); + .expect("Transaction::initial: transaction must contain at least one operation"); let (id, cob, clock) = store.create(message, actions, signer)?; // The history clock should be in sync with the tx clock. @@ -281,53 +262,49 @@ impl Transaction { } /// Add an operation to this transaction. - pub fn push(&mut self, action: T::Action) -> Result { - let nonce = self.rng.next_u64(); - let (id, blob) = encoding::encode(&action, nonce)?; + pub fn push(&mut self, action: T::Action) -> Result<(), Error> { + self.actions.push(action); - self.actions.push((action, id, nonce, blob)); - self.clock.tick(); - - Ok(id) + Ok(()) } /// Commit transaction. /// /// Returns a list of operations that can be applied onto an in-memory CRDT. pub fn commit( - self, + mut self, msg: &str, id: ObjectId, store: &mut Store, signer: &G, - ) -> Result<(Vec>, Lamport), Error> + ) -> Result<(Vec>, Lamport, EntryId), Error> where T::Action: Serialize + Clone, { let actions = NonEmpty::from_vec(self.actions) .expect("Transaction::commit: transaction must not be empty"); - let cob = store.update(id, msg, actions.clone().map(|(_, _, _, blob)| blob), signer)?; + let Updated { head, object } = store.update(id, msg, actions.clone(), signer)?; + let id = EntryId::from(head); let author = self.actor; - let timestamp = cob.history().timestamp().into(); + let timestamp = object.history().timestamp().into(); + let clock = self.clock.tick(); // The history clock should be in sync with the tx clock. - assert_eq!(cob.history().clock(), self.clock.get()); + assert_eq!(object.history().clock(), self.clock.get()); // Start the clock from where the transcation clock started. - let mut clock = self.start; let ops = actions .into_iter() - .map(|(action, id, nonce, _)| cob::Op { + .map(|action| cob::Op { id, - nonce, action, author, - clock: clock.tick(), + clock, timestamp, }) .collect(); - Ok((ops, clock)) + Ok((ops, clock, id)) } } @@ -335,42 +312,15 @@ pub mod encoding { use serde::Serialize; use crate::canonical::formatter::CanonicalFormatter; - use crate::cob::op::{Nonce, OpId}; /// Serialize the change into a byte string. - pub fn encode( - action: &A, - nonce: Nonce, - ) -> Result<(OpId, Vec), serde_json::Error> { + pub fn encode(action: A) -> Result, serde_json::Error> { let mut buf = Vec::new(); let mut serializer = serde_json::Serializer::with_formatter(&mut buf, CanonicalFormatter::new()); - serde_json::json!({ - "action": action, - "nonce": nonce, - }) - .serialize(&mut serializer)?; + action.serialize(&mut serializer)?; - // SAFETY: This really shouldn't fail, since we're providing a valid object type. - let oid = git2::Oid::hash_object(git2::ObjectType::Blob, buf.as_slice()) - .expect("encoding::encode: failed to get object hash for change") - .into(); - - Ok((oid, buf)) - } -} - -pub mod rng { - use rand::{rngs::StdRng, SeedableRng}; - - /// Get the "standard" CSPRNG, seeded from OS entropy. - /// The seed can be overwritten in debug mode with the `RAD_SEED` environment variable. - pub fn std() -> StdRng { - #[cfg(debug_assertions)] - if let Some(seed) = crate::env::seed() { - return StdRng::from_seed(seed); - } - StdRng::from_entropy() + Ok(buf) } } diff --git a/radicle/src/cob/test.rs b/radicle/src/cob/test.rs index cc16eb7f..4831b0d1 100644 --- a/radicle/src/cob/test.rs +++ b/radicle/src/cob/test.rs @@ -8,7 +8,7 @@ use serde::Serialize; use crate::cob::common::clock; use crate::cob::op::{Op, Ops}; use crate::cob::store::encoding; -use crate::cob::{EntryBlob, History}; +use crate::cob::History; use crate::crypto::{PublicKey, Signer}; use crate::git::Oid; use crate::test::arbitrary; @@ -30,17 +30,14 @@ where pub fn new(op: &Op) -> HistoryBuilder { let entry = arbitrary::oid(); let resource = arbitrary::oid(); - let (id, data) = encoding::encode(&op.action, op.nonce).unwrap(); + let data = encoding::encode(&op.action).unwrap(); Self { history: History::new_from_root( entry, op.author, resource, - NonEmpty::new(EntryBlob { - oid: id.into(), - data, - }), + NonEmpty::new(data), op.timestamp.as_secs(), ), resource, @@ -49,16 +46,13 @@ where } pub fn append(&mut self, op: &Op) -> &mut Self { - let (id, data) = encoding::encode(&op.action, op.nonce).unwrap(); + let data = encoding::encode(&op.action).unwrap(); self.history.extend( arbitrary::oid(), op.author, self.resource, - NonEmpty::new(EntryBlob { - oid: id.into(), - data, - }), + NonEmpty::new(data), op.timestamp.as_secs(), ); self @@ -153,15 +147,13 @@ impl Actor { /// Create a new operation. pub fn op(&mut self, action: A) -> Op { + let id = arbitrary::oid().into(); let author = *self.signer.public_key(); let clock = self.clock.tick(); let timestamp = clock::Physical::now(); - let nonce = fastrand::u64(..); - let (id, _) = encoding::encode(&action, nonce).unwrap(); let op = Op { id, action, - nonce, author, clock, timestamp, diff --git a/radicle/src/cob/thread.rs b/radicle/src/cob/thread.rs index b6046f1e..0a87a149 100644 --- a/radicle/src/cob/thread.rs +++ b/radicle/src/cob/thread.rs @@ -8,7 +8,7 @@ use thiserror::Error; use crate::cob; use crate::cob::common::{Reaction, Timestamp}; -use crate::cob::{ActorId, Op, OpId}; +use crate::cob::{ActorId, EntryId, Op}; use crdt::clock::Lamport; use crdt::{GMap, GSet, LWWSet, Max, Redactable, Semilattice}; @@ -29,11 +29,11 @@ pub enum OpError { /// For example, this can occur if an operation references anothern operation /// that hasn't happened yet. #[error("causal dependency {0:?} missing")] - Missing(OpId), + Missing(EntryId), } /// Identifies a comment. -pub type CommentId = OpId; +pub type CommentId = EntryId; /// A comment edit is just some text and an edit time. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] @@ -144,7 +144,7 @@ pub enum Action { Edit { id: CommentId, body: String }, /// Redact a change. Not all changes can be redacted. Redact { id: CommentId }, - /// React to a change. + /// React to a comment. React { to: CommentId, reaction: Reaction, @@ -166,7 +166,7 @@ pub struct Thread { /// Reactions to changes. reactions: GMap>, /// Comment timeline. - timeline: GSet<(Lamport, OpId)>, + timeline: GSet<(Lamport, EntryId)>, } impl Semilattice for Thread { @@ -277,6 +277,7 @@ impl cob::store::FromHistory for Thread { if self.comments.contains_key(&id) { continue; } + self.comments.insert( id, Redactable::Present(Comment::new(author, body, reply_to, timestamp)), @@ -352,7 +353,7 @@ mod tests { } /// Create a new comment. - pub fn comment(&mut self, body: &str, reply_to: Option) -> Op { + pub fn comment(&mut self, body: &str, reply_to: Option) -> Op { self.op(Action::Comment { body: String::from(body), reply_to, @@ -360,12 +361,12 @@ mod tests { } /// Create a new redaction. - pub fn redact(&mut self, id: OpId) -> Op { + pub fn redact(&mut self, id: CommentId) -> Op { self.op(Action::Redact { id }) } /// Edit a comment. - pub fn edit(&mut self, id: OpId, body: &str) -> Op { + pub fn edit(&mut self, id: CommentId, body: &str) -> Op { self.op(Action::Edit { id, body: body.to_owned(), @@ -373,7 +374,7 @@ mod tests { } /// React to a comment. - pub fn react(&mut self, to: OpId, reaction: Reaction, active: bool) -> Op { + pub fn react(&mut self, to: CommentId, reaction: Reaction, active: bool) -> Op { self.op(Action::React { to, reaction, @@ -419,7 +420,7 @@ mod tests { let rng = fastrand::Rng::with_seed(u64::arbitrary(g)); let gen = WeightedGenerator::< (Lamport, Op), - (Actor, Lamport, BTreeSet), + (Actor, Lamport, BTreeSet), >::new(rng.clone()) .variant(3, |(actor, clock, comments), rng| { let comment = actor.comment(