From 2b44ff1b55e371c419cf2076b7f97b4404b41a34 Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Thu, 9 Mar 2023 11:20:34 +0100 Subject: [PATCH] cob: Use `EntryId` as the operation identifier By introducing a small limitation: only allowing entries in the change graph to be addressable, instead of individual operations; we drastically simplify the CRDT implementation. There are four advantages: 1. Op ids are just regular SHA-1s 2. There's no need for relative IDs, ops never refer to other ops within the same commit 3. There's no need for a nonce, since commits can't collide, and neither can op IDs 4. `OpId` can just be an alias of `EntryId` The disadvantage of course, is that we have to be mindful of how we create op transactions, since each transaction creates an addressable unit. For example, we must not include multiple patch revisions in the same transaction. --- radicle-httpd/src/api/error.rs | 11 +++- radicle-httpd/src/api/json.rs | 4 +- radicle-httpd/src/api/v1/projects.rs | 22 ++++--- radicle-httpd/src/test.rs | 89 ++++++++++++++++------------ 4 files changed, 71 insertions(+), 55 deletions(-) diff --git a/radicle-httpd/src/api/error.rs b/radicle-httpd/src/api/error.rs index 5608a649c..91d593a1a 100644 --- a/radicle-httpd/src/api/error.rs +++ b/radicle-httpd/src/api/error.rs @@ -81,10 +81,17 @@ impl IntoResponse for Error { StatusCode::INTERNAL_SERVER_ERROR, Some(e.message().to_owned()), ), - _ => { + other => { tracing::error!("Error: {:?}", &self); - (StatusCode::INTERNAL_SERVER_ERROR, None) + if cfg!(debug_assertions) { + ( + StatusCode::INTERNAL_SERVER_ERROR, + Some(format!("{other:?}")), + ) + } else { + (StatusCode::INTERNAL_SERVER_ERROR, None) + } } }; diff --git a/radicle-httpd/src/api/json.rs b/radicle-httpd/src/api/json.rs index dd4bed70b..c542f556f 100644 --- a/radicle-httpd/src/api/json.rs +++ b/radicle-httpd/src/api/json.rs @@ -9,7 +9,7 @@ use serde_json::{json, Value}; use radicle::cob::issue::{Issue, IssueId}; use radicle::cob::patch::{Patch, PatchId}; use radicle::cob::thread::{self, CommentId}; -use radicle::cob::{Author, OpId, Timestamp}; +use radicle::cob::{Author, Timestamp}; use radicle_surf::blob::Blob; use radicle_surf::tree::Tree; use radicle_surf::{Commit, Stats}; @@ -132,7 +132,7 @@ fn name_in_path(path: &str) -> &str { #[derive(Serialize)] #[serde(rename_all = "camelCase")] struct Comment { - id: OpId, + id: CommentId, author: Author, body: String, reactions: [String; 0], diff --git a/radicle-httpd/src/api/v1/projects.rs b/radicle-httpd/src/api/v1/projects.rs index 11520f6cf..0720b25d7 100644 --- a/radicle-httpd/src/api/v1/projects.rs +++ b/radicle-httpd/src/api/v1/projects.rs @@ -1104,7 +1104,7 @@ mod routes { "assignees": [], "discussion": [ { - "id": ISSUE_DISCUSSION_ID, + "id": ISSUE_ID, "author": { "id": "did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi" }, @@ -1122,7 +1122,7 @@ mod routes { #[tokio::test] async fn test_projects_issues_create() { - const CREATED_ISSUE_ID: &str = "1b706bb9af971d43ec38718c71402e36055c5c89"; + const CREATED_ISSUE_ID: &str = "b457364fbe2ef0eac69a835a087f60ee13ccb367"; let tmp = tempfile::tempdir().unwrap(); let ctx = test::contributor(tmp.path()); @@ -1171,7 +1171,7 @@ mod routes { "status": "open", }, "discussion": [{ - "id": ISSUE_DISCUSSION_ID, + "id": CREATED_ISSUE_ID, "author": { "id": CONTRIBUTOR_PUB_KEY, }, @@ -1245,7 +1245,7 @@ mod routes { "replyTo": null, }, { - "id": "265af21e409eacc8eb150b73882ac3ada9d4aea3", + "id": "9685b141c2e939c3d60f8ca34f8c7bf01a609af1", "author": { "id": CONTRIBUTOR_PUB_KEY, }, @@ -1269,7 +1269,7 @@ mod routes { test::create_session(ctx).await; let body = serde_json::to_vec(&json!({ - "type":"thread", + "type": "thread", "action": { "type": "comment", "body": "This is a reply to the first comment", @@ -1277,9 +1277,7 @@ mod routes { }})) .unwrap(); - let response = get(&app, format!("/projects/{CONTRIBUTOR_RID}/issues")).await; - println!("{:?}", response.json().await); - + let _ = get(&app, format!("/projects/{CONTRIBUTOR_RID}/issues")).await; let response = patch( &app, format!("/projects/{CONTRIBUTOR_RID}/issues/{CONTRIBUTOR_ISSUE_ID}"), @@ -1358,7 +1356,7 @@ mod routes { "tags": [], "revisions": [ { - "id": "47878ed82515772f4c44e4796c330f4a74473559", + "id": PATCH_ID, "description": "", "reviews": [], } @@ -1389,7 +1387,7 @@ mod routes { "tags": [], "revisions": [ { - "id": "47878ed82515772f4c44e4796c330f4a74473559", + "id": PATCH_ID, "description": "", "reviews": [], } @@ -1401,7 +1399,7 @@ mod routes { #[tokio::test] async fn test_projects_create_patches() { - const CREATED_PATCH_ID: &str = "22f8fbe09f7430579dd0730e4f2394362d844647"; + const CREATED_PATCH_ID: &str = "54505091ff3561466cfbe83e7e23c21cb1bb8a17"; let tmp = tempfile::tempdir().unwrap(); let ctx = test::contributor(tmp.path()); @@ -1459,7 +1457,7 @@ mod routes { "tags": [], "revisions": [ { - "id": "73efc59cd9f787deff0ae1629f47f1d90f307282", + "id": CREATED_PATCH_ID, "description": "", "reviews": [], } diff --git a/radicle-httpd/src/test.rs b/radicle-httpd/src/test.rs index 2c5214dc6..5ba342468 100644 --- a/radicle-httpd/src/test.rs +++ b/radicle-httpd/src/test.rs @@ -1,4 +1,3 @@ -use std::convert::TryInto as _; use std::path::Path; use std::str::FromStr; use std::sync::Arc; @@ -13,47 +12,77 @@ use tower::ServiceExt; use radicle::cob::issue::Issues; use radicle::cob::patch::{MergeTarget, Patches}; +use radicle::crypto::ssh::keystore::MemorySigner; +use radicle::crypto::ssh::Keystore; +use radicle::crypto::{KeyPair, Seed, Signer}; use radicle::git::{raw as git2, RefString}; +use radicle::profile::Home; use radicle::storage::ReadStorage; +use radicle::Storage; use radicle_crypto::test::signer::MockSigner; use crate::api::{auth, Context}; pub const HEAD: &str = "1e978d19f251cd9821d9d9a76d1bd436bf0690d5"; pub const HEAD_1: &str = "f604ce9fd5b7cc77b7609beda45ea8760bee78f7"; -pub const PATCH_ID: &str = "73d5187bb38835a232ce6ff41102a94bb92e5130"; -pub const ISSUE_ID: &str = "959afe03c1dbea4f47462bb7824584a78741f59c"; -pub const ISSUE_DISCUSSION_ID: &str = "e2874702515026daf62d4385cafb88fef1fad0c8"; -pub const ISSUE_COMMENT_ID: &str = "905d72c1f13f282f86cb93ce9f7eb9464a08ba79"; +pub const PATCH_ID: &str = "6ec9a764a888576abc7e582dbf82a31e23a9789d"; +pub const ISSUE_ID: &str = "5ad77fa3f476beed9a26f49b2b3b844e61bef792"; +pub const ISSUE_DISCUSSION_ID: &str = "f1dff128a22e8183a23516dd9812e72e80914c92"; +pub const ISSUE_COMMENT_ID: &str = "845218041bf9eb8155bfa4aaa8f0c91ce18e5c13"; pub const SESSION_ID: &str = "u9MGAkkfkMOv0uDDB2WeUHBT7HbsO2Dy"; pub const TIMESTAMP: u64 = 1671125284; pub const CONTRIBUTOR_RID: &str = "rad:z4XaCmN3jLSeiMvW15YTDpNbDHFhG"; pub const CONTRIBUTOR_PUB_KEY: &str = "did:key:z6Mkk7oqY4pPxhMmGEotDYsFo97vhCj85BLY1H256HrJmjN8"; -pub const CONTRIBUTOR_ISSUE_ID: &str = "b991ccfa866e164b81a4aa0a7e082357b6c75306"; +pub const CONTRIBUTOR_ISSUE_ID: &str = "f1dff128a22e8183a23516dd9812e72e80914c92"; const PASSWORD: &str = "radicle"; +/// Create a new profile. +pub fn profile(home: &Path, seed: [u8; 32]) -> radicle::Profile { + let home = Home::new(home).unwrap(); + let storage = Storage::open(home.storage()).unwrap(); + let keystore = Keystore::new(&home.keys()); + let keypair = KeyPair::from_seed(Seed::from(seed)); + + radicle::storage::git::transport::local::register(storage.clone()); + keystore + .store(keypair.clone(), "radicle", PASSWORD.to_owned()) + .unwrap(); + + radicle::Profile { + home, + storage, + keystore, + public_key: keypair.pk.into(), + } +} + pub fn seed(dir: &Path) -> Context { - seed_with_signer(dir, false) + let home = dir.join("radicle"); + let profile = profile(home.as_path(), [0xff; 32]); + let signer = Box::new(MockSigner::from_seed([0xff; 32])); + + seed_with_signer(dir, profile, &signer) } pub fn contributor(dir: &Path) -> Context { - seed_with_signer(dir, true) + let mut seed = [0xff; 32]; + *seed.last_mut().unwrap() = 0xee; + + let home = dir.join("radicle"); + let profile = profile(home.as_path(), seed); + let signer = MemorySigner::load(&profile.keystore, PASSWORD.to_owned().into()).unwrap(); + + seed_with_signer(dir, profile, &signer) } -fn seed_with_signer(dir: &Path, self_signer: bool) -> Context { +fn seed_with_signer(dir: &Path, profile: radicle::Profile, signer: &G) -> Context { let workdir = dir.join("hello-world"); - let rad_home = dir.join("radicle"); env::set_var("RAD_COMMIT_TIME", TIMESTAMP.to_string()); env::set_var("RAD_PASSPHRASE", PASSWORD); - env::set_var( - "RAD_SEED", - "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffee", - ); fs::create_dir_all(&workdir).unwrap(); - fs::create_dir_all(&rad_home).unwrap(); // add commits to workdir (repo) let repo = git2::Repository::init(&workdir).unwrap(); @@ -95,42 +124,24 @@ fn seed_with_signer(dir: &Path, self_signer: bool) -> Context { ) .unwrap(); - // eq. rad auth - let profile = - radicle::Profile::init(rad_home.try_into().unwrap(), PASSWORD.to_owned()).unwrap(); - - let mock_signer = MockSigner::from_seed([0xff; 32]); - // rad init let repo = git2::Repository::open(&workdir).unwrap(); let name = "hello-world".to_string(); let description = "Rad repository for tests".to_string(); - let signer = if self_signer { - profile.signer().unwrap() - } else { - Box::new(mock_signer) - }; let branch = RefString::try_from("master").unwrap(); - let (id, _, _) = radicle::rad::init( - &repo, - &name, - &description, - branch, - &signer, - &profile.storage, - ) - .unwrap(); + let (id, _, _) = + radicle::rad::init(&repo, &name, &description, branch, signer, &profile.storage).unwrap(); let storage = &profile.storage; let repo = storage.repository(id).unwrap(); let mut issues = Issues::open(&repo).unwrap(); - issues + let _ = issues .create( "Issue #1".to_string(), "Change 'hello world' to 'hello everyone'".to_string(), &[], &[], - &signer, + signer, ) .unwrap(); @@ -138,7 +149,7 @@ fn seed_with_signer(dir: &Path, self_signer: bool) -> Context { let mut patches = Patches::open(&repo).unwrap(); let oid = radicle::git::Oid::from_str(HEAD).unwrap(); let base = radicle::git::Oid::from_str(HEAD_1).unwrap(); - patches + let _ = patches .create( "A new `hello word`", "change `hello world` in README to something else", @@ -146,7 +157,7 @@ fn seed_with_signer(dir: &Path, self_signer: bool) -> Context { base, oid, &[], - &signer, + signer, ) .unwrap();