From 06a92d52d9d8dbc61a2405eb5985c65982b08146 Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Mon, 27 Feb 2023 11:48:05 +0100 Subject: [PATCH] cob: Use hashes for operation ids It's fairly easy for a user to (by mistake or intentionally) create two operations with the same OpId. This patch makes it much less likely, and ensures that if the OpId is equal, it's because the operations are identical. --- 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 | 28 +- radicle-cli/src/commands/id.rs | 26 +- radicle-cli/tests/commands.rs | 15 +- radicle-cob/src/backend/git/change.rs | 34 +- radicle-cob/src/change/store.rs | 3 +- radicle-cob/src/change_graph/evaluation.rs | 5 +- radicle-cob/src/history/entry.rs | 33 +- radicle-cob/src/object/collaboration.rs | 2 +- .../src/object/collaboration/create.rs | 25 +- .../src/object/collaboration/update.rs | 26 +- radicle-cob/src/tests.rs | 4 +- radicle-crypto/src/lib.rs | 22 +- radicle-httpd/src/api/v1/projects.rs | 66 +-- radicle-httpd/src/test.rs | 4 +- radicle/Cargo.toml | 1 + radicle/src/cob.rs | 6 +- radicle/src/cob/common.rs | 1 + radicle/src/cob/identity.rs | 92 +++-- radicle/src/cob/issue.rs | 56 ++- radicle/src/cob/op.rs | 212 ++++------ radicle/src/cob/patch.rs | 146 ++++--- radicle/src/cob/store.rs | 87 +++- radicle/src/cob/test.rs | 86 +++- radicle/src/cob/thread.rs | 391 ++++++++++-------- radicle/src/lib.rs | 4 + 29 files changed, 865 insertions(+), 579 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e597384..2e1aa738 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1807,6 +1807,7 @@ 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 740e0d8c..217a5211 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 '57332790a2eabc0b2fd8c7ff48c3579d5812d405' created 🌱 +βœ“ Identity proposal '16d2b7c47bb9615da1a72e67f66f0e1d345be2e3' 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 'c3698d4e85f9d4c0ee536b34d6122fc7c81f7e2e' created 🌱 +βœ“ Identity proposal '9615d03e4d98cf413994b6fdadf170747064c23d' 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 57332790a2eabc0b2fd8c7ff48c3579d5812d405 --rev z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1 --no-confirm +$ rad id accept 16d2b7c47bb9615da1a72e67f66f0e1d345be2e3 --no-confirm βœ“ Accepted proposal βœ“ title: Add Alice description: Add Alice as a delegate @@ -137,7 +137,7 @@ Quorum Reached ``` ``` -$ rad id commit 57332790a2eabc0b2fd8c7ff48c3579d5812d405 --rev z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1 --no-confirm +$ rad id commit 16d2b7c47bb9615da1a72e67f66f0e1d345be2e3 --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 c3698d4e85f9d4c0ee536b34d6122fc7c81f7e2e --rev z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1 --no-confirm +$ rad id accept 9615d03e4d98cf413994b6fdadf170747064c23d --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 c3698d4e85f9d4c0ee536b34d6122fc7c81f7e2e --rev z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1 --no-confirm +$ rad id commit 9615d03e4d98cf413994b6fdadf170747064c23d --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 'z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1' +βœ— Id failed: the identity hashes do match 'd96f425412c9f8ad5d9a9a05c9831d0728e2338d =/= 475cdfbc8662853dd132ec564e4f5eb0f152dd7f' for the revision '6877dc63e001e7f7fcb285f5f530948b3d96b488' ``` So, let's fix this by running a rebase on the proposal's revision: ``` -$ rad id rebase c3698d4e85f9d4c0ee536b34d6122fc7c81f7e2e --rev z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1 --no-confirm -βœ“ Identity proposal 'c3698d4e85f9d4c0ee536b34d6122fc7c81f7e2e' rebased 🌱 -βœ“ Revision 'z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/4' +$ rad id rebase 9615d03e4d98cf413994b6fdadf170747064c23d --no-confirm +βœ“ Identity proposal '9615d03e4d98cf413994b6fdadf170747064c23d' rebased 🌱 +βœ“ Revision 'aaa890c3531f880c9901b162ab38016ceb559c9f' 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 c3698d4e85f9d4c0ee536b34d6122fc7c81f7e2e --rev z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/4 --delegates did:key:z6MkedTZGJGqgQ2py2b8kGecfxdt2yRdHWF6JpaZC47fovFn --no-confirm -βœ“ Identity proposal 'c3698d4e85f9d4c0ee536b34d6122fc7c81f7e2e' updated 🌱 -βœ“ Revision 'z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/6' +$ rad id update 9615d03e4d98cf413994b6fdadf170747064c23d --rev aaa890c3531f880c9901b162ab38016ceb559c9f --delegates did:key:z6MkedTZGJGqgQ2py2b8kGecfxdt2yRdHWF6JpaZC47fovFn --no-confirm +βœ“ Identity proposal '9615d03e4d98cf413994b6fdadf170747064c23d' updated 🌱 +βœ“ Revision '24ad4a6ce84b1ce4b8cc754494c23f1079020a14' 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 c3698d4e85f9d4c0ee536b34d6122fc7c81f7e2e --revisions +$ rad id show 9615d03e4d98cf413994b6fdadf170747064c23d --revisions ``` -$ rad id accept c3698d4e85f9d4c0ee536b34d6122fc7c81f7e2e --rev z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/6 --no-confirm +$ rad id accept 9615d03e4d98cf413994b6fdadf170747064c23d --rev 24ad4a6ce84b1ce4b8cc754494c23f1079020a14 --no-confirm βœ“ Accepted proposal βœ“ title: Add Bob description: Add Bob as a delegate @@ -385,7 +385,7 @@ Quorum Reached ``` ``` -$ rad id commit c3698d4e85f9d4c0ee536b34d6122fc7c81f7e2e --rev z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/6 --no-confirm +$ rad id commit 9615d03e4d98cf413994b6fdadf170747064c23d --rev 24ad4a6ce84b1ce4b8cc754494c23f1079020a14 --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 cd802d4c..dfeaa973 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 '06d9efa2a9aad06bfdf25a25690e1ec7db2c3c39' created 🌱 +βœ“ Identity proposal 'de4102c1b9b9b83683d7d9ca80c79ffebd62ac83' 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 06d9efa2a9aad06bfdf25a25690e1ec7db2c3c39 --rev z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1 --no-confirm +$ rad id reject de4102c1b9b9b83683d7d9ca80c79ffebd62ac83 --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 06d9efa2a9aad06bfdf25a25690e1ec7db2c3c39 --rev z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1 --no-confirm +$ rad id accept de4102c1b9b9b83683d7d9ca80c79ffebd62ac83 --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 06d9efa2a9aad06bfdf25a25690e1ec7db2c3c39 --rev z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1 --no-confirm +$ rad id commit de4102c1b9b9b83683d7d9ca80c79ffebd62ac83 --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 'dc00640d3152ea5f1df59f39f2f5983d2ad21810' created 🌱 +βœ“ Identity proposal '14a980c4061f06433ace03cf6b1e5eedba4f8cfc' 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 dc00640d3152ea5f1df59f39f2f5983d2ad21810 --no-confirm -βœ“ Closed identity proposal 'dc00640d3152ea5f1df59f39f2f5983d2ad21810' +$ rad id close 14a980c4061f06433ace03cf6b1e5eedba4f8cfc --no-confirm +βœ“ Closed identity proposal '14a980c4061f06433ace03cf6b1e5eedba4f8cfc' 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 -06d9efa2a9aad06bfdf25a25690e1ec7db2c3c39 "Add Bob" ❲committed❳ -dc00640d3152ea5f1df59f39f2f5983d2ad21810 "Update threshold" ❲closed❳ +14a980c4061f06433ace03cf6b1e5eedba4f8cfc "Update threshold" ❲closed❳ +de4102c1b9b9b83683d7d9ca80c79ffebd62ac83 "Add Bob" ❲committed❳ ``` And if we want to view the latest state of any proposal we can use the show command: ``` -$ rad id show dc00640d3152ea5f1df59f39f2f5983d2ad21810 +$ rad id show 14a980c4061f06433ace03cf6b1e5eedba4f8cfc 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 15986bae..ace95630 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 -2b4650e3c66d568132034de0d02871a2fbf9c5b5 "flux capacitor underpowered" +e379d630f91a6082d3c7677467eb0d4875635d74 "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 2b4650e3c66d568132034de0d02871a2fbf9c5b5 did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi +$ rad assign e379d630f91a6082d3c7677467eb0d4875635d74 did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi ``` It will now show in the list of issues assigned to us. ``` $ rad issue list --assigned -2b4650e3c66d568132034de0d02871a2fbf9c5b5 "flux capacitor underpowered" did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi +e379d630f91a6082d3c7677467eb0d4875635d74 "flux capacitor underpowered" did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi ``` Note: this can always be undone with the `unassign` subcommand. ``` -$ rad unassign 2b4650e3c66d568132034de0d02871a2fbf9c5b5 did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi +$ rad unassign e379d630f91a6082d3c7677467eb0d4875635d74 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 2b4650e3c66d568132034de0d02871a2fbf9c5b5 --message 'The flux capacitor needs 1.21 Gigawatts' -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/7 -$ rad comment 2b4650e3c66d568132034de0d02871a2fbf9c5b5 --reply-to z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/7 --message 'More power!' -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/8 +$ rad comment e379d630f91a6082d3c7677467eb0d4875635d74 --message 'The flux capacitor needs 1.21 Gigawatts' +f1895792f7b1b56590aa21e34454bde74d04649a +$ rad comment e379d630f91a6082d3c7677467eb0d4875635d74 --reply-to f1895792f7b1b56590aa21e34454bde74d04649a --message 'More power!' +0bf5f874c57ac0a5cc010a9895dd0fec9edc4f3d ``` diff --git a/radicle-cli/examples/rad-patch.md b/radicle-cli/examples/rad-patch.md index e414e5f1..c0351ead 100644 --- a/radicle-cli/examples/rad-patch.md +++ b/radicle-cli/examples/rad-patch.md @@ -45,7 +45,7 @@ No description provided. ╰─────────────────────────────────── -βœ“ Patch d4ef85f57a849bd845915d7a66a2192cd23811f6 created 🌱 +βœ“ Patch fd1df2db86867aa859541464fa334d0b22988ea7 created 🌱 ``` It will now be listed as one of the project's open patches. @@ -55,17 +55,17 @@ $ rad patch ❲YOU PROPOSED❳ -define power requirements d4ef85f57a8 R0 3e674d1 (flux-capacitor-power) ahead 1, behind 0 +define power requirements fd1df2db868 R0 3e674d1 (flux-capacitor-power) ahead 1, behind 0 └─ * opened by did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (you) [..] -└─ * patch id d4ef85f57a849bd845915d7a66a2192cd23811f6 +└─ * patch id fd1df2db86867aa859541464fa334d0b22988ea7 ❲OTHERS PROPOSED❳ Nothing to show. -$ rad patch show d4ef85f57a849bd845915d7a66a2192cd23811f6 +$ rad patch show fd1df2db86867aa859541464fa334d0b22988ea7 -patch d4ef85f57a849bd845915d7a66a2192cd23811f6 +patch fd1df2db86867aa859541464fa334d0b22988ea7 ╭─ define power requirements ─────── @@ -94,34 +94,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" --no-confirm d4ef85f57a849bd845915d7a66a2192cd23811f6 +$ rad patch update --message "Add README, just for the fun" --no-confirm fd1df2db86867aa859541464fa334d0b22988ea7 🌱 Updating patch for heartwood βœ“ Pushing HEAD to storage... βœ“ Analyzing remotes... -d4ef85f57a8 R0 (3e674d1) -> R1 (27857ec) +fd1df2db868 R0 (3e674d1) -> R1 (27857ec) 1 commit(s) ahead, 0 commit(s) behind -βœ“ Patch d4ef85f57a849bd845915d7a66a2192cd23811f6 updated 🌱 +βœ“ Patch fd1df2db86867aa859541464fa334d0b22988ea7 updated 🌱 ``` And lets leave a quick comment for our team: ``` -$ rad comment d4ef85f57a849bd845915d7a66a2192cd23811f6 --message 'I cannot wait to get back to the 90s!' -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/5 -$ rad comment d4ef85f57a849bd845915d7a66a2192cd23811f6 --message 'I cannot wait to get back to the 90s!' --reply-to z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/5 -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/6 +$ rad comment fd1df2db86867aa859541464fa334d0b22988ea7 --message 'I cannot wait to get back to the 90s!' +84ef44764de73695cf30e6b284585d2c50d6d0e5 +$ rad comment fd1df2db86867aa859541464fa334d0b22988ea7 --message 'I cannot wait to get back to the 90s!' --reply-to 84ef44764de73695cf30e6b284585d2c50d6d0e5 +2fa3ac18d82ebdafe73484a15fa9823355c4664b ``` Now, let's checkout the patch that we just created: ``` -$ rad patch checkout d4ef85f57a849bd845915d7a66a2192cd23811f6 +$ rad patch checkout fd1df2db86867aa859541464fa334d0b22988ea7 βœ“ Performing patch checkout... -βœ“ Switched to branch patch/d4ef85f57a8 +βœ“ Switched to branch patch/fd1df2db868 ``` diff --git a/radicle-cli/src/commands/id.rs b/radicle-cli/src/commands/id.rs index 55bf85f0..533b6ea2 100644 --- a/radicle-cli/src/commands/id.rs +++ b/radicle-cli/src/commands/id.rs @@ -468,12 +468,19 @@ fn select<'a>( previous: &Identity, interactive: &Interactive, ) -> anyhow::Result<(RevisionId, &'a identity::Revision)> { - let (id, revision) = match id { - None => { + let (id, revision) = match (id, interactive) { + (None, Interactive::Yes) => { let (id, revision) = term::proposal::revision_select(proposal).unwrap(); (*id, revision) } - Some(id) => { + (None, Interactive::No) => { + let (id, revision) = proposal + .revisions() + .next() + .ok_or(anyhow!("No revisions found!"))?; + (*id, revision) + } + (Some(id), _) => { let revision = proposal .revision(&id) .context(format!("No revision found for {id}"))? @@ -494,13 +501,20 @@ fn commit_select<'a>( previous: &'a Identity, interactive: &Interactive, ) -> anyhow::Result<(RevisionId, &'a identity::Revision)> { - let (id, revision) = match id { - None => { + let (id, revision) = match (id, interactive) { + (None, Interactive::Yes) => { let (id, revision) = term::proposal::revision_commit_select(proposal, previous).unwrap(); (*id, revision) } - Some(id) => { + (None, Interactive::No) => { + let (id, revision) = proposal + .revisions() + .find(|(_, r)| r.is_quorum_reached(previous)) + .ok_or(anyhow!("No revisions with quorum found"))?; + (*id, revision) + } + (Some(id), _) => { let revision = proposal .revision(&id) .context(format!("No revision found for {id}"))? diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index 303959ac..14ecbc59 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -16,6 +16,9 @@ use radicle_node::test::{ logger, }; +/// Seed used in tests. +const RAD_SEED: &str = "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; + /// Run a CLI test file. fn test<'a>( test: impl AsRef, @@ -40,6 +43,7 @@ fn test<'a>( .env("GIT_COMMITTER_NAME", "radicle") .env("RAD_HOME", home.to_string_lossy()) .env("RAD_PASSPHRASE", "radicle") + .env("RAD_SEED", RAD_SEED) .env("TZ", "UTC") .env("LANG", "C") .env(radicle_cob::git::RAD_COMMIT_TIME, "1671125284") @@ -54,16 +58,7 @@ fn test<'a>( #[test] fn rad_auth() { - test( - "examples/rad-auth.md", - Path::new("."), - None, - [( - "RAD_SEED", - "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", - )], - ) - .unwrap(); + test("examples/rad-auth.md", Path::new("."), None, []).unwrap(); } #[test] diff --git a/radicle-cob/src/backend/git/change.rs b/radicle-cob/src/backend/git/change.rs index 15e384c4..37b7addb 100644 --- a/radicle-cob/src/backend/git/change.rs +++ b/radicle-cob/src/backend/git/change.rs @@ -110,8 +110,16 @@ impl change::Storage for git2::Repository { history_type, }; - let revision = write_manifest(self, &manifest, &contents)?; + let (revision, blob_oids) = 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()); @@ -205,10 +213,13 @@ fn load_contents( entry.kind().and_then(|kind| match kind { git2::ObjectType::Blob => { let name = entry.name()?.parse::().ok()?; - let content = entry.to_object(repo).and_then(|object| { - object.peel_to_blob().map(|blob| blob.content().to_owned()) - }); - Some(content.map(|c| (name, c))) + let blob = entry + .to_object(repo) + .and_then(|object| object.peel_to_blob()) + .map(entry::EntryBlob::from) + .map(|b| (name, b)); + + Some(blob) } _ => None, }) @@ -279,8 +290,8 @@ where fn write_manifest( repo: &git2::Repository, manifest: &store::Manifest, - contents: &entry::Contents, -) -> Result { + contents: &NonEmpty>, +) -> Result<(git2::Oid, Vec), git2::Error> { 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 @@ -292,10 +303,13 @@ fn write_manifest( git2::FileMode::Blob.into(), )?; + let mut blob_oids = Vec::new(); for (ix, op) in contents.iter().enumerate() { - let change_blob = repo.blob(op.as_ref())?; - tb.insert(&ix.to_string(), change_blob, git2::FileMode::Blob.into())?; + 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()?; - tb.write() + Ok((tree_oid, blob_oids)) } diff --git a/radicle-cob/src/change/store.rs b/radicle-cob/src/change/store.rs index 08248659..85578cf9 100644 --- a/radicle-cob/src/change/store.rs +++ b/radicle-cob/src/change/store.rs @@ -5,6 +5,7 @@ use std::{error::Error, fmt}; +use nonempty::NonEmpty; use serde::{Deserialize, Serialize}; use crate::{ @@ -46,7 +47,7 @@ pub struct Template { pub history_type: String, pub tips: Vec, pub message: String, - pub contents: Contents, + pub contents: NonEmpty>, } #[derive(Clone, Debug)] diff --git a/radicle-cob/src/change_graph/evaluation.rs b/radicle-cob/src/change_graph/evaluation.rs index 6b006fd0..e494178a 100644 --- a/radicle-cob/src/change_graph/evaluation.rs +++ b/radicle-cob/src/change_graph/evaluation.rs @@ -9,7 +9,6 @@ use git_ext::Oid; use radicle_dag::Dag; use crate::history::entry::{EntryId, EntryWithClock}; -use crate::history::Clock; use crate::{change::Change, history, pruning_fold}; /// # Panics @@ -43,9 +42,9 @@ pub fn evaluate(root: Oid, graph: &Dag, rng: fastrand::Rng) -> hist .iter() .map(|e| { let entry = &entries[&EntryId::from(*e)]; - let clock = entry.clock(); + let clock = entry.range(); - clock + entry.contents().len() as Clock - 1 + *clock.end() }) .max() .unwrap_or_default() // When there are no operations, the clock is zero. diff --git a/radicle-cob/src/history/entry.rs b/radicle-cob/src/history/entry.rs index bb86717e..7fc597da 100644 --- a/radicle-cob/src/history/entry.rs +++ b/radicle-cob/src/history/entry.rs @@ -9,9 +9,27 @@ use radicle_crypto::PublicKey; 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; @@ -146,6 +164,19 @@ impl EntryWithClock { pub fn clock(&self) -> Clock { 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)) + } } impl pruning_fold::GraphNode for EntryWithClock { diff --git a/radicle-cob/src/object/collaboration.rs b/radicle-cob/src/object/collaboration.rs index ffc400e5..2983f963 100644 --- a/radicle-cob/src/object/collaboration.rs +++ b/radicle-cob/src/object/collaboration.rs @@ -8,7 +8,7 @@ use std::collections::BTreeSet; use git_ext::Oid; use crate::change::store::Manifest; -use crate::{change, identity::Identity, Contents, History, ObjectId, TypeName}; +use crate::{change, identity::Identity, History, ObjectId, TypeName}; pub mod error; diff --git a/radicle-cob/src/object/collaboration/create.rs b/radicle-cob/src/object/collaboration/create.rs index 58c047b1..966210bb 100644 --- a/radicle-cob/src/object/collaboration/create.rs +++ b/radicle-cob/src/object/collaboration/create.rs @@ -3,6 +3,8 @@ // This file is part of radicle-link, distributed under the GPLv3 with Radicle // Linking Exception. For full terms see the included LICENSE file. +use nonempty::NonEmpty; + use crate::Store; use super::*; @@ -12,7 +14,7 @@ pub struct Create { /// The type of history that will be used for this object. pub history_type: String, /// The CRDT history to initialize this object with. - pub contents: Contents, + pub contents: NonEmpty>, /// The typename for this object. pub typename: TypeName, /// The message to add when creating this object. @@ -61,35 +63,30 @@ where G: crypto::Signer, Resource: Identity, { - let Create { - ref contents, - ref typename, - .. - } = &args; - + let Create { ref typename, .. } = &args; let init_change = storage .store(resource.content_id(), signer, args.template()) .map_err(error::Create::from)?; + let object_id = init_change.id().into(); + + storage + .update(identifier, typename, &object_id, &init_change) + .map_err(|err| error::Create::Refs { err: Box::new(err) })?; let history = History::new_from_root( *init_change.id(), init_change.signature.key, resource.content_id(), - contents.clone(), + init_change.contents, init_change.timestamp, ); - let object_id = init_change.id().into(); - storage - .update(identifier, typename, &object_id, &init_change) - .map_err(|err| error::Create::Refs { err: Box::new(err) })?; - Ok(CollaborativeObject { manifest: Manifest { typename: args.typename, history_type: args.history_type, }, history, - id: init_change.id().into(), + id: object_id, }) } diff --git a/radicle-cob/src/object/collaboration/update.rs b/radicle-cob/src/object/collaboration/update.rs index 1eea666c..c504092b 100644 --- a/radicle-cob/src/object/collaboration/update.rs +++ b/radicle-cob/src/object/collaboration/update.rs @@ -3,9 +3,11 @@ // This file is part of radicle-link, distributed under the GPLv3 with Radicle // Linking Exception. For full terms see the included LICENSE file. +use nonempty::NonEmpty; + use crate::{ - change, change_graph::ChangeGraph, identity::Identity, CollaborativeObject, Contents, ObjectId, - Store, TypeName, + change, change_graph::ChangeGraph, identity::Identity, CollaborativeObject, ObjectId, Store, + TypeName, }; use super::error; @@ -15,7 +17,7 @@ pub struct Update { /// The type of history that will be used for this object. pub history_type: String, /// The CRDT changes to add to the object. - pub changes: Contents, + pub changes: NonEmpty>, /// The object ID of the object to be updated. pub object_id: ObjectId, /// The typename of the object to be updated. @@ -76,21 +78,23 @@ where change::Template { tips: object.tips().iter().cloned().collect(), history_type, - contents: changes.clone(), + contents: changes, typename: typename.clone(), message, }, )?; - object.history.extend( - change.id, - change.signature.key, - change.resource, - changes, - change.timestamp, - ); + storage .update(identifier, typename, &object_id, &change) .map_err(|err| error::Update::Refs { err: Box::new(err) })?; + object.history.extend( + change.id, + change.signature.key, + change.resource, + change.contents, + change.timestamp, + ); + Ok(object) } diff --git a/radicle-cob/src/tests.rs b/radicle-cob/src/tests.rs index 8aa72516..e2a244f8 100644 --- a/radicle-cob/src/tests.rs +++ b/radicle-cob/src/tests.rs @@ -201,7 +201,7 @@ fn traverse_cobs() { // traverse over the history and filter by changes that were only authorized by terry let contents = updated.history().traverse(Vec::new(), |mut acc, entry| { if entry.actor() == terry_signer.public_key() { - acc.push(entry.contents().head.to_vec()); + acc.push(entry.contents().head.data.clone()); } ControlFlow::Continue(acc) }); @@ -210,7 +210,7 @@ fn traverse_cobs() { // 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.to_vec()); + acc.push(entry.contents().head.data.clone()); ControlFlow::Continue(acc) }); diff --git a/radicle-crypto/src/lib.rs b/radicle-crypto/src/lib.rs index f6cbbfcb..d050c757 100644 --- a/radicle-crypto/src/lib.rs +++ b/radicle-crypto/src/lib.rs @@ -447,22 +447,34 @@ pub mod keypair { /// Generate a new keypair using OS randomness. pub fn generate() -> KeyPair { #[cfg(debug_assertions)] - if let Ok(seed) = std::env::var("RAD_SEED") { + if let Some(seed) = env::seed() { // Generate a keypair based on the given environment variable. // This is useful for debugging and testing, since the // public key can be known in advance. + return KeyPair::from_seed(Seed::new(seed)); + } + KeyPair::generate() + } +} + +pub mod env { + use std::env; + + /// Return the seed stored in the `RAD_SEED` environment variable, if any. + pub fn seed() -> Option<[u8; 32]> { + if let Ok(seed) = env::var("RAD_SEED") { let seed = (0..seed.len()) .step_by(2) .map(|i| u8::from_str_radix(&seed[i..i + 2], 16)) .collect::, _>>() - .expect("generate: invalid hexadecimal value set in `RAD_SEED`"); + .expect("env::seed: invalid hexadecimal value set in `RAD_SEED`"); let seed: [u8; 32] = seed .try_into() - .expect("generate: invalid seed length set in `RAD_SEED`"); + .expect("env::seed: invalid seed length set in `RAD_SEED`"); - return KeyPair::from_seed(Seed::new(seed)); + return Some(seed); } - KeyPair::generate() + None } } diff --git a/radicle-httpd/src/api/v1/projects.rs b/radicle-httpd/src/api/v1/projects.rs index 4940015b..740a36ef 100644 --- a/radicle-httpd/src/api/v1/projects.rs +++ b/radicle-httpd/src/api/v1/projects.rs @@ -454,13 +454,18 @@ async fn issue_update_handler( Path((project, issue_id)): Path<(Id, Oid)>, Json(action): Json, ) -> impl IntoResponse { - let sessions = ctx.sessions.write().await; - sessions.get(&token).ok_or(Error::Auth("Unauthorized"))?; + ctx.sessions + .write() + .await + .get(&token) + .ok_or(Error::Auth("Unauthorized"))?; + let storage = &ctx.profile.storage; let signer = ctx.profile.signer().unwrap(); let repo = storage.repository(project)?; let mut issues = Issues::open(ctx.profile.public_key, &repo)?; let mut issue = issues.get_mut(&issue_id.into())?; + match action { Action::Assign { add, remove } => { issue.assign(add, &signer)?; @@ -475,27 +480,24 @@ async fn issue_update_handler( Action::Edit { title } => { issue.edit(title, &signer)?; } - Action::Thread { action } => { - let mut actor = thread::Actor::new(ctx.profile.signer().unwrap()); - match action { - thread::Action::Comment { body, reply_to } => { - if let Some(reply_to) = reply_to { - issue.comment(body, reply_to, &signer)?; - } else { - issue.thread(body, &signer)?; - } - } - thread::Action::React { to, reaction, .. } => { - issue.react(to, reaction, &signer)?; - } - thread::Action::Edit { id, body } => { - actor.edit(id, &body); - } - thread::Action::Redact { id } => { - actor.redact(id); + Action::Thread { action } => match action { + thread::Action::Comment { body, reply_to } => { + if let Some(reply_to) = reply_to { + issue.comment(body, reply_to, &signer)?; + } else { + issue.thread(body, &signer)?; } } - } + thread::Action::React { to, reaction, .. } => { + issue.react(to, reaction, &signer)?; + } + thread::Action::Edit { .. } => { + todo!(); + } + thread::Action::Redact { .. } => { + todo!(); + } + }, }; Ok::<_, Error>(Json(json!({ "success": true }))) @@ -565,7 +567,7 @@ mod routes { use crate::test::{self, get, patch, post, HEAD, HEAD_1, ISSUE_ID, PATCH_ID}; - const CREATED_ISSUE_ID: &str = "b56febfba1e7dd20f4aea43ca2fe9dcf1fd448ba"; + const CREATED_ISSUE_ID: &str = "745052a1603000b9566445753d7e2fee1ff5041f"; #[tokio::test] async fn test_projects_root() { @@ -1045,7 +1047,7 @@ mod routes { "assignees": [], "discussion": [ { - "id": "z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1", + "id": "f0afe34f5bf4248df432f6b6a8818bcae360bbc2", "author": { "id": "did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi" }, @@ -1107,7 +1109,7 @@ mod routes { "status": "open", }, "discussion": [{ - "id": "z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1", + "id": "f0afe34f5bf4248df432f6b6a8818bcae360bbc2", "author": { "id": "did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi", }, @@ -1168,7 +1170,7 @@ mod routes { }, "discussion": [ { - "id": "z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1", + "id": "f0afe34f5bf4248df432f6b6a8818bcae360bbc2", "author": { "id": "did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi", }, @@ -1178,7 +1180,7 @@ mod routes { "replyTo": null, }, { - "id": "z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/5", + "id": "f7da49e705f60c39265dbdd748d786a620bc8030", "author": { "id": "did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi", }, @@ -1204,7 +1206,7 @@ mod routes { "action": { "type": "comment", "body": "This is a reply to the first comment", - "replyTo": "z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1", + "replyTo": "f0afe34f5bf4248df432f6b6a8818bcae360bbc2", }})) .unwrap(); let response = patch( @@ -1238,7 +1240,7 @@ mod routes { }, "discussion": [ { - "id": "z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1", + "id": "f0afe34f5bf4248df432f6b6a8818bcae360bbc2", "author": { "id": "did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi", }, @@ -1248,14 +1250,14 @@ mod routes { "replyTo": null, }, { - "id": "z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/5", + "id": "ab98b5b794d02c23d29769a39fe8e0b74624f3d8", "author": { "id": "did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi", }, "body": "This is a reply to the first comment", "reactions": [], "timestamp": 1673001014, - "replyTo": "z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1", + "replyTo": "f0afe34f5bf4248df432f6b6a8818bcae360bbc2", }, ], "tags": [], @@ -1285,7 +1287,7 @@ mod routes { "tags": [], "revisions": [ { - "id": "z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1", + "id": "d6ba305f78e2fa1ebcc55d8c3be8806bbce25fb4", "description": "", "reviews": [], } @@ -1316,7 +1318,7 @@ mod routes { "tags": [], "revisions": [ { - "id": "z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/1", + "id": "d6ba305f78e2fa1ebcc55d8c3be8806bbce25fb4", "description": "", "reviews": [], } diff --git a/radicle-httpd/src/test.rs b/radicle-httpd/src/test.rs index d3f9c948..ce12a261 100644 --- a/radicle-httpd/src/test.rs +++ b/radicle-httpd/src/test.rs @@ -23,8 +23,8 @@ use crate::api::{auth, Context}; pub const HEAD: &str = "1e978d19f251cd9821d9d9a76d1bd436bf0690d5"; pub const HEAD_1: &str = "f604ce9fd5b7cc77b7609beda45ea8760bee78f7"; -pub const PATCH_ID: &str = "4250f0117659ee4de9af99e699a63395cd6ffa1c"; -pub const ISSUE_ID: &str = "8adca8aad2a2cb99b9847d20193930cde2042a57"; +pub const PATCH_ID: &str = "afb3063f8f0343fa31d2a0d55bac2a6f4a77125e"; +pub const ISSUE_ID: &str = "331569cd5e4dcc55104363ebce92c78b0e5d67d4"; const PASSWORD: &str = "radicle"; diff --git a/radicle/Cargo.toml b/radicle/Cargo.toml index 19bbf4ee..973f2e25 100644 --- a/radicle/Cargo.toml +++ b/radicle/Cargo.toml @@ -24,6 +24,7 @@ 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 ef27923a..68af6b0a 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::{ - identity::Identity, object::collaboration::error, CollaborativeObject, Contents, Create, Entry, - History, ObjectId, TypeName, Update, + history::entry::EntryBlob, identity::Identity, object::collaboration::error, + CollaborativeObject, Contents, Create, Entry, History, ObjectId, TypeName, Update, }; pub use common::*; -pub use op::{Actor, ActorId, Op, OpId}; +pub use op::{ActorId, Op, OpId}; use radicle_cob as cob; diff --git a/radicle/src/cob/common.rs b/radicle/src/cob/common.rs index dec49191..88c79b93 100644 --- a/radicle/src/cob/common.rs +++ b/radicle/src/cob/common.rs @@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize}; use crate::prelude::*; +pub use radicle_crdt::clock; pub use radicle_crdt::clock::Physical as Timestamp; /// Author. diff --git a/radicle/src/cob/identity.rs b/radicle/src/cob/identity.rs index 43832ef9..a289563f 100644 --- a/radicle/src/cob/identity.rs +++ b/radicle/src/cob/identity.rs @@ -3,7 +3,7 @@ use std::{ops::Deref, str::FromStr}; use crypto::{PublicKey, Signature}; use once_cell::sync::Lazy; use radicle_cob::{ObjectId, TypeName}; -use radicle_crdt::{clock, GMap, LWWMap, LWWReg, Max, Redactable, Semilattice}; +use radicle_crdt::{clock, GMap, GSet, LWWMap, LWWReg, Max, Redactable, Semilattice}; use radicle_crypto::{Signer, Verified}; use radicle_git_ext::Oid; use serde::{Deserialize, Serialize}; @@ -144,6 +144,8 @@ pub struct Proposal { state: LWWReg>, /// List of revisions for this proposal. revisions: GMap>, + /// Timeline of events. + timeline: GSet<(clock::Lamport, OpId)>, } #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] @@ -169,6 +171,7 @@ impl Default for Proposal { description: Max::from(String::default()).into(), state: Max::from(State::default()).into(), revisions: GMap::default(), + timeline: GSet::default(), } } } @@ -270,11 +273,12 @@ impl Proposal { /// All the [`Revision`]s that have not been redacted. pub fn revisions(&self) -> impl DoubleEndedIterator { - self.revisions - .iter() - .filter_map(|(rid, r)| -> Option<(&RevisionId, &Revision)> { - r.get().map(|r| (rid, r)) - }) + self.timeline.iter().filter_map(|(_, id)| { + self.revisions + .get(id) + .and_then(Redactable::get) + .map(|rev| (id, rev)) + }) } pub fn latest_by(&self, who: &Did) -> Option<(&RevisionId, &Revision)> { @@ -302,10 +306,12 @@ impl store::FromHistory for Proposal { fn apply(&mut self, ops: impl IntoIterator) -> Result<(), Self::Error> { for op in ops { - let id = op.id(); + let id = op.id; let author = Author::new(op.author); let timestamp = op.timestamp; + self.timeline.insert((op.clock, id)); + match op.action { Action::Accept { revision, @@ -335,14 +341,30 @@ impl store::FromHistory for Proposal { Some(Redactable::Redacted) => return Err(ApplyError::Redacted(revision)), None => return Err(ApplyError::Missing(revision)), }, - Action::Revision { current, proposed } => self.revisions.insert( - id, - Redactable::Present(Revision::new(author, current, proposed, timestamp)), - ), + Action::Revision { current, proposed } => { + // Since revisions are keyed by content hash, we shouldn't re-insert a revision + // if it already exists, otherwise this will be resolved via the `merge` + // operation of `Redactable`. + if self.revisions.contains_key(&id) { + continue; + } + self.revisions.insert( + id, + Redactable::Present(Revision::new(author, current, proposed, timestamp)), + ) + } + Action::Thread { revision, action } => match self.revisions.get_mut(&revision) { - Some(Redactable::Present(revision)) => revision - .discussion - .apply([cob::Op::new(action, op.author, op.timestamp, op.clock)])?, + Some(Redactable::Present(revision)) => { + revision.discussion.apply([cob::Op::new( + op.id, + action, + op.nonce, + op.author, + op.timestamp, + op.clock, + )])? + } Some(Redactable::Redacted) => return Err(ApplyError::Redacted(revision)), None => return Err(ApplyError::Missing(revision)), }, @@ -451,34 +473,50 @@ impl Revision { } impl store::Transaction { - pub fn accept(&mut self, revision: RevisionId, signature: Signature) -> OpId { + pub fn accept( + &mut self, + revision: RevisionId, + signature: Signature, + ) -> Result { self.push(Action::Accept { revision, signature, }) } - pub fn reject(&mut self, revision: RevisionId) -> OpId { + pub fn reject(&mut self, revision: RevisionId) -> Result { self.push(Action::Reject { revision }) } - pub fn edit(&mut self, title: impl ToString, description: impl ToString) -> OpId { + pub fn edit( + &mut self, + title: impl ToString, + description: impl ToString, + ) -> Result { self.push(Action::Edit { title: title.to_string(), description: description.to_string(), }) } - pub fn redact(&mut self, revision: RevisionId) -> OpId { + pub fn redact(&mut self, revision: RevisionId) -> Result { self.push(Action::Redact { revision }) } - pub fn revision(&mut self, current: Oid, proposed: Doc) -> OpId { + pub fn revision( + &mut self, + current: Oid, + proposed: Doc, + ) -> Result { self.push(Action::Revision { current, proposed }) } /// Start a proposal revision discussion. - pub fn thread(&mut self, revision: RevisionId, body: S) -> OpId { + pub fn thread( + &mut self, + revision: RevisionId, + body: S, + ) -> Result { self.push(Action::Thread { revision, action: thread::Action::Comment { @@ -494,7 +532,7 @@ impl store::Transaction { revision: RevisionId, body: S, reply_to: thread::CommentId, - ) -> OpId { + ) -> Result { self.push(Action::Thread { revision, action: thread::Action::Comment { @@ -536,10 +574,10 @@ impl<'a, 'g> ProposalMut<'a, 'g> { ) -> Result where G: Signer, - F: FnOnce(&mut Transaction) -> T, + F: FnOnce(&mut Transaction) -> Result, { - let mut tx = Transaction::new(*signer.public_key(), self.clock); - let output = operations(&mut tx); + 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)?; self.proposal.apply(ops)?; @@ -654,8 +692,10 @@ impl<'a> Proposals<'a> { ) -> Result, Error> { let (id, proposal, clock) = Transaction::initial("Create proposal", &mut self.raw, signer, |tx| { - tx.revision(current.into(), proposed); - tx.edit(title, description); + tx.revision(current.into(), proposed)?; + tx.edit(title, description)?; + + Ok(()) })?; // Just a sanity check that our clock is advancing as expected. debug_assert_eq!(clock.get(), 2); diff --git a/radicle/src/cob/issue.rs b/radicle/src/cob/issue.rs index 5f724015..329d957e 100644 --- a/radicle/src/cob/issue.rs +++ b/radicle/src/cob/issue.rs @@ -143,8 +143,14 @@ impl store::FromHistory for Issue { } } Action::Thread { action } => { - self.thread - .apply([cob::Op::new(action, op.author, op.timestamp, op.clock)])?; + self.thread.apply([cob::Op::new( + op.id, + action, + op.nonce, + op.author, + op.timestamp, + op.clock, + )])?; } } } @@ -199,7 +205,7 @@ impl store::Transaction { &mut self, add: impl IntoIterator, remove: impl IntoIterator, - ) -> OpId { + ) -> Result { let add = add.into_iter().collect::>(); let remove = remove.into_iter().collect::>(); @@ -207,19 +213,19 @@ impl store::Transaction { } /// Set the issue title. - pub fn edit(&mut self, title: impl ToString) -> OpId { + pub fn edit(&mut self, title: impl ToString) -> Result { self.push(Action::Edit { title: title.to_string(), }) } /// Lifecycle an issue. - pub fn lifecycle(&mut self, state: State) -> OpId { + pub fn lifecycle(&mut self, state: State) -> Result { self.push(Action::Lifecycle { state }) } /// Create the issue thread. - pub fn thread(&mut self, body: S) -> CommentId { + pub fn thread(&mut self, body: S) -> Result { self.push(Action::from(thread::Action::Comment { body: body.to_string(), reply_to: None, @@ -227,7 +233,11 @@ impl store::Transaction { } /// Comment on an issue. - pub fn comment(&mut self, body: S, reply_to: CommentId) -> CommentId { + pub fn comment( + &mut self, + body: S, + reply_to: CommentId, + ) -> Result { self.push(Action::from(thread::Action::Comment { body: body.to_string(), reply_to: Some(reply_to), @@ -239,7 +249,7 @@ impl store::Transaction { &mut self, add: impl IntoIterator, remove: impl IntoIterator, - ) -> OpId { + ) -> Result { let add = add.into_iter().collect::>(); let remove = remove.into_iter().collect::>(); @@ -247,7 +257,7 @@ impl store::Transaction { } /// React to an issue comment. - pub fn react(&mut self, to: CommentId, reaction: Reaction) -> OpId { + pub fn react(&mut self, to: CommentId, reaction: Reaction) -> Result { self.push(Action::Thread { action: thread::Action::React { to, @@ -342,6 +352,7 @@ impl<'a, 'g> IssueMut<'a, 'g> { signer: &G, ) -> Result { self.transaction("Unassign", signer, |tx| tx.assign([], assignees)) + .map_err(Error::from) } pub fn transaction( @@ -352,10 +363,11 @@ impl<'a, 'g> IssueMut<'a, 'g> { ) -> Result where G: Signer, - F: FnOnce(&mut Transaction) -> T, + F: FnOnce(&mut Transaction) -> Result, { - let mut tx = Transaction::new(*signer.public_key(), self.clock); - let output = operations(&mut tx); + 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)?; self.issue.apply(ops)?; @@ -435,10 +447,12 @@ impl<'a> Issues<'a> { ) -> Result, Error> { let (id, issue, clock) = Transaction::initial("Create issue", &mut self.raw, signer, |tx| { - tx.thread(description); - tx.assign(assignees.to_owned(), []); - tx.edit(title); - tx.tag(tags.to_owned(), []); + tx.thread(description)?; + tx.assign(assignees.to_owned(), [])?; + tx.edit(title)?; + tx.tag(tags.to_owned(), [])?; + + Ok(()) })?; // Just a sanity check that our clock is advancing as expected. debug_assert_eq!(clock.get(), 4); @@ -700,7 +714,8 @@ mod test { .create("My first issue", "Blah blah blah.", &[], &[], &signer) .unwrap(); - let comment = OpId::initial(*signer.public_key()); + let (comment, _) = issue.root(); + let comment = *comment; let reaction = Reaction::new('πŸ₯³').unwrap(); issue.react(comment, reaction, &signer).unwrap(); @@ -717,12 +732,12 @@ mod test { fn test_issue_reply() { let tmp = tempfile::tempdir().unwrap(); let (_, signer, project) = test::setup::context(&tmp); - let author = *signer.public_key(); let mut issues = Issues::open(*signer.public_key(), &project).unwrap(); let mut issue = issues .create("My first issue", "Blah blah blah.", &[], &[], &signer) .unwrap(); - let root = OpId::root(author); + let (root, _) = issue.root(); + let root = *root; let c1 = issue.comment("Hi hi hi.", root, &signer).unwrap(); let c2 = issue.comment("Ha ha ha.", root, &signer).unwrap(); @@ -792,7 +807,8 @@ mod test { .unwrap(); // The root thread op id is always the same. - let c0 = OpId::root(author); + let (c0, _) = issue.root(); + let c0 = *c0; 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 ced7b46d..9b4cc518 100644 --- a/radicle/src/cob/op.rs +++ b/radicle/src/cob/op.rs @@ -1,4 +1,3 @@ -use std::collections::BTreeMap; use std::fmt; use std::str; use std::str::FromStr; @@ -10,37 +9,49 @@ use thiserror::Error; use radicle_cob::history::EntryWithClock; use radicle_crdt::clock; use radicle_crdt::clock::Lamport; -use radicle_crypto::{PublicKey, Signer}; +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(Lamport, ActorId); +pub struct OpId(git::Oid); impl OpId { /// Create a new operation id. - pub fn new(clock: Lamport, actor: ActorId) -> Self { - Self(clock, actor) - } - - /// Get the initial operation id for the given actor. - pub fn initial(actor: ActorId) -> Self { - 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 + 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.1, self.0) + 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()) } } @@ -53,7 +64,7 @@ impl From for String { // Used by `serde::Deserialize`. impl TryFrom for OpId { - type Error = OpIdError; + type Error = git::raw::Error; fn try_from(value: String) -> Result { value.as_str().try_into() @@ -70,7 +81,7 @@ pub enum OpIdError { } impl FromStr for OpId { - type Err = OpIdError; + type Err = git::raw::Error; fn from_str(s: &str) -> Result { Self::try_from(s) @@ -78,26 +89,19 @@ impl FromStr for OpId { } impl TryFrom<&str> for OpId { - type Error = OpIdError; + type Error = git::raw::Error; fn try_from(s: &str) -> Result { - if s.is_empty() { - return Err(OpIdError::Empty); - } - - let Some((actor_id, clock)) = s.split_once('/') else { - return Err(OpIdError::BadFormat); - }; - Ok(Self( - Lamport::from_str(clock).map_err(|_| OpIdError::BadFormat)?, - ActorId::from_str(actor_id).map_err(|_| OpIdError::BadFormat)?, - )) + 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 { @@ -107,14 +111,29 @@ 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, /// 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. @@ -125,30 +144,38 @@ pub struct Op { impl PartialOrd for Op { fn partial_cmp(&self, other: &Self) -> Option { - self.id().partial_cmp(&other.id()) + self.id.partial_cmp(&other.id) } } impl Ord for Op { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.id().cmp(&other.id()) + self.id.cmp(&other.id) } } -impl Op { +impl Op { pub fn new( + id: OpId, action: A, + nonce: Nonce, author: ActorId, timestamp: impl Into, clock: Lamport, ) -> Self { Self { + id, action, + nonce, author, clock, timestamp: timestamp.into(), } } + + pub fn id(&self) -> OpId { + self.id + } } pub struct Ops(pub NonEmpty>); @@ -160,109 +187,24 @@ where type Error = OpEncodingError; fn try_from(entry: &'a EntryWithClock) -> Result { - let mut clock = entry.clock().into(); - - entry - .contents() - .clone() - .try_map(|op| { - let action = serde_json::from_slice(&op)?; + let ops = entry + .changes() + .map(|(clock, blob)| { + let OpBlob { action, nonce } = serde_json::from_slice(blob.data.as_slice())?; let op = Op { + id: blob.oid.into(), action, + nonce, author: *entry.actor(), - clock, + clock: clock.into(), timestamp: entry.timestamp().into(), }; - clock.tick(); - - Ok(op) + Ok::<_, Self::Error>(op) }) - .map(Self) - } -} - -impl Op { - /// Get the op id. - /// This uniquely identifies each operation in the CRDT. - pub fn id(&self) -> OpId { - OpId(self.clock, self.author) - } -} - -/// An object that can be used to create and sign operations. -#[derive(Default)] -pub struct Actor { - pub signer: G, - pub clock: Lamport, - pub ops: BTreeMap<(Lamport, PublicKey), Op>, -} - -impl Actor { - pub fn new(signer: G) -> Self { - Self { - signer, - clock: Lamport::default(), - ops: BTreeMap::default(), - } - } - - pub fn receive(&mut self, ops: impl IntoIterator>) -> Lamport { - for op in ops { - let clock = op.clock; - - self.ops.insert((clock, op.author), op); - self.clock.merge(clock); - } - self.clock - } - - /// Reset actor state to initial state. - pub fn reset(&mut self) { - self.ops.clear(); - self.clock = Lamport::default(); - } - - /// Returned an ordered list of events. - pub fn timeline(&self) -> impl Iterator> { - self.ops.values() - } - - /// Create a new operation. - pub fn op(&mut self, action: A) -> Op { - let author = *self.signer.public_key(); - let clock = self.clock.tick(); - let timestamp = clock::Physical::now(); - let op = Op { - action, - author, - clock, - timestamp, - }; - self.ops.insert((self.clock, author), op.clone()); - - op - } -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn test_opid_try_from_str() { - let s = "z6MksFqXN3Yhqk8pTJdUGLwATkRfQvwZXPqR2qMEhbS9wzpT/12"; - let id = OpId::try_from(s).expect("Op ID parses string"); - assert_eq!(s, id.to_string(), "string conversion is consistent"); - - let s = ""; - assert!(OpId::try_from(s).is_err(), "empty strings are invalid"); - - let s = "jlkjfksgi"; - assert!(OpId::try_from(s).is_err(), "badly formatted string"); - - assert_eq!( - serde_json::from_str::(serde_json::to_string(&id).unwrap().as_str()).unwrap(), - id - ); + .collect::, _>>()?; + + // SAFETY: Entry is guaranteed to have at least one operation. + #[allow(clippy::unwrap_used)] + Ok(Self(NonEmpty::from_vec(ops).unwrap())) } } diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index d6639864..32d62adf 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use radicle_crdt::clock; -use radicle_crdt::{GMap, LWWReg, LWWSet, Max, Redactable, Semilattice}; +use radicle_crdt::{GMap, GSet, LWWReg, LWWSet, Lamport, Max, Redactable, Semilattice}; use crate::cob; use crate::cob::common::{Author, Tag, Timestamp}; @@ -134,6 +134,8 @@ pub struct Patch { /// List of patch revisions. The initial changeset is part of the /// first revision. pub revisions: GMap>, + /// Timeline of operations. + pub timeline: GSet<(Lamport, OpId)>, } impl Semilattice for Patch { @@ -156,6 +158,7 @@ impl Default for Patch { target: Max::from(MergeTarget::default()).into(), tags: LWWSet::default(), revisions: GMap::default(), + timeline: GSet::default(), } } } @@ -199,11 +202,12 @@ impl Patch { } pub fn revisions(&self) -> impl DoubleEndedIterator { - self.revisions - .iter() - .filter_map(|(rid, r)| -> Option<(&RevisionId, &Revision)> { - r.get().map(|r| (rid, r)) - }) + self.timeline.iter().filter_map(|(_, id)| { + self.revisions + .get(id) + .and_then(Redactable::get) + .map(|rev| (id, rev)) + }) } pub fn head(&self) -> &git::Oid { @@ -244,10 +248,12 @@ impl store::FromHistory for Patch { fn apply(&mut self, ops: impl IntoIterator) -> Result<(), ApplyError> { for op in ops { - let id = op.id(); + let id = op.id; let author = Author::new(op.author); let timestamp = op.timestamp; + self.timeline.insert((op.clock, id)); + match op.action { Action::Edit { title, @@ -271,6 +277,12 @@ impl store::FromHistory for Patch { base, oid, } => { + // Since revisions are keyed by content hash, we shouldn't re-insert a revision + // if it already exists, otherwise this will be resolved via the `merge` + // operation of `Redactable`. + if self.revisions.contains_key(&id) { + continue; + } self.revisions.insert( id, Redactable::Present(Revision::new( @@ -323,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(action, op.author, timestamp, op.clock)])?; + revision.discussion.apply([cob::Op::new( + op.id, action, op.nonce, op.author, timestamp, op.clock, + )])?; } else { return Err(ApplyError::Missing(revision)); } @@ -549,7 +561,7 @@ impl store::Transaction { title: impl ToString, description: impl ToString, target: MergeTarget, - ) -> OpId { + ) -> Result { self.push(Action::Edit { title: title.to_string(), description: description.to_string(), @@ -558,7 +570,11 @@ impl store::Transaction { } /// Start a patch revision discussion. - pub fn thread(&mut self, revision: RevisionId, body: S) -> OpId { + pub fn thread( + &mut self, + revision: RevisionId, + body: S, + ) -> Result { self.push(Action::Thread { revision, action: thread::Action::Comment { @@ -574,7 +590,7 @@ impl store::Transaction { revision: RevisionId, body: S, reply_to: Option, - ) -> OpId { + ) -> Result { self.push(Action::Thread { revision, action: thread::Action::Comment { @@ -591,7 +607,7 @@ impl store::Transaction { verdict: Option, comment: Option, inline: Vec, - ) -> OpId { + ) -> Result { self.push(Action::Review { revision, comment, @@ -601,7 +617,7 @@ impl store::Transaction { } /// Merge a patch revision. - pub fn merge(&mut self, revision: RevisionId, commit: git::Oid) -> OpId { + pub fn merge(&mut self, revision: RevisionId, commit: git::Oid) -> Result { self.push(Action::Merge { revision, commit }) } @@ -611,7 +627,7 @@ impl store::Transaction { description: impl ToString, base: impl Into, oid: impl Into, - ) -> OpId { + ) -> Result { self.push(Action::Revision { description: description.to_string(), base: base.into(), @@ -624,7 +640,7 @@ impl store::Transaction { &mut self, add: impl IntoIterator, remove: impl IntoIterator, - ) -> OpId { + ) -> Result { let add = add.into_iter().collect::>(); let remove = remove.into_iter().collect::>(); @@ -663,10 +679,10 @@ impl<'a, 'g> PatchMut<'a, 'g> { ) -> Result where G: Signer, - F: FnOnce(&mut Transaction) -> T, + F: FnOnce(&mut Transaction) -> Result, { - let mut tx = Transaction::new(*signer.public_key(), self.clock); - let output = operations(&mut tx); + 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)?; self.patch.apply(ops)?; @@ -803,9 +819,11 @@ impl<'a> Patches<'a> { ) -> Result, Error> { let (id, patch, clock) = Transaction::initial("Create patch", &mut self.raw, signer, |tx| { - tx.revision(String::default(), base, oid); - tx.edit(title, description, target); - tx.tag(tags.to_owned(), []); + tx.revision(String::default(), base, oid)?; + tx.edit(title, description, target)?; + tx.tag(tags.to_owned(), [])?; + + Ok(()) })?; // Just a sanity check that our clock is advancing as expected. debug_assert_eq!(clock.get(), 3); @@ -884,7 +902,7 @@ mod test { use qcheck::{Arbitrary, TestResult}; use super::*; - use crate::cob::op::{Actor, ActorId}; + use crate::cob::test::Actor; use crate::crypto::test::signer::MockSigner; use crate::test; @@ -908,9 +926,13 @@ mod test { impl Arbitrary for Changes { fn arbitrary(g: &mut qcheck::Gen) -> Self { - type State = (clock::Lamport, Vec, Vec); + type State = ( + Actor, + clock::Lamport, + Vec, + Vec, + ); - let author = ActorId::from([0; 32]); let rng = fastrand::Rng::with_seed(u64::arbitrary(g)); let oids = iter::repeat_with(|| { git::Oid::try_from( @@ -924,35 +946,35 @@ mod test { .take(16) .collect::>(); - let gen = WeightedGenerator::<(clock::Lamport, Action), State>::new(rng.clone()) - .variant(1, |(clock, _, _), rng| { + let gen = WeightedGenerator::<(clock::Lamport, Op), State>::new(rng.clone()) + .variant(1, |(actor, clock, _, _), rng| { Some(( clock.tick(), - Action::Edit { + actor.op(Action::Edit { title: iter::repeat_with(|| rng.alphabetic()).take(8).collect(), description: iter::repeat_with(|| rng.alphabetic()).take(16).collect(), target: MergeTarget::Delegates, - }, + }), )) }) - .variant(1, |(clock, revisions, _), rng| { + .variant(1, |(actor, clock, revisions, _), rng| { if revisions.is_empty() { return None; } let revision = revisions[rng.usize(..revisions.len())]; let commit = oids[rng.usize(..oids.len())]; - Some((clock.tick(), Action::Merge { revision, commit })) + Some((clock.tick(), actor.op(Action::Merge { revision, commit }))) }) - .variant(1, |(clock, revisions, _), rng| { + .variant(1, |(actor, clock, revisions, _), rng| { if revisions.is_empty() { return None; } let revision = revisions[rng.usize(..revisions.len())]; - Some((clock.tick(), Action::Redact { revision })) + Some((clock.tick(), actor.op(Action::Redact { revision }))) }) - .variant(1, |(clock, _, tags), rng| { + .variant(1, |(actor, clock, _, tags), rng| { let add = iter::repeat_with(|| rng.alphabetic()) .take(rng.usize(0..=3)) .map(|c| Tag::new(c).unwrap()) @@ -965,32 +987,29 @@ mod test { for tag in &add { tags.push(tag.clone()); } - Some((clock.tick(), Action::Tag { add, remove })) + Some((clock.tick(), actor.op(Action::Tag { add, remove }))) }) - .variant(1, |(clock, revisions, _), rng| { + .variant(1, |(actor, clock, revisions, _), rng| { let oid = oids[rng.usize(..oids.len())]; let base = oids[rng.usize(..oids.len())]; let description = iter::repeat_with(|| rng.alphabetic()).take(6).collect(); + let op = actor.op(Action::Revision { + description, + base, + oid, + }); if rng.bool() { - revisions.push(OpId::new(clock.tick(), author)); + revisions.push(op.id); } - Some(( - *clock, - Action::Revision { - description, - base, - oid, - }, - )) + Some((*clock, op)) }); let mut changes = Vec::new(); let mut permutations: [Vec; N] = array::from_fn(|_| Vec::new()); - let timestamp = Timestamp::now() + rng.u64(..60); - for (clock, action) in gen.take(g.size()) { - changes.push(Op::new(action, author, timestamp, clock)); + for (_, op) in gen.take(g.size()) { + changes.push(op); } for p in &mut permutations { @@ -1231,7 +1250,7 @@ mod test { } #[test] - fn test_revision_redacted_reinsert() { + fn test_revision_redact_reinsert() { let base = git::Oid::from_str("cb18e95ada2bb38aadd8e6cef0963ce37a87add3").unwrap(); let oid = git::Oid::from_str("518d5069f94c03427f694bb494ac1cd7d1339380").unwrap(); let mut alice = Actor::<_, Action>::new(MockSigner::default()); @@ -1251,6 +1270,30 @@ mod test { assert_eq!(p1, p2); } + #[test] + fn test_revision_merge_reinsert() { + let base = git::Oid::from_str("cb18e95ada2bb38aadd8e6cef0963ce37a87add3").unwrap(); + let oid = git::Oid::from_str("518d5069f94c03427f694bb494ac1cd7d1339380").unwrap(); + let mut alice = Actor::<_, Action>::new(MockSigner::default()); + let mut p1 = Patch::default(); + let mut p2 = Patch::default(); + + let a1 = alice.op(Action::Revision { + description: String::new(), + base, + oid, + }); + let a2 = alice.op(Action::Merge { + revision: a1.id(), + commit: oid, + }); + + p1.apply([a1.clone(), a2.clone(), a1.clone()]).unwrap(); + p2.apply([a1.clone(), a1, a2]).unwrap(); + + assert_eq!(p1, p2); + } + #[test] fn test_patch_review_edit() { let tmp = tempfile::tempdir().unwrap(); @@ -1328,10 +1371,9 @@ mod test { assert_eq!(patch.description(), Some("Blah blah blah.")); assert_eq!(patch.version(), 0); - let r1 = patch + let _ = patch .update("I've made changes.", base, rev1_oid, &signer) .unwrap(); - assert_eq!(r1.clock().get(), 4); 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 35e19829..25a4f701 100644 --- a/radicle/src/cob/store.rs +++ b/radicle/src/cob/store.rs @@ -6,11 +6,13 @@ 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; use crate::cob::common::Author; -use crate::cob::op::{Op, OpId, Ops}; +use crate::cob::op::{Nonce, Op, OpId, Ops}; use crate::cob::CollaborativeObject; use crate::cob::{ActorId, Create, History, ObjectId, TypeName, Update}; use crate::crypto::PublicKey; @@ -27,7 +29,7 @@ pub const HISTORY_TYPE: &str = "radicle"; /// All collaborative objects implement this trait. pub trait FromHistory: Sized + Default { /// The underlying action composing each operation. - type Action: for<'de> Deserialize<'de>; + type Action: for<'de> Deserialize<'de> + Serialize; /// Error returned by `apply` function. type Error: std::error::Error; @@ -92,6 +94,7 @@ pub struct Store<'a, T> { identity: Identity, raw: &'a storage::Repository, witness: PhantomData, + rng: StdRng, } impl<'a, T> AsRef for Store<'a, T> { @@ -103,6 +106,7 @@ impl<'a, T> AsRef for Store<'a, T> { impl<'a, T> Store<'a, T> { /// Open a new generic store. pub fn open(whoami: PublicKey, store: &'a storage::Repository) -> Result { + let rng = rng::std(); let identity = Identity::load(&whoami, store)?; Ok(Self { @@ -110,6 +114,7 @@ impl<'a, T> Store<'a, T> { whoami, raw: store, witness: PhantomData, + rng, }) } @@ -122,6 +127,11 @@ impl<'a, T> Store<'a, T> { pub fn public_key(&self) -> &PublicKey { &self.whoami } + + /// 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> @@ -133,10 +143,10 @@ where &self, object_id: ObjectId, message: &str, - actions: impl Into>, + actions: impl Into>>, signer: &G, ) -> Result { - let changes = actions.into().try_map(|e| encoding::encode(&e))?; + let changes = actions.into(); cob::update( self.raw, @@ -158,10 +168,10 @@ where pub fn create( &self, message: &str, - actions: impl Into>, + actions: impl Into>>, signer: &G, ) -> Result<(ObjectId, T, Lamport), Error> { - let contents = actions.into().try_map(|e| encoding::encode(&e))?; + let contents = actions.into(); let cob = cob::create( self.raw, signer, @@ -226,18 +236,20 @@ pub struct Transaction { actor: ActorId, start: Lamport, clock: Lamport, - actions: Vec, + rng: StdRng, + actions: Vec<(T::Action, OpId, Nonce, Vec)>, } impl Transaction { /// Create a new transaction. - pub fn new(actor: ActorId, clock: Lamport) -> Self { + pub fn new(actor: ActorId, clock: Lamport, rng: StdRng) -> Self { let start = clock; Self { actor, start, clock, + rng, actions: Vec::new(), } } @@ -251,7 +263,7 @@ impl Transaction { ) -> Result<(ObjectId, T, Lamport), Error> where G: Signer, - F: FnOnce(&mut Self), + F: FnOnce(&mut Self) -> Result<(), Error>, T::Action: Serialize + Clone, { let actor = *signer.public_key(); @@ -259,12 +271,14 @@ impl Transaction { actor, start: Lamport::initial(), clock: Lamport::initial(), + rng: store.rng(), actions: Vec::new(), }; - operations(&mut tx); + operations(&mut tx)?; let actions = NonEmpty::from_vec(tx.actions) - .expect("Transaction::initial: transaction must contain at least one operation"); + .expect("Transaction::initial: transaction must contain at least one operation") + .map(|(_, _, _, blob)| blob); let (id, cob, clock) = store.create(message, actions, signer)?; // The history clock should be in sync with the tx clock. @@ -274,9 +288,14 @@ impl Transaction { } /// Add an operation to this transaction. - pub fn push(&mut self, action: T::Action) -> cob::OpId { - self.actions.push(action); - OpId::new(self.clock.tick(), self.actor) + pub fn push(&mut self, action: T::Action) -> Result { + let nonce = self.rng.next_u64(); + let (id, blob) = encoding::encode(&action, nonce)?; + + self.actions.push((action, id, nonce, blob)); + self.clock.tick(); + + Ok(id) } /// Commit transaction. @@ -294,7 +313,7 @@ impl Transaction { { let actions = NonEmpty::from_vec(self.actions) .expect("Transaction::commit: transaction must not be empty"); - let cob = store.update(id, msg, actions.clone(), signer)?; + let cob = store.update(id, msg, actions.clone().map(|(_, _, _, blob)| blob), signer)?; let author = self.actor; let timestamp = cob.history().timestamp().into(); @@ -305,7 +324,9 @@ impl Transaction { let mut clock = self.start; let ops = actions .into_iter() - .map(|action| cob::Op { + .map(|(action, id, nonce, _)| cob::Op { + id, + nonce, action, author, clock: clock.tick(), @@ -321,15 +342,43 @@ 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(obj: &T) -> Result, serde_json::Error> { + pub fn encode( + action: &A, + nonce: Nonce, + ) -> Result<(OpId, Vec), serde_json::Error> { let mut buf = Vec::new(); let mut serializer = serde_json::Serializer::with_formatter(&mut buf, CanonicalFormatter::new()); - obj.serialize(&mut serializer)?; + serde_json::json!({ + "action": action, + "nonce": nonce, + }) + .serialize(&mut serializer)?; - Ok(buf) + // 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 crate::env; + 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) = env::seed() { + return StdRng::from_seed(seed); + } + StdRng::from_entropy() } } diff --git a/radicle/src/cob/test.rs b/radicle/src/cob/test.rs index e444e9cd..cc16eb7f 100644 --- a/radicle/src/cob/test.rs +++ b/radicle/src/cob/test.rs @@ -1,13 +1,15 @@ -use std::collections::BTreeSet; +use std::collections::{BTreeMap, BTreeSet}; use std::marker::PhantomData; use std::ops::{ControlFlow, Deref}; use nonempty::NonEmpty; use serde::Serialize; +use crate::cob::common::clock; use crate::cob::op::{Op, Ops}; use crate::cob::store::encoding; -use crate::cob::History; +use crate::cob::{EntryBlob, History}; +use crate::crypto::{PublicKey, Signer}; use crate::git::Oid; use crate::test::arbitrary; @@ -28,14 +30,17 @@ where pub fn new(op: &Op) -> HistoryBuilder { let entry = arbitrary::oid(); let resource = arbitrary::oid(); - let contents = encoding::encode(&op.action).unwrap(); + let (id, data) = encoding::encode(&op.action, op.nonce).unwrap(); Self { history: History::new_from_root( entry, op.author, resource, - NonEmpty::new(contents), + NonEmpty::new(EntryBlob { + oid: id.into(), + data, + }), op.timestamp.as_secs(), ), resource, @@ -44,11 +49,16 @@ where } pub fn append(&mut self, op: &Op) -> &mut Self { + let (id, data) = encoding::encode(&op.action, op.nonce).unwrap(); + self.history.extend( arbitrary::oid(), op.author, self.resource, - NonEmpty::new(encoding::encode(&op.action).unwrap()), + NonEmpty::new(EntryBlob { + oid: id.into(), + data, + }), op.timestamp.as_secs(), ); self @@ -95,3 +105,69 @@ where { HistoryBuilder::new(op) } + +/// An object that can be used to create and sign operations. +pub struct Actor { + pub signer: G, + pub clock: clock::Lamport, + pub ops: BTreeMap<(clock::Lamport, PublicKey), Op>, +} + +impl Default for Actor { + fn default() -> Self { + Self::new(G::default()) + } +} + +impl Actor { + pub fn new(signer: G) -> Self { + Self { + signer, + clock: clock::Lamport::default(), + ops: BTreeMap::default(), + } + } +} + +impl Actor { + pub fn receive(&mut self, ops: impl IntoIterator>) -> clock::Lamport { + for op in ops { + let clock = op.clock; + + self.ops.insert((clock, op.author), op); + self.clock.merge(clock); + } + self.clock + } + + /// Reset actor state to initial state. + pub fn reset(&mut self) { + self.ops.clear(); + self.clock = clock::Lamport::default(); + } + + /// Returned an ordered list of events. + pub fn timeline(&self) -> impl Iterator> { + self.ops.values() + } + + /// Create a new operation. + pub fn op(&mut self, action: A) -> Op { + 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, + }; + self.ops.insert((self.clock, author), op.clone()); + + op + } +} diff --git a/radicle/src/cob/thread.rs b/radicle/src/cob/thread.rs index 9b0d1006..b6046f1e 100644 --- a/radicle/src/cob/thread.rs +++ b/radicle/src/cob/thread.rs @@ -1,5 +1,4 @@ use std::cmp::Ordering; -use std::ops::{Deref, DerefMut}; use std::str::FromStr; use once_cell::sync::Lazy; @@ -10,10 +9,9 @@ use thiserror::Error; use crate::cob; use crate::cob::common::{Reaction, Timestamp}; use crate::cob::{ActorId, Op, OpId}; -use crate::crypto::Signer; use crdt::clock::Lamport; -use crdt::{GMap, LWWSet, Max, Redactable, Semilattice}; +use crdt::{GMap, GSet, LWWSet, Max, Redactable, Semilattice}; /// Type name of a thread, as well as the domain for all thread operations. /// Note that threads are not usually used standalone. They are embeded into other COBs. @@ -167,12 +165,15 @@ pub struct Thread { comments: GMap>, /// Reactions to changes. reactions: GMap>, + /// Comment timeline. + timeline: GSet<(Lamport, OpId)>, } impl Semilattice for Thread { fn merge(&mut self, other: Self) { self.comments.merge(other.comments); self.reactions.merge(other.reactions); + self.timeline.merge(other.timeline); } } @@ -181,6 +182,7 @@ impl Thread { Self { comments: GMap::singleton(id, Redactable::Present(comment)), reactions: GMap::default(), + timeline: GSet::default(), } } @@ -204,6 +206,10 @@ impl Thread { } } + pub fn root(&self) -> (&CommentId, &Comment) { + self.first().expect("Thread::root: thread is empty") + } + pub fn first(&self) -> Option<(&CommentId, &Comment)> { self.comments().next() } @@ -238,9 +244,12 @@ impl Thread { } pub fn comments(&self) -> impl DoubleEndedIterator + '_ { - self.comments - .iter() - .filter_map(|(id, comment)| comment.get().map(|comment| (id, comment))) + self.timeline.iter().filter_map(|(_, id)| { + self.comments + .get(id) + .and_then(Redactable::get) + .map(|comment| (id, comment)) + }) } } @@ -254,12 +263,20 @@ impl cob::store::FromHistory for Thread { fn apply(&mut self, ops: impl IntoIterator>) -> Result<(), OpError> { for op in ops.into_iter() { - let id = op.id(); + let id = op.id; let author = op.author; let timestamp = op.timestamp; + self.timeline.insert((op.clock, op.id)); + match op.action { Action::Comment { body, reply_to } => { + // Since comments are keyed by content hash, we shouldn't re-insert a comment + // if it already exists, otherwise this will be resolved via the `merge` + // operation of `Redactable`. + if self.comments.contains_key(&id) { + continue; + } self.comments.insert( id, Redactable::Present(Comment::new(author, body, reply_to, timestamp)), @@ -296,73 +313,12 @@ impl cob::store::FromHistory for Thread { } } -/// An object that can be used to create and sign changes. -pub struct Actor { - inner: cob::Actor, -} - -impl Default for Actor { - fn default() -> Self { - Self { - inner: cob::Actor::new(G::default()), - } - } -} - -impl Actor { - pub fn new(signer: G) -> Self { - Self { - inner: cob::Actor::new(signer), - } - } - - /// Create a new thread. - pub fn thread(&self) -> Thread { - Thread::default() - } - - /// Create a new comment. - pub fn comment(&mut self, body: &str, reply_to: Option) -> Op { - self.op(Action::Comment { - body: String::from(body), - reply_to, - }) - } - - /// Create a new redaction. - pub fn redact(&mut self, id: OpId) -> Op { - self.op(Action::Redact { id }) - } - - /// Edit a comment. - pub fn edit(&mut self, id: OpId, body: &str) -> Op { - self.op(Action::Edit { - id, - body: body.to_owned(), - }) - } -} - -impl Deref for Actor { - type Target = cob::Actor; - - fn deref(&self) -> &Self::Target { - &self.inner - } -} - -impl DerefMut for Actor { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.inner - } -} - #[cfg(test)] mod tests { use std::collections::BTreeSet; + use std::ops::{Deref, DerefMut}; use std::{array, iter}; - use nonempty::NonEmpty; use pretty_assertions::assert_eq; use qcheck::{Arbitrary, TestResult}; @@ -373,6 +329,72 @@ mod tests { use crate::cob::store::FromHistory; use crate::cob::test; use crate::crypto::test::signer::MockSigner; + use crate::crypto::Signer; + + /// An object that can be used to create and sign changes. + pub struct Actor { + inner: cob::test::Actor, + } + + impl Default for Actor { + fn default() -> Self { + Self { + inner: cob::test::Actor::new(G::default()), + } + } + } + + impl Actor { + pub fn new(signer: G) -> Self { + Self { + inner: cob::test::Actor::new(signer), + } + } + + /// Create a new comment. + pub fn comment(&mut self, body: &str, reply_to: Option) -> Op { + self.op(Action::Comment { + body: String::from(body), + reply_to, + }) + } + + /// Create a new redaction. + pub fn redact(&mut self, id: OpId) -> Op { + self.op(Action::Redact { id }) + } + + /// Edit a comment. + pub fn edit(&mut self, id: OpId, body: &str) -> Op { + self.op(Action::Edit { + id, + body: body.to_owned(), + }) + } + + /// React to a comment. + pub fn react(&mut self, to: OpId, reaction: Reaction, active: bool) -> Op { + self.op(Action::React { + to, + reaction, + active, + }) + } + } + + impl Deref for Actor { + type Target = cob::test::Actor; + + fn deref(&self) -> &Self::Target { + &self.inner + } + } + + impl DerefMut for Actor { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.inner + } + } #[derive(Clone)] struct Changes { @@ -394,75 +416,63 @@ mod tests { impl Arbitrary for Changes { fn arbitrary(g: &mut qcheck::Gen) -> Self { - let author = ActorId::from([0; 32]); let rng = fastrand::Rng::with_seed(u64::arbitrary(g)); - let root = OpId::initial(author); - let gen = - WeightedGenerator::<(Lamport, Action), (Lamport, BTreeSet)>::new(rng.clone()) - .variant(3, |(clock, comments), rng| { - comments.insert(OpId::new(clock.tick(), author)); + let gen = WeightedGenerator::< + (Lamport, Op), + (Actor, Lamport, BTreeSet), + >::new(rng.clone()) + .variant(3, |(actor, clock, comments), rng| { + let comment = actor.comment( + iter::repeat_with(|| rng.alphabetic()) + .take(4) + .collect::() + .as_str(), + None, + ); + comments.insert(comment.id); - Some(( - *clock, - Action::Comment { - body: iter::repeat_with(|| rng.alphabetic()).take(16).collect(), - reply_to: Some(root), - }, - )) - }) - .variant(2, |(clock, comments), rng| { - if comments.is_empty() { - return None; - } - let id = *comments.iter().nth(rng.usize(..comments.len())).unwrap(); + Some((*clock, comment)) + }) + .variant(2, |(actor, clock, comments), rng| { + if comments.is_empty() { + return None; + } + let id = *comments.iter().nth(rng.usize(..comments.len())).unwrap(); + let edit = actor.edit( + id, + iter::repeat_with(|| rng.alphabetic()) + .take(4) + .collect::() + .as_str(), + ); - Some(( - *clock, - Action::Edit { - id, - body: iter::repeat_with(|| rng.alphabetic()).take(16).collect(), - }, - )) - }) - .variant(2, |(clock, comments), rng| { - if comments.is_empty() { - return None; - } - let to = *comments.iter().nth(rng.usize(..comments.len())).unwrap(); + Some((*clock, edit)) + }) + .variant(2, |(actor, clock, comments), rng| { + if comments.is_empty() { + return None; + } + let to = *comments.iter().nth(rng.usize(..comments.len())).unwrap(); + let react = actor.react(to, Reaction::new('✨').unwrap(), rng.bool()); - Some(( - clock.tick(), - Action::React { - to, - reaction: Reaction::new('✨').unwrap(), - active: rng.bool(), - }, - )) - }) - .variant(2, |(clock, comments), rng| { - if comments.is_empty() { - return None; - } - let id = *comments.iter().nth(rng.usize(..comments.len())).unwrap(); - comments.remove(&id); + Some((clock.tick(), react)) + }) + .variant(2, |(actor, clock, comments), rng| { + if comments.is_empty() { + return None; + } + let id = *comments.iter().nth(rng.usize(..comments.len())).unwrap(); + comments.remove(&id); + let redact = actor.redact(id); - Some((clock.tick(), Action::Redact { id })) - }); + Some((clock.tick(), redact)) + }); - let mut ops = vec![Op::new( - Action::Comment { - body: String::default(), - reply_to: None, - }, - author, - Timestamp::now(), - Lamport::initial(), - )]; + let mut ops = vec![Actor::::default().comment("", None)]; let mut permutations: [Vec>; N] = array::from_fn(|_| Vec::new()); - for (clock, action) in gen.take(g.size()) { - let timestamp = Timestamp::now() + rng.u64(..60); - ops.push(Op::new(action, author, timestamp, clock)); + for (_, op) in gen.take(g.size()) { + ops.push(op); } for p in &mut permutations { @@ -474,19 +484,6 @@ mod tests { } } - mod setup { - use super::*; - use crate::storage::git as storage; - use cob::store::Store; - - pub fn store<'a, G: Signer>( - signer: &G, - repo: &'a storage::Repository, - ) -> Store<'a, Thread> { - Store::::open(*signer.public_key(), repo).unwrap() - } - } - #[test] fn test_redact_comment() { let tmp = tempfile::tempdir().unwrap(); @@ -538,36 +535,6 @@ mod tests { assert_eq!(t1, t2); } - #[test] - fn test_storage() { - let tmp = tempfile::tempdir().unwrap(); - let (_, signer, repository) = radicle::test::setup::context(&tmp); - let store = setup::store(&signer, &repository); - let mut alice = Actor::new(signer); - - let a0 = alice.comment("Thread root", None); - let a1 = alice.comment("First comment", Some(a0.id())); - let a2 = alice.comment("Second comment", Some(a0.id())); - - let mut expected = Thread::default(); - expected - .apply([a0.clone(), a1.clone(), a2.clone()]) - .unwrap(); - - let (id, _, _) = store - .create("Thread created", a0.action, &alice.signer) - .unwrap(); - - let actions = NonEmpty::from_vec(vec![a1.action, a2.action]).unwrap(); - store - .update(id, "Thread updated", actions, &alice.signer) - .unwrap(); - - let (actual, _) = store.get(&id).unwrap().unwrap(); - - assert_eq!(actual, expected); - } - #[test] fn test_timelines_basic() { let mut alice = Actor::::default(); @@ -667,6 +634,84 @@ mod tests { } } + #[test] + fn test_duplicate_comments() { + let mut alice = Actor::::default(); + let mut bob = Actor::::default(); + + let a0 = alice.comment("Hello World!", None); + let b0 = bob.comment("Hello World!", None); + + let mut a = test::history::(&a0); + let mut b = a.clone(); + + b.append(&b0); + a.merge(b); + + let (thread, _) = Thread::from_history(&a).unwrap(); + + assert_eq!(thread.comments().count(), 2); + + let (first_id, first) = thread.comments().nth(0).unwrap(); + let (second_id, second) = thread.comments().nth(1).unwrap(); + + assert!(first_id != second_id); // The ids are not the same, + assert_eq!(first.edits, second.edits); // despite the content being the same. + } + + #[test] + fn test_duplicate_comments_same_author() { + let mut alice = Actor::::default(); + + let a0 = alice.comment("Hello World!", None); + let a1 = alice.comment("Hello World!", None); + let a2 = alice.comment("Hello World!", None); + + // These simulate two devices sharing the same key. + let mut h1 = test::history::(&a0); + let mut h2 = h1.clone(); + let mut h3 = h1.clone(); + + // Alice writes the same comment on both devices, not realizing what she has done. + h1.append(&a1); + h2.append(&a2); + + // Eventually the histories are merged by a third party. + h3.merge(h1); + h3.merge(h2); + + let (thread, _) = Thread::from_history(&h3).unwrap(); + + // The three comments, distinct yet identical in terms of content, are preserved. + assert_eq!(thread.comments().count(), 3); + + let (first_id, first) = thread.comments().nth(0).unwrap(); + let (second_id, second) = thread.comments().nth(1).unwrap(); + let (third_id, third) = thread.comments().nth(2).unwrap(); + + // Their IDs are not the same. + assert!(first_id != second_id); + assert!(second_id != third_id); + // Their content are the same. + assert_eq!(first, second); + assert_eq!(second, third); + } + + #[test] + fn test_comment_edit_reinsert() { + let mut alice = Actor::::default(); + let mut t1 = Thread::default(); + let mut t2 = Thread::default(); + + let a1 = alice.comment("Hello.", None); + let a2 = alice.edit(a1.id(), "Hello World."); + + t1.apply([a1.clone(), a2.clone(), a1.clone()]).unwrap(); + t2.apply([a1.clone(), a1, a2]).unwrap(); + + assert_eq!(t1, t2); + } + #[test] fn prop_invariants() { fn property(log: Changes<3>) -> TestResult { diff --git a/radicle/src/lib.rs b/radicle/src/lib.rs index 2c10f25e..60fcacd7 100644 --- a/radicle/src/lib.rs +++ b/radicle/src/lib.rs @@ -36,3 +36,7 @@ pub mod prelude { pub use profile::Profile; pub use storage::{BranchName, ReadRepository, ReadStorage, WriteRepository, WriteStorage}; } + +pub mod env { + pub use crypto::env::*; +}