Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
cob: Evaluate COB state eagerly
cloudhead committed 2 years ago
commit 457b82f72325f554919b94d808628014e484494f
parent 65fa3cbb1bd3ddaa56ee8e19df8379345d366d96
30 files changed +876 -421
modified radicle-cob/src/backend/git/change.rs
@@ -2,6 +2,7 @@

use std::collections::BTreeMap;
use std::convert::TryFrom;
+
use std::iter;
use std::path::PathBuf;

use git_ext::author::Author;
@@ -95,7 +96,7 @@ impl change::Storage for git2::Repository {
    fn store<Signer>(
        &self,
        resource: Self::Parent,
-
        parents: Vec<Self::Parent>,
+
        mut parents: Vec<Self::Parent>,
        signer: &Signer,
        spec: store::Template<Self::ObjectId>,
    ) -> Result<Entry, Self::StoreError>
@@ -118,11 +119,19 @@ impl change::Storage for git2::Repository {
            ExtendedSignature::new(*key, sig)
        };

+
        // Make sure there are no duplicates in the parents list.
+
        parents.dedup();
+
        parents.sort();
+

        let (id, timestamp) = write_commit(
            self,
-
            resource,
-
            parents.clone(),
-
            tips,
+
            *resource,
+
            // Commit to tips, extra parents and resource.
+
            tips.iter()
+
                .cloned()
+
                .chain(parents.clone())
+
                .chain(iter::once(resource))
+
                .map(git2::Oid::from),
            message,
            signature.clone(),
            tree,
@@ -133,7 +142,7 @@ impl change::Storage for git2::Repository {
            revision: revision.into(),
            signature,
            resource,
-
            parents,
+
            parents: tips.into_iter().chain(parents).collect(),
            manifest,
            contents,
            timestamp,
@@ -242,31 +251,14 @@ fn load_contents(repo: &git2::Repository, tree: &git2::Tree) -> Result<Contents,
    NonEmpty::collect(ops.into_values()).ok_or_else(|| error::Load::NoChange(tree.id().into()))
}

-
fn write_commit<O>(
+
fn write_commit(
    repo: &git2::Repository,
-
    resource: O,
-
    parents: Vec<O>,
-
    tips: Vec<O>,
+
    resource: git2::Oid,
+
    parents: impl IntoIterator<Item = git2::Oid>,
    message: String,
    signature: ExtendedSignature,
    tree: git2::Tree,
-
) -> Result<(Oid, Timestamp), error::Create>
-
where
-
    O: AsRef<git2::Oid>,
-
{
-
    let resource = *resource.as_ref();
-
    // Add extra parents ensuring there are no duplicates.
-
    let mut parents = parents.iter().map(|o| *o.as_ref()).collect::<Vec<_>>();
-
    parents.sort();
-
    parents.dedup();
-

-
    let parents = tips
-
        .iter()
-
        .map(|o| *o.as_ref())
-
        .chain(parents.into_iter())
-
        .chain(std::iter::once(resource))
-
        .collect::<Vec<_>>();
-

+
) -> Result<(Oid, Timestamp), error::Create> {
    let trailers: Vec<OwnedTrailer> = vec![trailers::ResourceCommitTrailer::from(resource).into()];
    let author = repo.signature()?;
    let timestamp = author.when().seconds();
modified radicle-cob/src/change.rs
@@ -7,7 +7,5 @@ pub use store::{Contents, EntryId, Storage, Template, Timestamp};

use crate::signatures::ExtendedSignature;

-
/// A single change in the change graph. The layout of changes in the repository
-
/// is specified in the RFC (docs/rfc/0662-collaborative-objects.adoc)
-
/// under "Change Commits".
+
/// A single change in the change graph.
pub type Entry = store::Entry<Oid, Oid, ExtendedSignature>;
modified radicle-cob/src/change/store.rs
@@ -59,7 +59,7 @@ pub type Timestamp = u64;
/// A unique identifier for a history entry.
pub type EntryId = Oid;

-
#[derive(Clone, Debug)]
+
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Entry<Resource, Id, Signature> {
    /// The content address of the `Change` itself.
    pub id: Id,
modified radicle-cob/src/change_graph.rs
@@ -7,10 +7,18 @@ use git_ext::Oid;
use radicle_dag::Dag;

use crate::{
-
    change, object, signatures::ExtendedSignature, CollaborativeObject, Entry, History, ObjectId,
-
    TypeName,
+
    change, object, object::collaboration::Evaluate, signatures::ExtendedSignature,
+
    CollaborativeObject, Entry, EntryId, History, ObjectId, TypeName,
};

+
#[derive(Debug, thiserror::Error)]
+
pub enum EvaluateError {
+
    #[error("unable to initialize object: {0}")]
+
    Init(Box<dyn std::error::Error + Sync + Send + 'static>),
+
    #[error("invalid signature for entry '{0}'")]
+
    Signature(EntryId),
+
}
+

/// The graph of changes for a particular collaborative object
pub(super) struct ChangeGraph {
    object_id: ObjectId,
@@ -30,6 +38,7 @@ impl ChangeGraph {
        S: change::Storage<ObjectId = Oid, Parent = Oid, Signatures = ExtendedSignature>,
    {
        log::info!("loading object '{}' '{}'", typename, oid);
+

        let mut builder = GraphBuilder::default();
        let mut edges_to_process: Vec<(Oid, Oid)> = Vec::new();

@@ -81,40 +90,45 @@ impl ChangeGraph {

    /// Given a graph evaluate it to produce a collaborative object. This will
    /// filter out branches of the graph which do not have valid signatures.
-
    pub(crate) fn evaluate(self) -> CollaborativeObject {
+
    pub(crate) fn evaluate<S, T: Evaluate<S>>(
+
        mut self,
+
        store: &S,
+
    ) -> Result<CollaborativeObject<T>, EvaluateError> {
        let root = *self.object_id;
-
        let root_node = self
+
        let root = self
            .graph
            .get(&root)
            .expect("ChangeGraph::evaluate: root must be part of change graph");

-
        let manifest = root_node.manifest.clone();
-
        let graph = self
-
            .graph
-
            .fold(&[root], Dag::new(), |mut graph, _, change| {
-
                // Check the change signatures are valid.
-
                if !change.valid_signatures() {
-
                    return ControlFlow::Break(graph);
-
                }
-
                let entry = change.value.clone();
-
                let id = *entry.id();
-

-
                graph.node(id, entry);
-

-
                for k in &change.dependents {
-
                    graph.dependency(*k, id);
-
                }
-
                for k in &change.dependencies {
-
                    graph.dependency(id, *k);
-
                }
-
                ControlFlow::Continue(graph)
-
            });
+
        if !root.valid_signatures() {
+
            return Err(EvaluateError::Signature(root.id));
+
        }
+
        // Evaluate the root separately, since we can't have a COB without a valid root.
+
        // Then, traverse the graph starting from the root's dependents.
+
        let mut object =
+
            T::init(&root.value, store).map_err(|e| EvaluateError::Init(Box::new(e)))?;
+
        let children = Vec::from_iter(root.dependents.iter().cloned());
+
        let manifest = root.manifest.clone();
+
        let root = root.id;
+

+
        self.graph.prune(&children, |_, entry| {
+
            // Check the entry signatures are valid.
+
            if !entry.valid_signatures() {
+
                return ControlFlow::Break(());
+
            }
+
            // Apply the entry to the state, and if there's an error, prune that branch.
+
            if object.apply(entry, store).is_err() {
+
                return ControlFlow::Break(());
+
            }
+
            ControlFlow::Continue(())
+
        });

-
        CollaborativeObject {
+
        Ok(CollaborativeObject {
            manifest,
-
            history: History::new((*root).into(), graph),
+
            object,
+
            history: History::new(root, self.graph),
            id: self.object_id,
-
        }
+
        })
    }

    /// Get the tips of the collaborative object
modified radicle-cob/src/history.rs
@@ -39,16 +39,6 @@ impl History {
        Self::new(id, Dag::root(id, root))
    }

-
    /// Get the current history timestamp.
-
    /// This is the latest timestamp of any tip.
-
    pub fn timestamp(&self) -> Timestamp {
-
        self.graph
-
            .tips()
-
            .map(|(_, n)| n.timestamp)
-
            .max()
-
            .unwrap_or_default()
-
    }
-

    /// Get all the tips of the graph.
    pub fn tips(&self) -> BTreeSet<Oid> {
        self.graph.tips().map(|(_, entry)| *entry.id()).collect()
modified radicle-cob/src/lib.rs
@@ -91,7 +91,8 @@ pub use type_name::TypeName;

pub mod object;
pub use object::{
-
    create, get, info, list, remove, update, CollaborativeObject, Create, ObjectId, Update, Updated,
+
    create, get, info, list, remove, update, CollaborativeObject, Create, Evaluate, ObjectId,
+
    Update, Updated,
};

#[cfg(test)]
@@ -116,9 +117,9 @@ mod tests;
/// [`git2::Repository`]. It is expected that the underlying storage
/// for `object::Storage` will also be `git2::Repository`, but if not
/// please open an issue to change the definition of `Store` :)
-
pub trait Store<I = crypto::PublicKey>
+
pub trait Store
where
-
    Self: object::Storage<Identifier = I>
+
    Self: object::Storage
        + change::Storage<
            StoreError = git::change::error::Create,
            LoadError = git::change::error::Load,
modified radicle-cob/src/object.rs
@@ -9,8 +9,8 @@ use thiserror::Error;

pub mod collaboration;
pub use collaboration::{
-
    create, get, info, list, parse_refstr, remove, update, CollaborativeObject, Create, Update,
-
    Updated,
+
    create, get, info, list, parse_refstr, remove, update, CollaborativeObject, Create, Evaluate,
+
    Update, Updated,
};

pub mod storage;
modified radicle-cob/src/object/collaboration.rs
@@ -1,11 +1,12 @@
// Copyright © 2022 The Radicle Link Contributors
-

-
use std::collections::BTreeSet;
+
use std::convert::Infallible;
+
use std::fmt::Debug;

use git_ext::Oid;
+
use nonempty::NonEmpty;

use crate::change::store::{Manifest, Version};
-
use crate::{change, History, ObjectId, TypeName};
+
use crate::{change, Entry, History, ObjectId, TypeName};

pub mod error;

@@ -28,16 +29,22 @@ pub use update::{update, Update, Updated};

/// A collaborative object
#[derive(Debug, Clone, PartialEq, Eq)]
-
pub struct CollaborativeObject {
+
pub struct CollaborativeObject<T> {
    /// The manifest of this object
-
    pub(crate) manifest: Manifest,
-
    /// The CRDT history we know about for this object
-
    pub(crate) history: History,
+
    pub manifest: Manifest,
+
    /// The materialized object resulting from traversing the history.
+
    pub object: T,
+
    /// The history DAG.
+
    pub history: History,
    /// The id of the object
-
    pub(crate) id: ObjectId,
+
    pub id: ObjectId,
}

-
impl CollaborativeObject {
+
impl<T> CollaborativeObject<T> {
+
    pub fn object(&self) -> &T {
+
        &self.object
+
    }
+

    pub fn history(&self) -> &History {
        &self.history
    }
@@ -53,9 +60,30 @@ impl CollaborativeObject {
    pub fn manifest(&self) -> &Manifest {
        &self.manifest
    }
+
}
+

+
/// An object that can be built by evaluating a history.
+
pub trait Evaluate<R>: Sized + Debug + 'static {
+
    type Error: std::error::Error + Send + Sync + 'static;
+

+
    /// Initialize the object with the first (root) history entry.
+
    fn init(entry: &Entry, store: &R) -> Result<Self, Self::Error>;
+

+
    /// Apply a history entry to the evaluated state.
+
    fn apply(&mut self, entry: &Entry, store: &R) -> Result<(), Self::Error>;
+
}
+

+
impl<R> Evaluate<R> for NonEmpty<Entry> {
+
    type Error = Infallible;
+

+
    fn init(entry: &Entry, _store: &R) -> Result<Self, Self::Error> {
+
        Ok(Self::new(entry.clone()))
+
    }
+

+
    fn apply(&mut self, entry: &Entry, _store: &R) -> Result<(), Self::Error> {
+
        self.push(entry.clone());

-
    fn tips(&self) -> BTreeSet<Oid> {
-
        self.history.tips().into_iter().map(Oid::from).collect()
+
        Ok(())
    }
}

modified radicle-cob/src/object/collaboration/create.rs
@@ -1,8 +1,10 @@
// Copyright © 2022 The Radicle Link Contributors

use nonempty::NonEmpty;
+
use radicle_crypto::PublicKey;

use crate::Embed;
+
use crate::Evaluate;
use crate::Store;

use super::*;
@@ -51,16 +53,17 @@ impl Create {
///
/// The `args` are the metadata for this [`CollaborativeObject`]. See
/// [`Create`] for further information.
-
pub fn create<S, I, G>(
+
pub fn create<T, S, G>(
    storage: &S,
    signer: &G,
    resource: Oid,
    parents: Vec<Oid>,
-
    identifier: &S::Identifier,
+
    identifier: &PublicKey,
    args: Create,
-
) -> Result<CollaborativeObject, error::Create>
+
) -> Result<CollaborativeObject<T>, error::Create>
where
-
    S: Store<I>,
+
    T: Evaluate<S>,
+
    S: Store,
    G: crypto::Signer,
{
    let type_name = args.type_name.clone();
@@ -69,9 +72,10 @@ where
        .store(resource, parents, signer, args.template())
        .map_err(error::Create::from)?;
    let object_id = init_change.id().into();
+
    let object = T::init(&init_change, storage).map_err(error::Create::evaluate)?;

    storage
-
        .update(identifier, &type_name, &object_id, &init_change.id)
+
        .update(identifier, &type_name, &object_id, &object_id)
        .map_err(|err| error::Create::Refs { err: Box::new(err) })?;

    let history = History::new_from_root(init_change);
@@ -79,6 +83,7 @@ where
    Ok(CollaborativeObject {
        manifest: Manifest::new(type_name, version),
        history,
+
        object,
        id: object_id,
    })
}
modified radicle-cob/src/object/collaboration/error.rs
@@ -6,8 +6,8 @@ use crate::git;

#[derive(Debug, Error)]
pub enum Create {
-
    #[error("Invalid automerge history")]
-
    InvalidAutomergeHistory,
+
    #[error(transparent)]
+
    Evaluate(Box<dyn std::error::Error + Send + Sync + 'static>),
    #[error(transparent)]
    CreateChange(#[from] git::change::error::Create),
    #[error("failed to updated references for during object creation")]
@@ -21,6 +21,12 @@ pub enum Create {
    SignerIsNotAuthor,
}

+
impl Create {
+
    pub(crate) fn evaluate(err: impl std::error::Error + Send + Sync + 'static) -> Self {
+
        Self::Evaluate(Box::new(err))
+
    }
+
}
+

#[derive(Debug, Error)]
#[error("failed to remove object: {err}")]
pub struct Remove {
@@ -30,6 +36,8 @@ pub struct Remove {

#[derive(Debug, Error)]
pub enum Retrieve {
+
    #[error("object failed to evaluate: {0}")]
+
    Evaluate(Box<dyn std::error::Error + Send + Sync + 'static>),
    #[error(transparent)]
    Git(#[from] git2::Error),
    #[error("failed to get references during object retrieval")]
@@ -41,8 +49,16 @@ pub enum Retrieve {
    Io(#[from] std::io::Error),
}

+
impl Retrieve {
+
    pub(crate) fn evaluate(err: impl std::error::Error + Send + Sync + 'static) -> Self {
+
        Self::Evaluate(Box::new(err))
+
    }
+
}
+

#[derive(Debug, Error)]
pub enum Update {
+
    #[error("object failed to evaluate: {0}")]
+
    Evaluate(Box<dyn std::error::Error + Send + Sync + 'static>),
    #[error("no object found")]
    NoSuchObject,
    #[error(transparent)]
@@ -59,3 +75,9 @@ pub enum Update {
    #[error("signer must belong to the author")]
    SignerIsNotAuthor,
}
+

+
impl Update {
+
    pub(crate) fn evaluate(err: impl std::error::Error + Send + Sync + 'static) -> Self {
+
        Self::Evaluate(Box::new(err))
+
    }
+
}
modified radicle-cob/src/object/collaboration/get.rs
@@ -1,6 +1,6 @@
// Copyright © 2022 The Radicle Link Contributors

-
use crate::{change_graph::ChangeGraph, CollaborativeObject, ObjectId, Store, TypeName};
+
use crate::{change_graph::ChangeGraph, CollaborativeObject, Evaluate, ObjectId, Store, TypeName};

use super::error;

@@ -13,16 +13,20 @@ use super::error;
/// The `typename` is the type of object to be found, while the
/// `object_id` is the identifier for the particular object under that
/// type.
-
pub fn get<S, I>(
+
pub fn get<T, S>(
    storage: &S,
    typename: &TypeName,
    oid: &ObjectId,
-
) -> Result<Option<CollaborativeObject>, error::Retrieve>
+
) -> Result<Option<CollaborativeObject<T>>, error::Retrieve>
where
-
    S: Store<I>,
+
    T: Evaluate<S>,
+
    S: Store,
{
    let tip_refs = storage
        .objects(typename, oid)
        .map_err(|err| error::Retrieve::Refs { err: Box::new(err) })?;
-
    Ok(ChangeGraph::load(storage, tip_refs.iter(), typename, oid).map(|graph| graph.evaluate()))
+

+
    ChangeGraph::load(storage, tip_refs.iter(), typename, oid)
+
        .map(|graph| graph.evaluate(storage).map_err(error::Retrieve::evaluate))
+
        .transpose()
}
modified radicle-cob/src/object/collaboration/list.rs
@@ -1,6 +1,6 @@
// Copyright © 2022 The Radicle Link Contributors

-
use crate::{change_graph::ChangeGraph, CollaborativeObject, Store, TypeName};
+
use crate::{change_graph::ChangeGraph, CollaborativeObject, Evaluate, Store, TypeName};

use super::error;

@@ -11,12 +11,13 @@ use super::error;
/// [`Store`] for further information.
///
/// The `typename` is the type of objects to be listed.
-
pub fn list<S, I>(
+
pub fn list<T, S>(
    storage: &S,
    typename: &TypeName,
-
) -> Result<Vec<CollaborativeObject>, error::Retrieve>
+
) -> Result<Vec<CollaborativeObject<T>>, error::Retrieve>
where
-
    S: Store<I>,
+
    T: Evaluate<S>,
+
    S: Store,
{
    let references = storage
        .types(typename)
@@ -24,17 +25,20 @@ where
    log::trace!("loaded {} references", references.len());
    let mut result = Vec::new();
    for (oid, tip_refs) in references {
-
        log::trace!("loading object '{}'", oid);
+
        log::trace!("loading object '{oid}'");
        let loaded = ChangeGraph::load(storage, tip_refs.iter(), typename, &oid)
-
            .map(|graph| graph.evaluate());
+
            .map(|graph| graph.evaluate(storage).map_err(error::Retrieve::evaluate));

        match loaded {
-
            Some(obj) => {
-
                log::trace!("object '{}' found", oid);
+
            Some(Ok(obj)) => {
+
                log::trace!("object '{oid}' found");
                result.push(obj);
            }
+
            Some(Err(e)) => {
+
                log::trace!("object '{oid}' failed to load: {e}")
+
            }
            None => {
-
                log::trace!("object '{}' not found", oid);
+
                log::trace!("object '{oid}' not found");
            }
        }
    }
modified radicle-cob/src/object/collaboration/remove.rs
@@ -1,5 +1,7 @@
// Copyright © 2022 The Radicle Link Contributors

+
use radicle_crypto::PublicKey;
+

use crate::{ObjectId, Store, TypeName};

use super::error;
@@ -13,14 +15,14 @@ use super::error;
/// The `typename` is the type of object to be found, while the
/// `object_id` is the identifier for the particular object under that
/// type.
-
pub fn remove<S, I>(
+
pub fn remove<S>(
    storage: &S,
-
    identifier: &S::Identifier,
+
    identifier: &PublicKey,
    typename: &TypeName,
    oid: &ObjectId,
) -> Result<(), error::Remove>
where
-
    S: Store<I>,
+
    S: Store,
{
    storage
        .remove(identifier, typename, oid)
modified radicle-cob/src/object/collaboration/update.rs
@@ -2,21 +2,22 @@

use git_ext::Oid;
use nonempty::NonEmpty;
+
use radicle_crypto::PublicKey;

use crate::{
-
    change, change_graph::ChangeGraph, history::EntryId, CollaborativeObject, Embed, ObjectId,
-
    Store, TypeName,
+
    change, change_graph::ChangeGraph, history::EntryId, CollaborativeObject, Embed, Evaluate,
+
    ObjectId, Store, TypeName,
};

use super::error;

/// Result of an `update` operation.
#[derive(Debug)]
-
pub struct Updated {
+
pub struct Updated<T> {
    /// The new head commit of the DAG.
    pub head: Oid,
    /// The newly updated collaborative object.
-
    pub object: CollaborativeObject,
+
    pub object: CollaborativeObject<T>,
    /// Entry parents.
    pub parents: Vec<EntryId>,
}
@@ -54,16 +55,17 @@ pub struct Update {
///
/// The `args` are the metadata for this [`CollaborativeObject`]
/// udpate. See [`Update`] for further information.
-
pub fn update<S, I, G>(
+
pub fn update<T, S, G>(
    storage: &S,
    signer: &G,
    resource: Oid,
    parents: Vec<Oid>,
-
    identifier: &S::Identifier,
+
    identifier: &PublicKey,
    args: Update,
-
) -> Result<Updated, error::Update>
+
) -> Result<Updated<T>, error::Update>
where
-
    S: Store<I>,
+
    T: Evaluate<S>,
+
    S: Store,
    G: crypto::Signer,
{
    let Update {
@@ -78,32 +80,42 @@ where
        .objects(typename, &object_id)
        .map_err(|err| error::Update::Refs { err: Box::new(err) })?;

-
    let mut object = ChangeGraph::load(storage, existing_refs.iter(), typename, &object_id)
-
        .map(|graph| graph.evaluate())
+
    let graph = ChangeGraph::load(storage, existing_refs.iter(), typename, &object_id)
        .ok_or(error::Update::NoSuchObject)?;
+
    let mut object: CollaborativeObject<T> =
+
        graph.evaluate(storage).map_err(error::Update::evaluate)?;

-
    let change = storage.store(
+
    // Create a commit for this change, but don't update any references yet.
+
    let entry = storage.store(
        resource,
        parents,
        signer,
        change::Template {
-
            tips: object.tips().iter().cloned().collect(),
+
            tips: object.history.tips().into_iter().collect(),
            embeds,
            contents: changes,
            type_name: typename.clone(),
            message,
        },
    )?;
+
    let head = entry.id;
+
    let parents = entry.parents.to_vec();

+
    // Try to apply this change to our object. This prevents storing invalid updates.
+
    // Note that if this returns with an error, we are left with an unreachable
+
    // commit object created above. This is fine, as it will eventually get
+
    // garbage-collected by Git.
+
    object
+
        .object
+
        .apply(&entry, storage)
+
        .map_err(error::Update::evaluate)?;
+
    object.history.extend(entry);
+

+
    // Here we actually update the references to point to the new update.
    storage
-
        .update(identifier, typename, &object_id, &change.id)
+
        .update(identifier, typename, &object_id, &head)
        .map_err(|err| error::Update::Refs { err: Box::new(err) })?;

-
    let parents = change.parents.to_vec();
-
    let head = change.id;
-

-
    object.history.extend(change);
-

    Ok(Updated {
        object,
        head,
modified radicle-cob/src/object/storage.rs
@@ -4,6 +4,7 @@ use std::{collections::BTreeMap, error::Error};

use git_ext::ref_format::RefString;
use git_ext::Oid;
+
use radicle_crypto::PublicKey;

use crate::change::EntryId;
use crate::{ObjectId, TypeName};
@@ -58,8 +59,6 @@ pub trait Storage {
    type UpdateError: Error + Send + Sync + 'static;
    type RemoveError: Error + Send + Sync + 'static;

-
    type Identifier;
-

    /// Get all references which point to a head of the change graph for a
    /// particular object
    fn objects(
@@ -75,7 +74,7 @@ pub trait Storage {
    /// Update a ref to a particular collaborative object
    fn update(
        &self,
-
        identifier: &Self::Identifier,
+
        identifier: &PublicKey,
        typename: &TypeName,
        object_id: &ObjectId,
        entry: &EntryId,
@@ -84,7 +83,7 @@ pub trait Storage {
    /// Remove a ref to a particular collaborative object
    fn remove(
        &self,
-
        identifier: &Self::Identifier,
+
        identifier: &PublicKey,
        typename: &TypeName,
        object_id: &ObjectId,
    ) -> Result<(), Self::RemoveError>;
modified radicle-cob/src/test/storage.rs
@@ -1,5 +1,6 @@
use std::{collections::BTreeMap, convert::TryFrom as _};

+
use radicle_crypto::PublicKey;
use tempfile::TempDir;

use crate::{
@@ -8,8 +9,6 @@ use crate::{
    ObjectId, Store,
};

-
use super::identity::Urn;
-

pub mod error {
    use thiserror::Error;

@@ -56,7 +55,7 @@ impl Storage {
    }
}

-
impl Store<Urn> for Storage {}
+
impl Store for Storage {}

impl change::Storage for Storage {
    type StoreError = <git2::Repository as change::Storage>::StoreError;
@@ -106,8 +105,6 @@ impl object::Storage for Storage {
    type UpdateError = git2::Error;
    type RemoveError = git2::Error;

-
    type Identifier = Urn;
-

    fn objects(
        &self,
        typename: &crate::TypeName,
@@ -151,17 +148,12 @@ impl object::Storage for Storage {

    fn update(
        &self,
-
        identifier: &Self::Identifier,
+
        identifier: &PublicKey,
        typename: &crate::TypeName,
        object_id: &ObjectId,
        entry: &change::EntryId,
    ) -> Result<(), Self::UpdateError> {
-
        let name = format!(
-
            "refs/rad/{}/cobs/{}/{}",
-
            identifier.to_path(),
-
            typename,
-
            object_id
-
        );
+
        let name = format!("refs/rad/{}/cobs/{}/{}", identifier, typename, object_id);
        self.raw
            .reference(&name, (*entry).into(), true, "new change")?;
        Ok(())
@@ -169,16 +161,11 @@ impl object::Storage for Storage {

    fn remove(
        &self,
-
        identifier: &Self::Identifier,
+
        identifier: &PublicKey,
        typename: &crate::TypeName,
        object_id: &ObjectId,
    ) -> Result<(), Self::RemoveError> {
-
        let name = format!(
-
            "refs/rad/{}/cobs/{}/{}",
-
            identifier.to_path(),
-
            typename,
-
            object_id
-
        );
+
        let name = format!("refs/rad/{}/cobs/{}/{}", identifier, typename, object_id);
        self.raw.find_reference(&name)?.delete()?;

        Ok(())
modified radicle-cob/src/tests.rs
@@ -1,13 +1,13 @@
use std::ops::ControlFlow;

use crypto::test::signer::MockSigner;
+
use crypto::{PublicKey, Signer};
use git_ext::ref_format::{refname, Component, RefString};
-
use nonempty::nonempty;
+
use nonempty::{nonempty, NonEmpty};
use qcheck::Arbitrary;
-
use radicle_crypto::Signer;

use crate::{
-
    create, get, list, object, test::arbitrary::Invalid, update, Create, ObjectId, TypeName,
+
    create, get, list, object, test::arbitrary::Invalid, update, Create, Entry, ObjectId, TypeName,
    Update, Updated, Version,
};

@@ -24,12 +24,12 @@ fn roundtrip() {
        person: terry,
    };
    let typename = "xyz.rad.issue".parse::<TypeName>().unwrap();
-
    let cob = create(
+
    let cob = create::<NonEmpty<Entry>, _, _>(
        &storage,
        &signer,
        proj.project.content_id,
        vec![],
-
        &proj.identifier(),
+
        signer.public_key(),
        Create {
            contents: nonempty!(Vec::new()),
            type_name: typename.clone(),
@@ -58,12 +58,12 @@ fn list_cobs() {
        person: terry,
    };
    let typename = "xyz.rad.issue".parse::<TypeName>().unwrap();
-
    let issue_1 = create(
+
    let issue_1 = create::<NonEmpty<Entry>, _, _>(
        &storage,
        &signer,
        proj.project.content_id,
        vec![],
-
        &proj.identifier(),
+
        signer.public_key(),
        Create {
            contents: nonempty!(b"issue 1".to_vec()),
            type_name: typename.clone(),
@@ -79,7 +79,7 @@ fn list_cobs() {
        &signer,
        proj.project.content_id,
        vec![],
-
        &proj.identifier(),
+
        signer.public_key(),
        Create {
            contents: nonempty!(b"issue 2".to_vec()),
            type_name: typename.clone(),
@@ -110,12 +110,12 @@ fn update_cob() {
        person: terry,
    };
    let typename = "xyz.rad.issue".parse::<TypeName>().unwrap();
-
    let cob = create(
+
    let cob = create::<NonEmpty<Entry>, _, _>(
        &storage,
        &signer,
        proj.project.content_id,
        vec![],
-
        &proj.identifier(),
+
        signer.public_key(),
        Create {
            contents: nonempty!(Vec::new()),
            type_name: typename.clone(),
@@ -126,7 +126,7 @@ fn update_cob() {
    )
    .unwrap();

-
    let not_expected = get(&storage, &typename, cob.id())
+
    let not_expected = get::<NonEmpty<Entry>, _>(&storage, &typename, cob.id())
        .unwrap()
        .expect("BUG: cob was missing");

@@ -135,7 +135,7 @@ fn update_cob() {
        &signer,
        proj.project.content_id,
        vec![],
-
        &proj.identifier(),
+
        signer.public_key(),
        Update {
            changes: nonempty!(b"issue 1".to_vec()),
            object_id: *cob.id(),
@@ -151,7 +151,7 @@ fn update_cob() {
        .expect("BUG: cob was missing");

    assert_ne!(object, not_expected);
-
    assert_eq!(object, expected);
+
    assert_eq!(object, expected, "{object:#?} {expected:#?}");
}

#[test]
@@ -171,12 +171,12 @@ fn traverse_cobs() {
        person: neil,
    };
    let typename = "xyz.rad.issue".parse::<TypeName>().unwrap();
-
    let cob = create(
+
    let cob = create::<NonEmpty<Entry>, _, _>(
        &storage,
        &terry_signer,
        terry_proj.project.content_id,
        vec![],
-
        &terry_proj.identifier(),
+
        terry_signer.public_key(),
        Create {
            contents: nonempty!(b"issue 1".to_vec()),
            type_name: typename.clone(),
@@ -188,19 +188,19 @@ fn traverse_cobs() {
    .unwrap();
    copy_to(
        storage.as_raw(),
-
        &terry_proj,
+
        terry_signer.public_key(),
        &neil_proj,
        &typename,
        *cob.id(),
    )
    .unwrap();

-
    let Updated { object, .. } = update(
+
    let Updated { object, .. } = update::<NonEmpty<Entry>, _, _>(
        &storage,
        &neil_signer,
        neil_proj.project.content_id,
        vec![],
-
        &neil_proj.identifier(),
+
        neil_signer.public_key(),
        Update {
            changes: nonempty!(b"issue 2".to_vec()),
            object_id: *cob.id(),
@@ -306,18 +306,13 @@ fn gen<T: Arbitrary>(size: usize) -> T {

fn copy_to(
    repo: &git2::Repository,
-
    from: &test::RemoteProject,
+
    from: &PublicKey,
    to: &test::RemoteProject,
    typename: &TypeName,
    object: ObjectId,
) -> Result<(), git2::Error> {
    let original = {
-
        let name = format!(
-
            "refs/rad/{}/cobs/{}/{}",
-
            from.identifier().to_path(),
-
            typename,
-
            object
-
        );
+
        let name = format!("refs/rad/{}/cobs/{}/{}", from, typename, object);
        let r = repo.find_reference(&name)?;
        r.target().unwrap()
    };
modified radicle-dag/src/lib.rs
@@ -186,6 +186,37 @@ impl<K: Ord + Copy, V> Dag<K, V> {
    /// Fold over the graph in topological order, pruning branches along the way.
    ///
    /// To continue traversing a branch, return [`ControlFlow::Continue`] from the
+
    /// filter function. To stop traversal of a branch and prune it,
+
    /// return [`ControlFlow::Break`].
+
    pub fn prune<F>(&mut self, roots: &[K], mut filter: F)
+
    where
+
        F: for<'r> FnMut(&'r K, &'r Node<K, V>) -> ControlFlow<()>,
+
    {
+
        let mut visited = BTreeSet::new();
+
        let mut queue = VecDeque::<K>::from_iter(roots.iter().cloned());
+

+
        while let Some(next) = queue.pop_front() {
+
            if !visited.insert(next) {
+
                continue;
+
            }
+
            if let Some(node) = self.graph.get(&next) {
+
                match filter(&next, node) {
+
                    ControlFlow::Continue(()) => {
+
                        queue.extend(node.dependents.iter().cloned());
+
                    }
+
                    ControlFlow::Break(()) => {
+
                        // When pruning a node, we remove all transitive dependents on
+
                        // that node.
+
                        self.remove(&next);
+
                    }
+
                }
+
            }
+
        }
+
    }
+

+
    /// Fold over the graph in topological order, skipping certain branches.
+
    ///
+
    /// To continue traversing a branch, return [`ControlFlow::Continue`] from the
    /// filter function. To stop traversal of a branch, return [`ControlFlow::Break`].
    pub fn fold<A, F>(&self, roots: &[K], mut acc: A, mut filter: F) -> A
    where
@@ -208,6 +239,7 @@ impl<K: Ord + Copy, V> Dag<K, V> {
                        // When filtering out a node, we filter out all transitive dependents on
                        // that node by adding them to the already visited list.
                        visited.extend(self.descendants_of(node));
+

                        acc = a;
                    }
                }
@@ -216,6 +248,30 @@ impl<K: Ord + Copy, V> Dag<K, V> {
        acc
    }

+
    /// Remove a node from the graph, and all its dependents.
+
    pub fn remove(&mut self, key: &K) -> Option<Node<K, V>> {
+
        if let Some(node) = self.graph.remove(key) {
+
            self.tips.remove(key);
+
            self.roots.remove(key);
+

+
            for k in &node.dependencies {
+
                if let Some(dependency) = self.graph.get_mut(k) {
+
                    dependency.dependents.remove(key);
+

+
                    if dependency.dependents.is_empty() {
+
                        self.tips.insert(*k);
+
                    }
+
                }
+
            }
+
            for k in &node.dependents {
+
                self.remove(k);
+
            }
+
            Some(node)
+
        } else {
+
            None
+
        }
+
    }
+

    fn descendants_of(&self, from: &Node<K, V>) -> Vec<K> {
        let mut visited = BTreeSet::new();
        let mut stack = VecDeque::new();
@@ -535,4 +591,78 @@ mod tests {
        });
        assert_eq!(acc, vec!["R", "A2"]);
    }
+

+
    #[test]
+
    fn test_remove() {
+
        let mut dag = Dag::new();
+

+
        dag.node("R", ());
+
        dag.node("A1", ());
+
        dag.node("A2", ());
+
        dag.node("A3", ());
+
        dag.node("B1", ());
+
        dag.node("C1", ());
+
        dag.node("D1", ());
+

+
        dag.dependency("A1", "R");
+
        dag.dependency("A2", "R");
+
        dag.dependency("A3", "A2");
+
        dag.dependency("B1", "A1");
+
        dag.dependency("B1", "A2");
+
        dag.dependency("C1", "B1");
+
        dag.dependency("C1", "A3");
+
        dag.dependency("D1", "C1");
+
        dag.dependency("D1", "A2");
+

+
        dag.remove(&"C1");
+
        assert!(dag.get(&"C1").is_none());
+
        assert!(dag.get(&"D1").is_none());
+
        assert!(!dag.tips.contains(&"D1"));
+
        assert_eq!(dag.tips.iter().collect::<Vec<_>>(), vec![&"A3", &"B1"]);
+

+
        dag.remove(&"A3");
+
        assert_eq!(dag.tips.iter().collect::<Vec<_>>(), vec![&"B1"]);
+

+
        dag.remove(&"A1");
+
        assert!(dag.get(&"A1").is_none());
+
        assert!(dag.get(&"B1").is_none());
+
        assert!(dag.get(&"A2").is_some());
+
        assert_eq!(dag.tips.iter().collect::<Vec<_>>(), vec![&"A2"]);
+

+
        dag.remove(&"R");
+
        assert!(dag.is_empty());
+
        assert!(dag.tips.is_empty());
+
        assert!(dag.roots.is_empty());
+
    }
+

+
    #[test]
+
    fn test_prune() {
+
        let mut dag = Dag::new();
+

+
        dag.node("R", ());
+
        dag.node("A1", ());
+
        dag.node("A2", ());
+
        dag.node("B1", ());
+
        dag.node("C1", ());
+
        dag.node("D1", ());
+

+
        dag.dependency("A1", "R");
+
        dag.dependency("A2", "R");
+
        dag.dependency("B1", "A1");
+
        dag.dependency("C1", "B1");
+
        dag.dependency("D1", "C1");
+
        dag.dependency("D1", "A2");
+

+
        let a1 = dag.get(&"A1").unwrap();
+
        assert_eq!(dag.descendants_of(a1), vec!["B1", "C1", "D1"]);
+

+
        dag.prune(&["R"], |key, _| {
+
            if key == &"B1" {
+
                ControlFlow::Break(())
+
            } else {
+
                ControlFlow::Continue(())
+
            }
+
        });
+
        assert_eq!(dag.sorted(|a, b| a.cmp(b)), vec!["R", "A1", "A2"]);
+
    }
}
modified radicle-remote-helper/src/list.rs
@@ -23,7 +23,10 @@ pub enum Error {
}

/// List refs for fetching (`git fetch` and `git ls-remote`).
-
pub fn for_fetch<R: ReadRepository + cob::Store>(url: &Url, stored: &R) -> Result<(), Error> {
+
pub fn for_fetch<R: ReadRepository + cob::Store + 'static>(
+
    url: &Url,
+
    stored: &R,
+
) -> Result<(), Error> {
    if let Some(namespace) = url.namespace {
        // Listing namespaced refs.
        for (name, oid) in stored.references_of(&namespace)? {
@@ -68,7 +71,7 @@ pub fn for_push<R: ReadRepository>(profile: &Profile, stored: &R) -> Result<(),
}

/// List canonical patch references. These are magic refs that can be used to pull patch updates.
-
fn patch_refs<R: ReadRepository + cob::Store>(stored: &R) -> Result<(), Error> {
+
fn patch_refs<R: ReadRepository + cob::Store + 'static>(stored: &R) -> Result<(), Error> {
    let patches = radicle::cob::patch::Patches::open(stored)?;
    for patch in patches.all()? {
        let Ok((id, patch)) = patch else {
modified radicle/src/cob.rs
@@ -1,3 +1,4 @@
+
#![warn(clippy::unwrap_used)]
pub mod common;
pub mod identity;
pub mod issue;
@@ -11,7 +12,8 @@ pub mod test;

pub use cob::{
    change, history::EntryId, object, object::collaboration::error, CollaborativeObject, Contents,
-
    Create, Embed, Entry, History, Manifest, ObjectId, Store, TypeName, Update, Updated, Version,
+
    Create, Embed, Entry, Evaluate, History, Manifest, ObjectId, Store, TypeName, Update, Updated,
+
    Version,
};
pub use cob::{create, get, list, remove, update};
pub use common::*;
modified radicle/src/cob/common.rs
@@ -304,6 +304,7 @@ impl From<bool> for Authorization {
}

#[cfg(test)]
+
#[allow(clippy::unwrap_used)]
mod test {
    use super::*;

modified radicle/src/cob/identity.rs
@@ -11,8 +11,8 @@ use thiserror::Error;

use crate::{
    cob::{
-
        self,
-
        store::{self, FromHistory as _, HistoryAction, Transaction},
+
        self, op,
+
        store::{self, Cob, CobAction, Transaction},
        Reaction, Timestamp,
    },
    identity::{doc::DocError, Did, Identity, IdentityError},
@@ -102,7 +102,7 @@ pub enum Action {
    },
}

-
impl HistoryAction for Action {}
+
impl CobAction for Action {}

/// Error applying an operation onto a state.
#[derive(Error, Debug)]
@@ -160,6 +160,8 @@ pub enum Error {
    Apply(#[from] ApplyError),
    #[error("store: {0}")]
    Store(#[from] store::Error),
+
    #[error("op decoding failed: {0}")]
+
    Op(#[from] op::OpEncodingError),
}

/// Propose a new [`Doc`] for an [`Identity`]. The proposal can be
@@ -315,7 +317,7 @@ impl Proposal {
    }
}

-
impl store::FromHistory for Proposal {
+
impl store::Cob for Proposal {
    type Action = Action;
    type Error = ApplyError;

@@ -323,13 +325,13 @@ impl store::FromHistory for Proposal {
        &TYPENAME
    }

-
    fn init<R: ReadRepository>(op: Op, repo: &R) -> Result<Self, Self::Error> {
+
    fn from_root<R: ReadRepository>(op: Op, repo: &R) -> Result<Self, Self::Error> {
        let mut identity = Self::default();
-
        identity.apply(op, repo)?;
+
        identity.op(op, repo)?;
        Ok(identity)
    }

-
    fn apply<R: ReadRepository>(&mut self, op: Op, _repo: &R) -> Result<(), Self::Error> {
+
    fn op<R: ReadRepository>(&mut self, op: Op, _repo: &R) -> Result<(), ApplyError> {
        let id = op.id;
        let author = Author::new(op.author);
        let timestamp = op.timestamp;
@@ -438,6 +440,23 @@ impl store::FromHistory for Proposal {
    }
}

+
impl<R: ReadRepository> cob::Evaluate<R> for Proposal {
+
    type Error = Error;
+

+
    fn init(entry: &cob::Entry, repo: &R) -> Result<Self, Self::Error> {
+
        let op = Op::try_from(entry)?;
+
        let object = Proposal::from_root(op, repo)?;
+

+
        Ok(object)
+
    }
+

+
    fn apply(&mut self, entry: &cob::Entry, repo: &R) -> Result<(), Self::Error> {
+
        let op = Op::try_from(entry)?;
+

+
        self.op(op, repo).map_err(Error::Apply)
+
    }
+
}
+

mod lookup {
    use super::*;

@@ -550,7 +569,7 @@ impl Revision {
    }
}

-
impl store::Transaction<Proposal> {
+
impl<R: ReadRepository> store::Transaction<Proposal, R> {
    pub fn accept(
        &mut self,
        revision: RevisionId,
@@ -640,13 +659,13 @@ where
    ) -> Result<EntryId, Error>
    where
        G: Signer,
-
        F: FnOnce(&mut Transaction<Proposal>) -> Result<(), store::Error>,
+
        F: FnOnce(&mut Transaction<Proposal, R>) -> Result<(), store::Error>,
    {
-
        let mut tx = Transaction::new(*signer.public_key());
+
        let mut tx = Transaction::default();
        operations(&mut tx)?;
-
        let (ops, commit) = tx.commit(message, self.id, &mut self.store.raw, signer)?;

-
        self.proposal.apply(ops, self.store.as_ref())?;
+
        let (proposal, commit) = tx.commit(message, self.id, &mut self.store.raw, signer)?;
+
        self.proposal = proposal;

        Ok(commit)
    }
modified radicle/src/cob/issue.rs
@@ -9,10 +9,10 @@ use thiserror::Error;
use crate::cob;
use crate::cob::common::{Author, Authorization, Label, Reaction, Timestamp, Uri};
use crate::cob::store::Transaction;
-
use crate::cob::store::{FromHistory as _, HistoryAction};
+
use crate::cob::store::{Cob, CobAction};
use crate::cob::thread;
use crate::cob::thread::{Comment, CommentId, Thread};
-
use crate::cob::{store, ActorId, Embed, EntryId, ObjectId, TypeName};
+
use crate::cob::{op, store, ActorId, Embed, EntryId, ObjectId, TypeName};
use crate::crypto::Signer;
use crate::git;
use crate::identity::doc::{Doc, DocError};
@@ -35,20 +35,22 @@ pub enum Error {
    /// Error loading the identity document.
    #[error("identity doc failed to load: {0}")]
    Doc(#[from] DocError),
-
    #[error("description missing")]
-
    DescriptionMissing,
    #[error("thread apply failed: {0}")]
    Thread(#[from] thread::Error),
    #[error("store: {0}")]
    Store(#[from] store::Error),
-
    #[error("history: {0}")]
-
    History(Box<dyn std::error::Error + Sync + Send + 'static>),
    /// Action not authorized.
    #[error("{0} not authorized to apply {1:?}")]
    NotAuthorized(ActorId, Action),
+
    /// Action not allowed.
+
    #[error("action is not allowed: {0}")]
+
    NotAllowed(EntryId),
    /// General error initializing an issue.
    #[error("initialization failed: {0}")]
    Init(&'static str),
+
    /// Error decoding an operation.
+
    #[error("op decoding failed: {0}")]
+
    Op(#[from] op::OpEncodingError),
}

/// Reason why an issue was closed.
@@ -113,7 +115,7 @@ pub struct Issue {
    pub(super) thread: Thread,
}

-
impl store::FromHistory for Issue {
+
impl store::Cob for Issue {
    type Action = Action;
    type Error = Error;

@@ -121,7 +123,7 @@ impl store::FromHistory for Issue {
        &TYPENAME
    }

-
    fn init<R: ReadRepository>(op: Op, repo: &R) -> Result<Self, Self::Error> {
+
    fn from_root<R: ReadRepository>(op: Op, repo: &R) -> Result<Self, Self::Error> {
        let mut actions = op.actions.into_iter();
        let Some(Action::Comment { body, reply_to: None, embeds }) = actions.next() else {
            return Err(Error::Init("the first action must be of type `comment`"));
@@ -136,10 +138,10 @@ impl store::FromHistory for Issue {
        Ok(issue)
    }

-
    fn apply<R: ReadRepository>(&mut self, op: Op, repo: &R) -> Result<(), Error> {
+
    fn op<R: ReadRepository>(&mut self, op: Op, repo: &R) -> Result<(), Error> {
        let doc = repo.identity_doc_at(op.identity)?.verified()?;
        for action in op.actions {
-
            match self.authorization(&action, &op.author, &doc) {
+
            match self.authorization(&action, &op.author, &doc)? {
                Authorization::Allow => {
                    self.action(action, op.id, op.author, op.timestamp, op.identity, repo)?;
                }
@@ -155,6 +157,23 @@ impl store::FromHistory for Issue {
    }
}

+
impl<R: ReadRepository> cob::Evaluate<R> for Issue {
+
    type Error = Error;
+

+
    fn init(entry: &cob::Entry, repo: &R) -> Result<Self, Self::Error> {
+
        let op = Op::try_from(entry)?;
+
        let object = Issue::from_root(op, repo)?;
+

+
        Ok(object)
+
    }
+

+
    fn apply(&mut self, entry: &cob::Entry, repo: &R) -> Result<(), Self::Error> {
+
        let op = Op::try_from(entry)?;
+

+
        self.op(op, repo)
+
    }
+
}
+

impl Issue {
    /// Construct a new issue.
    pub fn new(thread: Thread) -> Self {
@@ -222,14 +241,13 @@ impl Issue {
        action: &Action,
        actor: &ActorId,
        doc: &Doc<Verified>,
-
    ) -> Authorization {
+
    ) -> Result<Authorization, Error> {
        if doc.is_delegate(actor) {
            // A delegate is authorized to do all actions.
-
            return Authorization::Allow;
+
            return Ok(Authorization::Allow);
        }
        let author: ActorId = *self.author().id().as_key();
-

-
        match action {
+
        let outcome = match action {
            // Only delegate can assign someone to an issue.
            Action::Assign { .. } => Authorization::Deny,
            // Issue authors can edit their own issues.
@@ -245,15 +263,20 @@ impl Issue {
            Action::Comment { .. } => Authorization::Allow,
            // All roles can edit or redact their own comments.
            Action::CommentEdit { id, .. } | Action::CommentRedact { id, .. } => {
-
                if let Some(comment) = self.comment(id) {
-
                    Authorization::from(*actor == comment.author())
+
                if let Some(comment) = self.thread.comments.get(id) {
+
                    if let Some(comment) = comment {
+
                        Authorization::from(*actor == comment.author())
+
                    } else {
+
                        Authorization::Unknown
+
                    }
                } else {
-
                    Authorization::Unknown
+
                    return Err(Error::Thread(thread::Error::Missing(*id)));
                }
            }
            // All roles can react to a comment on an issue.
            Action::CommentReact { .. } => Authorization::Allow,
-
        }
+
        };
+
        Ok(outcome)
    }
}

@@ -301,6 +324,10 @@ impl Issue {
                thread::edit(&mut self.thread, entry, id, timestamp, body, embeds)?;
            }
            Action::CommentRedact { id } => {
+
                let (root, _) = self.root();
+
                if id == *root {
+
                    return Err(Error::NotAllowed(entry));
+
                }
                thread::redact(&mut self.thread, entry, id)?;
            }
            Action::CommentReact {
@@ -323,7 +350,7 @@ impl Deref for Issue {
    }
}

-
impl store::Transaction<Issue> {
+
impl<R: ReadRepository> store::Transaction<Issue, R> {
    /// Assign DIDs to the issue.
    pub fn assign(&mut self, assignees: impl IntoIterator<Item = Did>) -> Result<(), store::Error> {
        self.push(Action::Assign {
@@ -355,6 +382,11 @@ impl store::Transaction<Issue> {
        })
    }

+
    /// Redact a comment.
+
    pub fn redact_comment(&mut self, id: CommentId) -> Result<(), store::Error> {
+
        self.push(Action::CommentRedact { id })
+
    }
+

    /// Lifecycle an issue.
    pub fn lifecycle(&mut self, state: State) -> Result<(), store::Error> {
        self.push(Action::Lifecycle { state })
@@ -473,9 +505,7 @@ where
        embeds: impl IntoIterator<Item = Embed>,
        signer: &G,
    ) -> Result<EntryId, Error> {
-
        let Some((id, _)) = self.thread.comments().next() else {
-
            return Err(Error::DescriptionMissing);
-
        };
+
        let (id, _) = self.root();
        let id = *id;
        self.transaction("Edit description", signer, |tx| {
            tx.edit_comment(id, description, embeds.into_iter().collect())
@@ -495,15 +525,20 @@ where
        embeds: impl IntoIterator<Item = Embed>,
        signer: &G,
    ) -> Result<EntryId, Error> {
-
        assert!(
-
            self.thread.comment(&reply_to).is_some(),
-
            "Comment {reply_to} not found"
-
        );
        self.transaction("Comment", signer, |tx| {
            tx.comment(body, reply_to, embeds.into_iter().collect())
        })
    }

+
    /// Redact a comment.
+
    pub fn redact_comment<G: Signer>(
+
        &mut self,
+
        id: CommentId,
+
        signer: &G,
+
    ) -> Result<EntryId, Error> {
+
        self.transaction("Redact comment", signer, |tx| tx.redact_comment(id))
+
    }
+

    /// Label an issue.
    pub fn label<G: Signer>(
        &mut self,
@@ -532,13 +567,13 @@ where
    ) -> Result<EntryId, Error>
    where
        G: Signer,
-
        F: FnOnce(&mut Transaction<Issue>) -> Result<(), store::Error>,
+
        F: FnOnce(&mut Transaction<Issue, R>) -> Result<(), store::Error>,
    {
-
        let mut tx = Transaction::new(*signer.public_key());
+
        let mut tx = Transaction::default();
        operations(&mut tx)?;
-
        let (ops, commit) = tx.commit(message, self.id, &mut self.store.raw, signer)?;

-
        self.issue.apply(ops, self.store.as_ref())?;
+
        let (issue, commit) = tx.commit(message, self.id, &mut self.store.raw, signer)?;
+
        self.issue = issue;

        Ok(commit)
    }
@@ -710,9 +745,10 @@ pub enum Action {
    },
}

-
impl HistoryAction for Action {}
+
impl CobAction for Action {}

#[cfg(test)]
+
#[allow(clippy::unwrap_used)]
mod test {
    use pretty_assertions::assert_eq;

@@ -1187,6 +1223,38 @@ mod test {
    }

    #[test]
+
    fn test_issue_comment_redact() {
+
        let test::setup::NodeWithRepo { node, repo, .. } = test::setup::NodeWithRepo::default();
+
        let mut issues = Issues::open(&*repo).unwrap();
+
        let mut issue = issues
+
            .create(
+
                "My first issue",
+
                "Blah blah blah.",
+
                &[],
+
                &[],
+
                [],
+
                &node.signer,
+
            )
+
            .unwrap();
+

+
        // The root thread op id is always the same.
+
        let (c0, _) = issue.root();
+
        let c0 = *c0;
+

+
        let comment = issue
+
            .comment("Ho ho ho.", c0, vec![], &node.signer)
+
            .unwrap();
+
        issue.reload().unwrap();
+
        assert_eq!(issue.comments().count(), 2);
+

+
        issue.redact_comment(comment, &node.signer).unwrap();
+
        assert_eq!(issue.comments().count(), 1);
+

+
        // Can't redact root comment.
+
        issue.redact_comment(*issue.id, &node.signer).unwrap_err();
+
    }
+

+
    #[test]
    fn test_issue_state_serde() {
        assert_eq!(
            serde_json::to_value(State::Open).unwrap(),
@@ -1358,4 +1426,175 @@ mod test {
        assert_eq!(e1.content, Uri::from(embed1_edited.oid()));
        assert_eq!(b1.content(), &embed1_edited.content);
    }
+

+
    #[test]
+
    fn test_invalid_actions() {
+
        let test::setup::NodeWithRepo { node, repo, .. } = test::setup::NodeWithRepo::default();
+
        let mut issues = Issues::open(&*repo).unwrap();
+
        let mut issue = issues
+
            .create(
+
                "My first issue",
+
                "Blah blah blah.",
+
                &[],
+
                &[],
+
                [],
+
                &node.signer,
+
            )
+
            .unwrap();
+
        let missing = arbitrary::oid();
+

+
        issue
+
            .comment("Invalid", missing, [], &node.signer)
+
            .unwrap_err();
+
        assert_eq!(issue.comments().count(), 1);
+
        issue.reload().unwrap();
+
        assert_eq!(issue.comments().count(), 1);
+

+
        let cob = cob::get::<Issue, _>(&*repo, Issue::type_name(), issue.id())
+
            .unwrap()
+
            .unwrap();
+

+
        assert_eq!(cob.history().len(), 1);
+
        assert_eq!(
+
            cob.history().tips().into_iter().collect::<Vec<_>>(),
+
            vec![*issue.id]
+
        );
+
    }
+

+
    #[test]
+
    fn test_invalid_tx() {
+
        let test::setup::NodeWithRepo { node, repo, .. } = test::setup::NodeWithRepo::default();
+
        let mut issues = Issues::open(&*repo).unwrap();
+
        let mut issue = issues
+
            .create(
+
                "My first issue",
+
                "Blah blah blah.",
+
                &[],
+
                &[],
+
                [],
+
                &node.signer,
+
            )
+
            .unwrap();
+
        let missing = arbitrary::oid();
+

+
        // An invalid comment which points to a missing parent.
+
        // Even creating it via a transaction will trigger an error.
+
        let mut tx = Transaction::<Issue, _>::default();
+
        tx.comment("Invalid comment", missing, vec![]).unwrap();
+
        tx.commit("Add comment", issue.id, &mut issue.store.raw, &node.signer)
+
            .unwrap_err();
+

+
        issue.reload().unwrap();
+
        assert_eq!(issue.comments().count(), 1);
+
    }
+

+
    #[test]
+
    fn test_invalid_cob() {
+
        use crate::crypto::test::signer::MockSigner;
+
        use cob::change::Storage as _;
+
        use cob::object::Storage as _;
+
        use nonempty::NonEmpty;
+

+
        let test::setup::NodeWithRepo { node, repo, .. } = test::setup::NodeWithRepo::default();
+
        let eve = MockSigner::default();
+
        let identity = repo.identity().unwrap().head;
+
        let missing = arbitrary::oid();
+
        let type_name = Issue::type_name().clone();
+
        let mut issues = Issues::open(&*repo).unwrap();
+
        let mut issue = issues
+
            .create(
+
                "My first issue",
+
                "Blah blah blah.",
+
                &[],
+
                &[],
+
                [],
+
                &node.signer,
+
            )
+
            .unwrap();
+

+
        // Initially, there is one node in the DAG.
+
        let cob = cob::get::<NonEmpty<cob::Entry>, _>(&*repo, &type_name, issue.id())
+
            .unwrap()
+
            .unwrap();
+

+
        assert_eq!(cob.history.len(), 1);
+
        assert_eq!(cob.object.len(), 1);
+

+
        // We have a valid issue. Now we're going to add an invalid action to it, by bypassing
+
        // the COB API. We do this using a different key, so that valid actions by
+
        // our issue author don't overwrite the invalid action, since there is
+
        // only one ref per COB per user.
+
        let action = Action::CommentRedact { id: missing };
+
        let action = cob::store::encoding::encode(action).unwrap();
+
        let contents = NonEmpty::new(action);
+
        let invalid = repo
+
            .store(
+
                identity,
+
                vec![],
+
                &eve,
+
                cob::change::Template {
+
                    tips: vec![*issue.id],
+
                    embeds: vec![],
+
                    contents: contents.clone(),
+
                    type_name: type_name.clone(),
+
                    message: String::from("Add invalid operation"),
+
                },
+
            )
+
            .unwrap();
+

+
        repo.update(eve.public_key(), &type_name, &issue.id, &invalid.id)
+
            .unwrap();
+

+
        // If we fetch the COB with its history, *without* trying to interpret it as an issue,
+
        // we'll see that all entries, including the invalid one are there.
+
        let cob = cob::get::<NonEmpty<cob::Entry>, _>(&*repo, &type_name, issue.id())
+
            .unwrap()
+
            .unwrap();
+

+
        assert_eq!(cob.history.len(), 2);
+
        assert_eq!(cob.object.len(), 2);
+
        assert_eq!(cob.object.last().contents(), &contents);
+

+
        // However, if we try to fetch it as an *issue*, the invalid comment is pruned.
+
        let cob = cob::get::<Issue, _>(&*repo, &type_name, issue.id())
+
            .unwrap()
+
            .unwrap();
+
        assert_eq!(cob.history.len(), 1);
+
        assert_eq!(cob.object.comments().count(), 1);
+
        assert!(cob.object.comment(&issue.id).is_some());
+

+
        // Additionally, when adding a *valid* comment, it does not build upon the bad operation.
+
        issue.reload().unwrap();
+
        issue
+
            .comment("Valid comment", *issue.id, vec![], &node.signer)
+
            .unwrap();
+
        issue.reload().unwrap();
+
        assert_eq!(issue.comments().count(), 2);
+
        assert_eq!(issue.thread.timeline().count(), 2);
+
        assert_eq!(issue.comments().last().unwrap().1.body(), "Valid comment");
+

+
        // The actual DAG contains 3 nodes, but only 2 were loaded as an issue.
+
        let cob = cob::get::<NonEmpty<cob::Entry>, _>(&*repo, &type_name, issue.id())
+
            .unwrap()
+
            .unwrap();
+

+
        assert_eq!(cob.history.len(), 3);
+
        assert_eq!(cob.object.len(), 3);
+

+
        // If Eve now writes a valid comment via the `Issue` type, it will overwrite her invalid
+
        // one, since it won't be loaded as a tip.
+
        issue
+
            .comment("Eve's comment", *issue.id, vec![], &eve)
+
            .unwrap();
+

+
        let cob = cob::get::<NonEmpty<cob::Entry>, _>(&*repo, &type_name, issue.id())
+
            .unwrap()
+
            .unwrap();
+

+
        // There are three nodes still, but they are all valid comments.
+
        // The invalid comment of Eve was replaced with a valid one.
+
        assert_eq!(issue.comments().count(), 3);
+
        assert_eq!(cob.history.len(), 3);
+
        assert_eq!(cob.object.len(), 3);
+
    }
}
modified radicle/src/cob/patch.rs
@@ -15,11 +15,11 @@ use thiserror::Error;
use crate::cob;
use crate::cob::common::{Author, Authorization, Label, Reaction, Timestamp};
use crate::cob::store::Transaction;
-
use crate::cob::store::{FromHistory as _, HistoryAction};
+
use crate::cob::store::{Cob, CobAction};
use crate::cob::thread;
use crate::cob::thread::Thread;
use crate::cob::thread::{Comment, CommentId, Reactions};
-
use crate::cob::{store, ActorId, EntryId, ObjectId, TypeName};
+
use crate::cob::{op, store, ActorId, EntryId, ObjectId, TypeName};
use crate::crypto::{PublicKey, Signer};
use crate::git;
use crate::identity;
@@ -84,9 +84,6 @@ pub type RevisionIx = usize;
/// Error applying an operation onto a state.
#[derive(Debug, Error)]
pub enum Error {
-
    /// Error trying to delete the protected root revision.
-
    #[error("refusing to delete root revision: {0}")]
-
    RootRevision(RevisionId),
    /// Causal dependency missing.
    ///
    /// This error indicates that the operations are not being applied
@@ -111,11 +108,14 @@ pub enum Error {
    /// Store error.
    #[error("store: {0}")]
    Store(#[from] store::Error),
-
    #[error("history error: {0}")]
-
    History(Box<dyn std::error::Error + Sync + Send + 'static>),
+
    #[error("op decoding failed: {0}")]
+
    Op(#[from] op::OpEncodingError),
    /// Action not authorized by the author
    #[error("{0} not authorized to apply {1:?}")]
    NotAuthorized(ActorId, Action),
+
    /// An illegal action.
+
    #[error("action is not allowed: {0}")]
+
    NotAllowed(EntryId),
    /// Initialization failed.
    #[error("initialization failed: {0}")]
    Init(&'static str),
@@ -264,7 +264,7 @@ pub enum Action {
    },
}

-
impl HistoryAction for Action {
+
impl CobAction for Action {
    fn parents(&self) -> Vec<git::Oid> {
        match self {
            Self::Revision { base, oid, .. } => {
@@ -376,24 +376,21 @@ pub struct Patch {
}

impl Patch {
-
    /// Create a valid patch
-
    pub fn new(
-
        title: String,
-
        target: MergeTarget,
-
        (revision_id, revision): (RevisionId, Revision),
-
    ) -> Self {
+
    /// Construct a new patch object from a revision.
+
    pub fn new(title: String, target: MergeTarget, (id, revision): (RevisionId, Revision)) -> Self {
        Self {
            title,
            state: State::default(),
            target,
            labels: BTreeSet::default(),
            merges: BTreeMap::default(),
-
            revisions: BTreeMap::from([(revision_id, Some(revision))]),
+
            revisions: BTreeMap::from_iter([(id, Some(revision))]),
            assignees: BTreeSet::default(),
-
            timeline: vec![revision_id.into_inner()],
+
            timeline: vec![id.into_inner()],
            reviews: BTreeMap::default(),
        }
    }
+

    /// Title of the patch.
    pub fn title(&self) -> &str {
        self.title.as_str()
@@ -764,6 +761,11 @@ impl Patch {
                }
            }
            Action::RevisionRedact { revision } => {
+
                // Not allowed to delete the root revision.
+
                let (root, _) = self.root();
+
                if revision == root {
+
                    return Err(Error::NotAllowed(entry));
+
                }
                // Redactions must have observed a revision to be valid.
                if let Some(r) = self.revisions.get_mut(&revision) {
                    // If the revision has already been merged, ignore the redaction. We
@@ -1020,7 +1022,7 @@ impl Patch {
    }
}

-
impl store::FromHistory for Patch {
+
impl store::Cob for Patch {
    type Action = Action;
    type Error = Error;

@@ -1028,7 +1030,7 @@ impl store::FromHistory for Patch {
        &TYPENAME
    }

-
    fn init<R: ReadRepository>(op: Op, repo: &R) -> Result<Self, Self::Error> {
+
    fn from_root<R: ReadRepository>(op: Op, repo: &R) -> Result<Self, Self::Error> {
        let mut actions = op.actions.into_iter();
        let Some(Action::Revision { description, base, oid, resolves }) = actions.next() else {
            return Err(Error::Init("the first action must be of type `revision`"));
@@ -1053,7 +1055,7 @@ impl store::FromHistory for Patch {
        Ok(patch)
    }

-
    fn apply<R: ReadRepository>(&mut self, op: Op, repo: &R) -> Result<(), Error> {
+
    fn op<R: ReadRepository>(&mut self, op: Op, repo: &R) -> Result<(), Error> {
        debug_assert!(!self.timeline.contains(&op.id));
        self.timeline.push(op.id);

@@ -1080,6 +1082,23 @@ impl store::FromHistory for Patch {
    }
}

+
impl<R: ReadRepository> cob::Evaluate<R> for Patch {
+
    type Error = Error;
+

+
    fn init(entry: &cob::Entry, repo: &R) -> Result<Self, Self::Error> {
+
        let op = Op::try_from(entry)?;
+
        let object = Patch::from_root(op, repo)?;
+

+
        Ok(object)
+
    }
+

+
    fn apply(&mut self, entry: &cob::Entry, repo: &R) -> Result<(), Self::Error> {
+
        let op = Op::try_from(entry)?;
+

+
        self.op(op, repo)
+
    }
+
}
+

mod lookup {
    use super::*;

@@ -1462,7 +1481,7 @@ impl Review {
    }
}

-
impl store::Transaction<Patch> {
+
impl<R: ReadRepository> store::Transaction<Patch, R> {
    pub fn edit(&mut self, title: impl ToString, target: MergeTarget) -> Result<(), store::Error> {
        self.push(Action::Edit {
            title: title.to_string(),
@@ -1685,7 +1704,7 @@ pub struct PatchMut<'a, 'g, R> {

impl<'a, 'g, R> PatchMut<'a, 'g, R>
where
-
    R: ReadRepository + SignRepository + cob::Store,
+
    R: ReadRepository + SignRepository + cob::Store + 'static,
{
    pub fn new(id: ObjectId, patch: Patch, store: &'g mut Patches<'a, R>) -> Self {
        Self { id, patch, store }
@@ -1713,13 +1732,13 @@ where
    ) -> Result<EntryId, Error>
    where
        G: Signer,
-
        F: FnOnce(&mut Transaction<Patch>) -> Result<(), store::Error>,
+
        F: FnOnce(&mut Transaction<Patch, R>) -> Result<(), store::Error>,
    {
-
        let mut tx = Transaction::new(*signer.public_key());
+
        let mut tx = Transaction::default();
        operations(&mut tx)?;
-
        let (op, commit) = tx.commit(message, self.id, &mut self.store.raw, signer)?;

-
        self.patch.apply(op, self.store.as_ref())?;
+
        let (patch, commit) = tx.commit(message, self.id, &mut self.store.raw, signer)?;
+
        self.patch = patch;

        Ok(commit)
    }
@@ -1765,9 +1784,6 @@ where
        revision: RevisionId,
        signer: &G,
    ) -> Result<EntryId, Error> {
-
        if revision.0 == *self.id {
-
            return Err(Error::RootRevision(revision));
-
        }
        self.transaction("Redact revision", signer, |tx| tx.redact(revision))
    }

@@ -2011,7 +2027,7 @@ impl<'a, R> Deref for Patches<'a, R> {

impl<'a, R> Patches<'a, R>
where
-
    R: ReadRepository + cob::Store,
+
    R: ReadRepository + cob::Store + 'static,
{
    /// Open an patches store.
    pub fn open(repository: &'a R) -> Result<Self, store::Error> {
@@ -2090,7 +2106,7 @@ where

impl<'a, R> Patches<'a, R>
where
-
    R: ReadRepository + SignRepository + cob::Store,
+
    R: ReadRepository + SignRepository + cob::Store + 'static,
{
    /// Open a new patch.
    pub fn create<'g, G: Signer>(
@@ -2180,6 +2196,7 @@ where
}

#[cfg(test)]
+
#[allow(clippy::unwrap_used)]
mod test {
    use std::str::FromStr;

@@ -2447,11 +2464,11 @@ mod test {
        let mut patch = Patch::from_ops([a1, a2], &repo).unwrap();
        assert_eq!(patch.revisions().count(), 2);

-
        patch.apply(a3, &repo).unwrap();
+
        patch.op(a3, &repo).unwrap();
        assert_eq!(patch.revisions().count(), 1);

-
        patch.apply(a4, &repo).unwrap();
-
        patch.apply(a5, &repo).unwrap();
+
        patch.op(a4, &repo).unwrap();
+
        patch.op(a5, &repo).unwrap();
    }

    #[test]
@@ -2478,7 +2495,7 @@ mod test {
            time,
            &alice,
        );
-
        h0.commit(
+
        let r1 = h0.commit(
            &Action::Revision {
                description: String::from("New"),
                base,
@@ -2493,7 +2510,7 @@ mod test {
        let mut h1 = h0.clone();
        h1.commit(
            &Action::RevisionRedact {
-
                revision: RevisionId(*h0.root().id()),
+
                revision: RevisionId(r1),
            },
            &alice,
        );
@@ -2755,6 +2772,7 @@ mod test {
        assert_eq!(patch.revisions().count(), 1);

        // The patch's root must always exist.
+
        assert_eq!(patch.latest(), patch.root());
        assert!(patch.redact(patch.latest().0, &alice.signer).is_err());
    }

modified radicle/src/cob/store.rs
@@ -3,31 +3,19 @@
#![allow(clippy::type_complexity)]
use std::fmt::Debug;
use std::marker::PhantomData;
-
use std::ops::ControlFlow;

use nonempty::NonEmpty;
use serde::{Deserialize, Serialize};

-
use crate::cob::common::Timestamp;
use crate::cob::op::Op;
-
use crate::cob::{
-
    ActorId, Create, Embed, EntryId, History, ObjectId, TypeName, Update, Updated, Version,
-
};
+
use crate::cob::{Create, Embed, EntryId, ObjectId, TypeName, Update, Updated, Version};
use crate::git;
use crate::prelude::*;
use crate::storage::git as storage;
use crate::storage::SignRepository;
use crate::{cob, identity};

-
#[derive(Debug, thiserror::Error)]
-
pub enum HistoryError<T: FromHistory> {
-
    #[error("apply: {0}")]
-
    Apply(T::Error),
-
    #[error("operation decoding failed: {0}")]
-
    Op(#[from] cob::op::OpEncodingError),
-
}
-

-
pub trait HistoryAction: Debug {
+
pub trait CobAction: Debug {
    /// Parent objects this action depends on. For example, patch revisions
    /// have the commit objects as their parent.
    fn parents(&self) -> Vec<git::Oid> {
@@ -35,33 +23,33 @@ pub trait HistoryAction: Debug {
    }
}

-
/// A type that can be materialized from an event history.
-
/// All collaborative objects implement this trait.
-
pub trait FromHistory: Sized + PartialEq + Debug {
+
/// A collaborative object. Can be materialized from an operation history.
+
pub trait Cob: Sized + PartialEq + Debug {
    /// The underlying action composing each operation.
-
    type Action: HistoryAction + for<'de> Deserialize<'de> + Serialize;
+
    type Action: CobAction + for<'de> Deserialize<'de> + Serialize;
    /// Error returned by `apply` function.
    type Error: std::error::Error + Send + Sync + 'static;

    /// The object type name.
    fn type_name() -> &'static TypeName;

-
    /// Initialize a collarorative object
-
    fn init<R: ReadRepository>(op: Op<Self::Action>, repo: &R) -> Result<Self, Self::Error>;
+
    /// Initialize a collarorative object from a root operation.
+
    fn from_root<R: ReadRepository>(op: Op<Self::Action>, repo: &R) -> Result<Self, Self::Error>;

-
    /// Apply a list of operations to the state.
-
    fn apply<R: ReadRepository>(
+
    /// Apply an operation to the state.
+
    fn op<R: ReadRepository>(
        &mut self,
        op: Op<Self::Action>,
        repo: &R,
-
    ) -> Result<(), Self::Error>;
+
    ) -> Result<(), <Self as Cob>::Error>;

+
    #[cfg(test)]
    /// Create an object from a history.
    fn from_history<R: ReadRepository>(
-
        history: &History,
+
        history: &crate::cob::History,
        repo: &R,
-
    ) -> Result<Self, HistoryError<Self>> {
-
        self::from_history::<R, Self>(history, repo)
+
    ) -> Result<Self, test::HistoryError<Self>> {
+
        test::from_history::<R, Self>(history, repo)
    }

    #[cfg(test)]
@@ -75,43 +63,14 @@ pub trait FromHistory: Sized + PartialEq + Debug {
        let Some(init) = ops.next() else {
            panic!("FromHistory::from_ops: operations list is empty");
        };
-
        let mut state = Self::init(init, repo)?;
+
        let mut state = Self::from_root(init, repo)?;
        for op in ops {
-
            state.apply(op, repo)?;
+
            state.op(op, repo)?;
        }
        Ok(state)
    }
}

-
/// Turn a history into a concrete type, by traversing the history and applying each operation
-
/// to the state, skipping branches that return errors.
-
pub fn from_history<R: ReadRepository, T: FromHistory>(
-
    history: &History,
-
    repo: &R,
-
) -> Result<T, HistoryError<T>> {
-
    let root = history.root();
-
    let children = history.children_of(root.id());
-
    let op = Op::try_from(root)?;
-
    let initial = T::init(op, repo).map_err(HistoryError::Apply)?;
-
    let obj = history.traverse(initial, &children, |mut acc, _, entry| {
-
        match Op::try_from(entry) {
-
            Ok(op) => {
-
                if let Err(err) = acc.apply(op, repo) {
-
                    log::warn!("Error applying op to `{}` state: {err}", T::type_name());
-
                    return ControlFlow::Break(acc);
-
                }
-
            }
-
            Err(err) => {
-
                log::warn!("Error decoding ops for `{}` state: {err}", T::type_name());
-
                return ControlFlow::Break(acc);
-
            }
-
        }
-
        ControlFlow::Continue(acc)
-
    });
-

-
    Ok(obj)
-
}
-

/// Store error.
#[derive(Debug, thiserror::Error)]
pub enum Error {
@@ -137,14 +96,6 @@ pub enum Error {
        #[source]
        err: git::Error,
    },
-
    #[error("history: {0}")]
-
    History(Box<dyn std::error::Error + Sync + Send + 'static>),
-
}
-

-
impl Error {
-
    fn history(e: impl std::error::Error + Send + Sync + 'static) -> Self {
-
        Self::History(Box::new(e))
-
    }
}

/// Storage for collaborative objects of a specific type `T` in a single repository.
@@ -176,7 +127,7 @@ impl<'a, T, R: ReadRepository> Store<'a, T, R> {
impl<'a, T, R> Store<'a, T, R>
where
    R: ReadRepository + SignRepository + cob::Store,
-
    T: FromHistory + 'static,
+
    T: Cob + cob::Evaluate<R>,
    T::Action: Serialize,
{
    /// Update an object.
@@ -187,7 +138,7 @@ where
        actions: impl Into<NonEmpty<T::Action>>,
        embeds: Vec<Embed>,
        signer: &G,
-
    ) -> Result<Updated, Error> {
+
    ) -> Result<Updated<T>, Error> {
        let actions = actions.into();
        let parents = actions.iter().flat_map(T::Action::parents).collect();
        let changes = actions.try_map(encoding::encode)?;
@@ -205,7 +156,6 @@ where
                changes,
            },
        )?;
-

        self.repo.sign_refs(signer).map_err(Error::SignRefs)?;

        Ok(updated)
@@ -222,7 +172,7 @@ where
        let actions = actions.into();
        let parents = actions.iter().flat_map(T::Action::parents).collect();
        let contents = actions.try_map(encoding::encode)?;
-
        let cob = cob::create(
+
        let cob = cob::create::<T, _, G>(
            self.repo,
            signer,
            self.identity,
@@ -236,11 +186,9 @@ where
                contents,
            },
        )?;
-
        let object = T::from_history(cob.history(), self.repo).map_err(Error::history)?;
-

        self.repo.sign_refs(signer).map_err(Error::SignRefs)?;

-
        Ok((*cob.id(), object))
+
        Ok((*cob.id(), cob.object))
    }

    /// Remove an object.
@@ -268,30 +216,21 @@ where
impl<'a, T, R> Store<'a, T, R>
where
    R: ReadRepository + cob::Store,
-
    T: FromHistory + 'static,
+
    T: cob::Evaluate<R> + Cob,
    T::Action: Serialize,
{
    /// Get an object.
    pub fn get(&self, id: &ObjectId) -> Result<Option<T>, Error> {
-
        let cob = cob::get(self.repo, T::type_name(), id)?;
-

-
        if let Some(cob) = cob {
-
            let obj = T::from_history(cob.history(), self.repo).map_err(Error::history)?;
-

-
            Ok(Some(obj))
-
        } else {
-
            Ok(None)
-
        }
+
        cob::get::<T, _>(self.repo, T::type_name(), id)
+
            .map(|r| r.map(|cob| cob.object))
+
            .map_err(Error::from)
    }

    /// Return all objects.
    pub fn all(&self) -> Result<impl Iterator<Item = Result<(ObjectId, T), Error>> + 'a, Error> {
-
        let raw = cob::list(self.repo, T::type_name())?;
+
        let raw = cob::list::<T, _>(self.repo, T::type_name())?;

-
        Ok(raw.into_iter().map(|o| {
-
            let obj = T::from_history(o.history(), self.repo).map_err(Error::history)?;
-
            Ok((*o.id(), obj))
-
        }))
+
        Ok(raw.into_iter().map(|o| Ok((*o.id(), o.object))))
    }

    /// Return true if the list of issues is empty.
@@ -301,7 +240,7 @@ where

    /// Return objects count.
    pub fn count(&self) -> Result<usize, Error> {
-
        let raw = cob::list(self.repo, T::type_name())?;
+
        let raw = cob::list::<T, _>(self.repo, T::type_name())?;

        Ok(raw.len())
    }
@@ -309,24 +248,25 @@ where

/// Allows operations to be batched atomically.
#[derive(Debug)]
-
pub struct Transaction<T: FromHistory> {
-
    actor: ActorId,
+
pub struct Transaction<T: Cob + cob::Evaluate<R>, R> {
    actions: Vec<T::Action>,
    embeds: Vec<Embed>,
+
    repo: PhantomData<R>,
}

-
impl<T: FromHistory + 'static> Transaction<T> {
-
    /// Create a new transaction.
-
    pub fn new(actor: ActorId) -> Self {
+
impl<T: Cob + cob::Evaluate<R>, R> Default for Transaction<T, R> {
+
    fn default() -> Self {
        Self {
-
            actor,
            actions: Vec::new(),
            embeds: Vec::new(),
+
            repo: PhantomData,
        }
    }
+
}

+
impl<T: Cob + cob::Evaluate<R>, R> Transaction<T, R> {
    /// Create a new transaction to be used as the initial set of operations for a COB.
-
    pub fn initial<R, G, F>(
+
    pub fn initial<G, F>(
        message: &str,
        store: &mut Store<T, R>,
        signer: &G,
@@ -338,16 +278,13 @@ impl<T: FromHistory + 'static> Transaction<T> {
        R: ReadRepository + SignRepository + cob::Store,
        T::Action: Serialize + Clone,
    {
-
        let actor = *signer.public_key();
-
        let mut tx = Transaction::new(actor);
-

+
        let mut tx = Transaction::default();
        operations(&mut tx)?;

        let actions = NonEmpty::from_vec(tx.actions)
-
            .expect("Transaction::initial: transaction must contain at least one operation");
-
        let (id, cob) = store.create(message, actions, tx.embeds, signer)?;
+
            .expect("Transaction::initial: transaction must contain at least one action");

-
        Ok((id, cob))
+
        store.create(message, actions, tx.embeds, signer)
    }

    /// Add an operation to this transaction.
@@ -367,40 +304,23 @@ impl<T: FromHistory + 'static> Transaction<T> {
    /// Commit transaction.
    ///
    /// Returns an operation that can be applied onto an in-memory state.
-
    pub fn commit<R, G: Signer>(
+
    pub fn commit<G: Signer>(
        self,
        msg: &str,
        id: ObjectId,
        store: &mut Store<T, R>,
        signer: &G,
-
    ) -> Result<(cob::Op<T::Action>, EntryId), Error>
+
    ) -> Result<(T, EntryId), Error>
    where
        R: ReadRepository + SignRepository + cob::Store,
        T::Action: Serialize + Clone,
    {
        let actions = NonEmpty::from_vec(self.actions)
            .expect("Transaction::commit: transaction must not be empty");
-
        let Updated {
-
            head,
-
            object,
-
            parents,
-
        } = store.update(id, msg, actions.clone(), self.embeds, signer)?;
-
        let id = head;
-
        let author = self.actor;
-
        let timestamp = Timestamp::from_secs(object.history().timestamp());
-
        let identity = store.identity;
-
        let manifest = object.manifest().clone();
-
        let op = cob::Op {
-
            id,
-
            actions,
-
            author,
-
            timestamp,
-
            parents,
-
            identity,
-
            manifest,
-
        };
+
        let Updated { head, object, .. } =
+
            store.update(id, msg, actions.clone(), self.embeds, signer)?;

-
        Ok((op, id))
+
        Ok((object.object, head))
    }
}

@@ -409,18 +329,11 @@ pub fn ops<R: cob::Store>(
    id: &ObjectId,
    type_name: &TypeName,
    repo: &R,
-
) -> Result<Vec<Op<Vec<u8>>>, Error> {
-
    let cob = cob::get(repo, type_name, id)?;
+
) -> Result<NonEmpty<Op<Vec<u8>>>, Error> {
+
    let cob = cob::get::<NonEmpty<cob::Entry>, _>(repo, type_name, id)?;

    if let Some(cob) = cob {
-
        let root = cob.history().root();
-
        let ops = cob
-
            .history()
-
            .traverse(Vec::new(), &[root.id], |mut ops, _, entry| {
-
                ops.push(Op::from(entry.clone()));
-
                ControlFlow::Continue(ops)
-
            });
-
        Ok(ops)
+
        Ok(cob.object.map(Op::from))
    } else {
        Err(Error::NotFound(type_name.clone(), *id))
    }
@@ -442,3 +355,47 @@ pub mod encoding {
        Ok(buf)
    }
}
+

+
#[cfg(test)]
+
pub mod test {
+
    use super::*;
+

+
    #[derive(Debug, thiserror::Error)]
+
    pub enum HistoryError<T: Cob> {
+
        #[error("apply: {0}")]
+
        Apply(T::Error),
+
        #[error("operation decoding failed: {0}")]
+
        Op(#[from] cob::op::OpEncodingError),
+
    }
+

+
    /// Turn a history into a concrete type, by traversing the history and applying each operation
+
    /// to the state, skipping branches that return errors.
+
    pub fn from_history<R: ReadRepository, T: Cob>(
+
        history: &crate::cob::History,
+
        repo: &R,
+
    ) -> Result<T, HistoryError<T>> {
+
        use std::ops::ControlFlow;
+

+
        let root = history.root();
+
        let children = history.children_of(root.id());
+
        let op = Op::try_from(root)?;
+
        let initial = T::from_root(op, repo).map_err(HistoryError::Apply)?;
+
        let obj = history.traverse(initial, &children, |mut acc, _, entry| {
+
            match Op::try_from(entry) {
+
                Ok(op) => {
+
                    if let Err(err) = acc.op(op, repo) {
+
                        log::warn!("Error applying op to `{}` state: {err}", T::type_name());
+
                        return ControlFlow::Break(acc);
+
                    }
+
                }
+
                Err(err) => {
+
                    log::warn!("Error decoding ops for `{}` state: {err}", T::type_name());
+
                    return ControlFlow::Break(acc);
+
                }
+
            }
+
            ControlFlow::Continue(acc)
+
        });
+

+
        Ok(obj)
+
    }
+
}
modified radicle/src/cob/test.rs
@@ -1,3 +1,4 @@
+
#![allow(clippy::unwrap_used)]
use std::marker::PhantomData;
use std::ops::Deref;

@@ -20,7 +21,7 @@ use crate::prelude::Did;
use crate::storage::ReadRepository;
use crate::test::arbitrary;

-
use super::store::FromHistory;
+
use super::store::Cob;
use super::thread;

/// Convenience type for building histories.
@@ -53,7 +54,7 @@ impl HistoryBuilder<thread::Thread> {
    }
}

-
impl<T: FromHistory> HistoryBuilder<T>
+
impl<T: Cob> HistoryBuilder<T>
where
    T::Action: for<'de> Deserialize<'de> + Serialize + Eq + 'static,
{
@@ -129,7 +130,7 @@ impl<A> Deref for HistoryBuilder<A> {
}

/// Create a new test history.
-
pub fn history<T: FromHistory, G: Signer>(
+
pub fn history<T: Cob, G: Signer>(
    actions: &[T::Action],
    time: Timestamp,
    signer: &G,
@@ -159,7 +160,7 @@ impl<G> Actor<G> {

impl<G: Signer> Actor<G> {
    /// Create a new operation.
-
    pub fn op_with<T: FromHistory>(
+
    pub fn op_with<T: Cob>(
        &mut self,
        actions: impl IntoIterator<Item = T::Action>,
        identity: Oid,
@@ -193,10 +194,7 @@ impl<G: Signer> Actor<G> {
    }

    /// Create a new operation.
-
    pub fn op<T: FromHistory>(
-
        &mut self,
-
        actions: impl IntoIterator<Item = T::Action>,
-
    ) -> Op<T::Action>
+
    pub fn op<T: Cob>(&mut self, actions: impl IntoIterator<Item = T::Action>) -> Op<T::Action>
    where
        T::Action: Clone + Serialize,
    {
@@ -222,7 +220,7 @@ impl<G: Signer> Actor<G> {
        oid: git::Oid,
        repo: &R,
    ) -> Result<Patch, patch::Error> {
-
        Patch::init(
+
        Patch::from_root(
            self.op::<Patch>([
                patch::Action::Revision {
                    description: description.to_string(),
@@ -244,7 +242,7 @@ impl<G: Signer> Actor<G> {
///
/// Doesn't encode in the same way as we do in production, but attempts to include the same data
/// that feeds into the hash entropy, so that changing any input will change the resulting oid.
-
pub fn encoded<T: FromHistory, G: Signer>(
+
pub fn encoded<T: Cob, G: Signer>(
    action: &T::Action,
    timestamp: Timestamp,
    parents: impl IntoIterator<Item = Oid>,
modified radicle/src/cob/thread.rs
@@ -8,7 +8,8 @@ use thiserror::Error;

use crate::cob;
use crate::cob::common::{Reaction, Timestamp, Uri};
-
use crate::cob::{ActorId, Embed, EntryId, Op};
+
use crate::cob::store::Cob;
+
use crate::cob::{op, ActorId, Embed, EntryId, Op};
use crate::git;
use crate::prelude::ReadRepository;

@@ -38,6 +39,8 @@ pub enum Error {
    /// Object initialization failed.
    #[error("initialization failed: {0}")]
    Init(&'static str),
+
    #[error("op decoding failed: {0}")]
+
    Op(#[from] op::OpEncodingError),
}

/// Identifies a comment.
@@ -229,7 +232,7 @@ pub enum Action {
    },
}

-
impl cob::store::HistoryAction for Action {}
+
impl cob::store::CobAction for Action {}

impl From<Action> for nonempty::NonEmpty<Action> {
    fn from(action: Action) -> Self {
@@ -241,9 +244,9 @@ impl From<Action> for nonempty::NonEmpty<Action> {
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Thread<T = Comment<()>> {
    /// The comments under the thread.
-
    comments: BTreeMap<CommentId, Option<T>>,
+
    pub(crate) comments: BTreeMap<CommentId, Option<T>>,
    /// Comment timeline.
-
    timeline: Vec<CommentId>,
+
    pub(crate) timeline: Vec<CommentId>,
}

impl<T> Default for Thread<T> {
@@ -299,6 +302,10 @@ impl<T> Thread<T> {
                .map(|comment| (id, comment))
        })
    }
+

+
    pub fn timeline(&self) -> impl DoubleEndedIterator<Item = &EntryId> + '_ {
+
        self.timeline.iter()
+
    }
}

impl Thread {
@@ -350,7 +357,7 @@ impl<L> Thread<Comment<L>> {
    }
}

-
impl cob::store::FromHistory for Thread {
+
impl cob::store::Cob for Thread {
    type Action = Action;
    type Error = Error;

@@ -358,7 +365,7 @@ impl cob::store::FromHistory for Thread {
        &TYPENAME
    }

-
    fn init<R: ReadRepository>(op: Op<Action>, repo: &R) -> Result<Self, Self::Error> {
+
    fn from_root<R: ReadRepository>(op: Op<Action>, repo: &R) -> Result<Self, Self::Error> {
        let author = op.author;
        let entry = op.id;
        let timestamp = op.timestamp;
@@ -385,7 +392,7 @@ impl cob::store::FromHistory for Thread {
        Ok(thread)
    }

-
    fn apply<R: ReadRepository>(&mut self, op: Op<Action>, repo: &R) -> Result<(), Error> {
+
    fn op<R: ReadRepository>(&mut self, op: Op<Action>, repo: &R) -> Result<(), Error> {
        for action in op.actions {
            self.action(action, op.id, op.author, op.timestamp, op.identity, repo)?;
        }
@@ -393,6 +400,23 @@ impl cob::store::FromHistory for Thread {
    }
}

+
impl<R: ReadRepository> cob::Evaluate<R> for Thread {
+
    type Error = Error;
+

+
    fn init(entry: &cob::Entry, repo: &R) -> Result<Self, Self::Error> {
+
        let op = Op::try_from(entry)?;
+
        let object = <Thread as Cob>::from_root(op, repo)?;
+

+
        Ok(object)
+
    }
+

+
    fn apply(&mut self, entry: &cob::Entry, repo: &R) -> Result<(), Self::Error> {
+
        let op = Op::try_from(entry)?;
+

+
        self.op(op, repo)
+
    }
+
}
+

pub fn comment<L>(
    thread: &mut Thread<Comment<L>>,
    id: EntryId,
@@ -406,6 +430,11 @@ pub fn comment<L>(
    if body.is_empty() {
        return Err(Error::Comment(id));
    }
+
    if let Some(id) = reply_to {
+
        if !thread.comments.contains_key(&id) {
+
            return Err(Error::Missing(id));
+
        }
+
    }
    debug_assert!(!thread.timeline.contains(&id));
    thread.timeline.push(id);

@@ -522,6 +551,7 @@ pub fn unresolve<T>(
}

#[cfg(test)]
+
#[allow(clippy::unwrap_used)]
mod tests {
    use std::ops::{Deref, DerefMut};

@@ -530,7 +560,7 @@ mod tests {

    use super::*;
    use crate as radicle;
-
    use crate::cob::store::FromHistory;
+
    use crate::cob::store::Cob;
    use crate::cob::test;
    use crate::crypto::test::signer::MockSigner;
    use crate::crypto::Signer;
@@ -609,7 +639,7 @@ mod tests {

        // Redact the second comment.
        let a3 = alice.redact(a1.id());
-
        thread.apply(a3, &repo).unwrap();
+
        thread.op(a3, &repo).unwrap();

        let (_, comment0) = thread.comments().nth(0).unwrap();
        let (_, comment1) = thread.comments().nth(1).unwrap();
@@ -806,7 +836,7 @@ mod tests {
        let mut t = Thread::default();
        let id = arbitrary::entry_id();

-
        t.apply(alice.redact(id), &repo).unwrap_err();
+
        t.op(alice.redact(id), &repo).unwrap_err();
    }

    #[test]
@@ -816,7 +846,7 @@ mod tests {
        let mut t = Thread::default();
        let id = arbitrary::entry_id();

-
        t.apply(alice.edit(id, "Edited"), &repo).unwrap_err();
+
        t.op(alice.edit(id, "Edited"), &repo).unwrap_err();
    }

    #[test]
modified radicle/src/storage/git.rs
@@ -492,7 +492,6 @@ impl ReadRepository for Repository {
            let oid = e.target().ok_or(Error::InvalidRef)?;
            let (_, category, _, _) = refname.non_empty_components();

-
            // Only sign known ref categories.
            if [
                git::name::HEADS,
                git::name::TAGS,
modified radicle/src/storage/git/cob.rs
@@ -15,7 +15,7 @@ use crate::storage::{
};
use crate::{
    git, identity,
-
    identity::{doc::DocError, IdentityError},
+
    identity::{doc::DocError, IdentityError, PublicKey},
};

use super::{RemoteId, Repository};
@@ -80,8 +80,6 @@ impl cob::object::Storage for Repository {
    type UpdateError = git2::Error;
    type RemoveError = git2::Error;

-
    type Identifier = RemoteId;
-

    fn objects(
        &self,
        typename: &cob::TypeName,
@@ -136,7 +134,7 @@ impl cob::object::Storage for Repository {

    fn update(
        &self,
-
        identifier: &Self::Identifier,
+
        identifier: &PublicKey,
        typename: &cob::TypeName,
        object_id: &cob::ObjectId,
        entry: &cob::EntryId,
@@ -156,7 +154,7 @@ impl cob::object::Storage for Repository {

    fn remove(
        &self,
-
        identifier: &Self::Identifier,
+
        identifier: &PublicKey,
        typename: &cob::TypeName,
        object_id: &cob::ObjectId,
    ) -> Result<(), Self::RemoveError> {
@@ -216,12 +214,16 @@ impl<'a, R: storage::WriteRepository> change::Storage for DraftStore<'a, R> {
    }
}

-
impl<'a> SignRepository for DraftStore<'a> {
+
impl<'a, R: storage::ReadRepository> SignRepository for DraftStore<'a, R> {
    fn sign_refs<G: crypto::Signer>(
        &self,
        signer: &G,
    ) -> Result<storage::refs::SignedRefs<Verified>, Error> {
-
        self.repo.sign_refs(signer)
+
        // Since this is a draft store, we do not actually want to sign the refs.
+
        // Instead, we just return the existing signed refs.
+
        let remote = self.repo.remote(signer.public_key())?;
+

+
        Ok(remote.refs)
    }
}

@@ -344,8 +346,6 @@ impl<'a, R: storage::WriteRepository> cob::object::Storage for DraftStore<'a, R>
    type UpdateError = git2::Error;
    type RemoveError = git2::Error;

-
    type Identifier = RemoteId;
-

    fn objects(
        &self,
        typename: &cob::TypeName,
@@ -382,7 +382,7 @@ impl<'a, R: storage::WriteRepository> cob::object::Storage for DraftStore<'a, R>

    fn update(
        &self,
-
        identifier: &Self::Identifier,
+
        identifier: &PublicKey,
        typename: &cob::TypeName,
        object_id: &cob::ObjectId,
        entry: &cob::history::EntryId,
@@ -402,7 +402,7 @@ impl<'a, R: storage::WriteRepository> cob::object::Storage for DraftStore<'a, R>

    fn remove(
        &self,
-
        identifier: &Self::Identifier,
+
        identifier: &PublicKey,
        typename: &cob::TypeName,
        object_id: &cob::ObjectId,
    ) -> Result<(), Self::RemoveError> {
modified radicle/src/test.rs
@@ -155,6 +155,12 @@ pub mod setup {
        }
    }

+
    impl std::ops::DerefMut for NodeRepo {
+
        fn deref_mut(&mut self) -> &mut Self::Target {
+
            &mut self.repo
+
        }
+
    }
+

    /// A repository checkout.
    pub struct NodeRepoCheckout {
        checkout: git::raw::Repository,