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::*; +}