Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: changes done while porting iroh-willow #45

Merged
merged 7 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 79 additions & 5 deletions data-model/src/entry.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::cmp::Ordering;

#[cfg(feature = "dev")]
use arbitrary::{size_hint::and_all, Arbitrary};

Expand All @@ -13,7 +15,7 @@ pub type Timestamp = u64;

/// The metadata associated with each Payload.
/// [Definition](https://willowprotocol.org/specs/data-model/index.html#Entry).
#[derive(Debug, PartialEq, Eq, Clone)]
#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub struct Entry<const MCL: usize, const MCC: usize, const MPL: usize, N, S, PD>
where
N: NamespaceId,
Expand All @@ -28,10 +30,10 @@ where
path: Path<MCL, MCC, MPL>,
/// The claimed creation time of the [`Entry`].
timestamp: Timestamp,
/// The length of the Payload in bytes.
payload_length: u64,
/// The result of applying hash_payload to the Payload.
payload_digest: PD,
/// The length of the Payload in bytes.
payload_length: u64,
}

impl<const MCL: usize, const MCC: usize, const MPL: usize, N, S, PD> Entry<MCL, MCC, MPL, N, S, PD>
Expand Down Expand Up @@ -101,6 +103,52 @@ where
}
}

/// Returns an ordering between `self` and `other`.
///
/// See the implementation of [`Ord`] on [`Entry`].
impl<const MCL: usize, const MCC: usize, const MPL: usize, N, S, PD> PartialOrd
for Entry<MCL, MCC, MPL, N, S, PD>
where
N: NamespaceId + Ord,
S: SubspaceId,
PD: PayloadDigest,
{
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}

/// Returns an ordering between `self` and `other`.
///
/// The ordering is the ordering defined in the [sideloading protocol]: Entries are first compared
/// by namespace id, then by subspace id, then by path. If those are all equal, the entries are
/// sorted by their newer relation (see [`Self::is_newer_than`]).
///
/// [sideloading protocol]: https://willowprotocol.org/specs/sideloading/index.html#sideload_protocol
impl<const MCL: usize, const MCC: usize, const MPL: usize, N, S, PD> Ord
for Entry<MCL, MCC, MPL, N, S, PD>
where
N: NamespaceId + Ord,
S: SubspaceId,
PD: PayloadDigest,
{
fn cmp(&self, other: &Self) -> Ordering {
self.namespace_id
.cmp(&other.namespace_id)
.then_with(|| self.subspace_id.cmp(&other.subspace_id))
.then_with(|| self.path.cmp(&other.path))
.then_with(|| {
if self.is_newer_than(other) {
Ordering::Greater
} else if other.is_newer_than(self) {
Ordering::Less
} else {
Ordering::Equal
}
})
}
}

use syncify::syncify;
use syncify::syncify_replace;

Expand Down Expand Up @@ -230,8 +278,8 @@ impl std::error::Error for UnauthorisedWriteError {}
/// [Definition](https://willowprotocol.org/specs/data-model/index.html#AuthorisedEntry).
#[derive(Debug, PartialEq, Eq, Clone)]
pub struct AuthorisedEntry<const MCL: usize, const MCC: usize, const MPL: usize, N, S, PD, AT>(
pub Entry<MCL, MCC, MPL, N, S, PD>,
pub AT,
Entry<MCL, MCC, MPL, N, S, PD>,
AT,
Copy link
Contributor Author

@Frando Frando Aug 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the fields should not be pub: If this type is intended to symbolize an entry+token pair where the authorisation was checked (which the new function indicates, and which is also how the term is used in the willow spec) then I think the fields should be private, otherwise AuthorisedEntry(some_entry, some_token) is a too-easy-to-get-wrong constructor for an unchecked AuthorisedEntry.

)
where
N: NamespaceId,
Expand Down Expand Up @@ -260,6 +308,32 @@ where

Err(UnauthorisedWriteError)
}

/// Construct an [`AuthorisedEntry`] without checking if the token permits the writing of this
/// entry.
///
/// Should only be used if the token was created by ourselves or previously validated.
pub fn new_unchecked(entry: Entry<MCL, MCC, MPL, N, S, PD>, token: AT) -> Self
where
AT: AuthorisationToken<MCL, MCC, MPL, N, S, PD>,
{
Self(entry, token)
}

/// Split into [`Entry`] and [`AuthorisationToken`] halves.
pub fn into_parts(self) -> (Entry<MCL, MCC, MPL, N, S, PD>, AT) {
(self.0, self.1)
}

/// Get a reference to the [`Entry`].
pub fn entry(&self) -> &Entry<MCL, MCC, MPL, N, S, PD> {
&self.0
}

/// Get a reference to the [`AuthorisationToken`].
pub fn token(&self) -> &AT {
&self.1
}
}

#[cfg(test)]
Expand Down
11 changes: 10 additions & 1 deletion data-model/src/grouping/area_of_interest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,20 @@ pub struct AreaOfInterest<const MCL: usize, const MCC: usize, const MPL: usize,
impl<const MCL: usize, const MCC: usize, const MPL: usize, S: SubspaceId>
AreaOfInterest<MCL, MCC, MPL, S>
{
/// Creates a new [`AreaOfInterest`].
pub fn new(area: Area<MCL, MCC, MPL, S>, max_count: u64, max_size: u64) -> Self {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just easier to type than initializing with let aoi = AreaOfInterest { area, max_count, max_size } and leads to less line breaks.

Self {
area,
max_count,
max_size,
}
}

/// Return the intersection of this [`AreaOfInterest`] with another.
/// [Definition](https://willowprotocol.org/specs/grouping-entries/index.html#aoi_intersection).
pub fn intersection(
&self,
other: AreaOfInterest<MCL, MCC, MPL, S>,
other: &AreaOfInterest<MCL, MCC, MPL, S>,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't consume other so should take by ref to not need cloning at call site

) -> Option<AreaOfInterest<MCL, MCC, MPL, S>> {
match self.area.intersection(&other.area) {
None => None,
Expand Down
33 changes: 7 additions & 26 deletions data-model/src/grouping/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use arbitrary::{Arbitrary, Error as ArbitraryError};

use crate::path::Path;

#[derive(Debug, Hash, PartialEq, Eq)]
#[derive(Debug, Hash, PartialEq, Eq, Clone, Copy)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those can easily be copy

/// Determines whether a [`Range`] is _closed_ or _open_.
pub enum RangeEnd<T: Ord> {
/// A [closed range](https://willowprotocol.org/specs/grouping-entries/index.html#closed_range) consists of a [start value](https://willowprotocol.org/specs/grouping-entries/index.html#start_value) and an [end_value](https://willowprotocol.org/specs/grouping-entries/index.html#end_value).
Expand Down Expand Up @@ -119,18 +119,6 @@ impl<const MCL: usize, const MCC: usize, const MPL: usize> PartialOrd<RangeEnd<P
}
}

impl<T> Clone for RangeEnd<T>
where
T: Ord + Clone,
{
fn clone(&self) -> Self {
match self {
RangeEnd::Closed(val) => RangeEnd::Closed(val.clone()),
RangeEnd::Open => RangeEnd::Open,
}
}
}

#[cfg(feature = "dev")]
impl<'a, T> Arbitrary<'a> for RangeEnd<T>
where
Expand All @@ -152,7 +140,7 @@ where
/// One-dimensional grouping over a type of value.
///
/// [Definition](https://willowprotocol.org/specs/grouping-entries/index.html#range)
#[derive(Debug, Hash, PartialEq, Eq)]
#[derive(Debug, Hash, PartialEq, Eq, Clone, Copy)]
pub struct Range<T: Ord> {
/// A range [includes](https://willowprotocol.org/specs/grouping-entries/index.html#range_include) all values greater than or equal to its [start value](https://willowprotocol.org/specs/grouping-entries/index.html#start_value) **and** less than its [end_value](https://willowprotocol.org/specs/grouping-entries/index.html#end_value)
pub start: T,
Expand All @@ -164,6 +152,11 @@ impl<T> Range<T>
where
T: Ord + Clone,
{
/// Construct a range.
pub fn new(start: T, end: RangeEnd<T>) -> Self {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, a constructor for when you have a RangeEnd available (eg from another range) saves line breaks at callsite

Self { start, end }
}

/// Construct a new [open range](https://willowprotocol.org/specs/grouping-entries/index.html#open_range) from a [start value](https://willowprotocol.org/specs/grouping-entries/index.html#start_value).
pub fn new_open(start: T) -> Self {
Self {
Expand Down Expand Up @@ -248,18 +241,6 @@ impl<T: Ord> PartialOrd for Range<T> {
}
}

impl<T> Clone for Range<T>
where
T: Ord + Clone,
{
fn clone(&self) -> Self {
Self {
start: self.start.clone(),
end: self.end.clone(),
}
}
}

#[cfg(feature = "dev")]
impl<'a, T> Arbitrary<'a> for Range<T>
where
Expand Down
9 changes: 9 additions & 0 deletions data-model/src/grouping/range_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ impl<const MCL: usize, const MCC: usize, const MPL: usize, S: SubspaceId>
}
}

/// Create a new [`Range3d`] that covers everything.
pub fn new_full() -> Self {
Self::new(
Default::default(),
Range::new_open(Path::new_empty()),
Default::default(),
)
}

/// Return a reference to the range of [`SubspaceId`]s.
/// [Definition](https://willowprotocol.org/specs/grouping-entries/index.html#SubspaceRange).
pub fn subspaces(&self) -> &Range<S> {
Expand Down
2 changes: 1 addition & 1 deletion fuzz/fuzz_targets/mc_is_authorised_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ fuzz_target!(|data: (
let (entry, token) = data;

let is_within_granted_area = token.capability.granted_area().includes_entry(&entry);
let is_write_cap = token.capability.access_mode() == &AccessMode::Write;
let is_write_cap = token.capability.access_mode() == AccessMode::Write;

let mut consumer = IntoVec::<u8>::new();
entry.encode(&mut consumer).unwrap();
Expand Down
6 changes: 3 additions & 3 deletions meadowcap/src/communal_capability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl<NamespacePublicKey: std::fmt::Debug> std::error::Error
/// A capability that implements [communal namespaces](https://willowprotocol.org/specs/meadowcap/index.html#communal_namespace).
///
/// [Definition](https://willowprotocol.org/specs/meadowcap/index.html#communal_capabilities).
#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct CommunalCapability<
const MCL: usize,
const MCC: usize,
Expand Down Expand Up @@ -157,8 +157,8 @@ where
/// The kind of access this capability grants.
///
/// [Definition](https://willowprotocol.org/specs/meadowcap/index.html#communal_cap_mode)
pub fn access_mode(&self) -> &AccessMode {
&self.access_mode
pub fn access_mode(&self) -> AccessMode {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy types should be returned by value

self.access_mode
}

/// The user to whom this capability grants access.
Expand Down
4 changes: 2 additions & 2 deletions meadowcap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub trait IsCommunal {
}

/// A delegation of access rights to a user for a given [area](https://willowprotocol.org/specs/grouping-entries/index.html#areas).
#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct Delegation<
const MCL: usize,
const MCC: usize,
Expand Down Expand Up @@ -100,7 +100,7 @@ where
}

/// A mode granting read or write access to some [`Area`].
#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub enum AccessMode {
Read,
Write,
Expand Down
43 changes: 43 additions & 0 deletions meadowcap/src/mc_authorisation_token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,49 @@ pub struct McAuthorisationToken<
pub signature: UserSignature,
}

impl<
const MCL: usize,
const MCC: usize,
const MPL: usize,
NamespacePublicKey,
NamespaceSignature,
UserPublicKey,
UserSignature,
>
McAuthorisationToken<
MCL,
MCC,
MPL,
NamespacePublicKey,
NamespaceSignature,
UserPublicKey,
UserSignature,
>
where
NamespacePublicKey: NamespaceId + Encodable + Verifier<NamespaceSignature> + IsCommunal,
UserPublicKey: SubspaceId + Encodable + Verifier<UserSignature>,
NamespaceSignature: Encodable + Clone,
UserSignature: Encodable + Clone,
{
pub fn new(
capability: McCapability<
MCL,
MCC,
MPL,
NamespacePublicKey,
NamespaceSignature,
UserPublicKey,
UserSignature,
>,
signature: UserSignature,
) -> Self {
Self {
capability,
signature,
}
}
}

impl<
const MCL: usize,
const MCC: usize,
Expand Down
14 changes: 7 additions & 7 deletions meadowcap/src/mc_capability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl std::error::Error for NotAWriteCapabilityError {}
/// A Meadowcap capability.
///
/// [Definition](https://willowprotocol.org/specs/meadowcap/index.html#Capability)
#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "dev", derive(Arbitrary))]
pub enum McCapability<
const MCL: usize,
Expand Down Expand Up @@ -96,9 +96,9 @@ where
}

/// Create a new owned capability granting access to the [full area](https://willowprotocol.org/specs/grouping-entries/index.html#full_area) of the [namespace](https://willowprotocol.org/specs/data-model/index.html#namespace) to the given `UserPublicKey`.
pub async fn new_owned<NamespaceSecret>(
pub fn new_owned<NamespaceSecret>(
namespace_key: NamespacePublicKey,
namespace_secret: NamespaceSecret,
namespace_secret: &NamespaceSecret,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • No need for this to be async.
  • NamespaceSecret isn't consumed, so take by ref to not have to clone at call site

user_key: UserPublicKey,
access_mode: AccessMode,
) -> Result<Self, OwnedCapabilityCreationError<NamespacePublicKey>>
Expand All @@ -111,7 +111,7 @@ where
}

/// The kind of access this capability grants.
pub fn access_mode(&self) -> &AccessMode {
pub fn access_mode(&self) -> AccessMode {
match self {
Self::Communal(cap) => cap.access_mode(),
Self::Owned(cap) => cap.access_mode(),
Expand Down Expand Up @@ -251,7 +251,7 @@ where
/// Return a new [`McAuthorisationToken`], or an error if the resulting signature was not for the capability's receiver.
pub fn authorisation_token<UserSecret, PD>(
&self,
entry: Entry<MCL, MCC, MPL, NamespacePublicKey, UserPublicKey, PD>,
entry: &Entry<MCL, MCC, MPL, NamespacePublicKey, UserPublicKey, PD>,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entry isn't consumed so take ref to avoid clone at call site

secret: UserSecret,
) -> Result<
McAuthorisationToken<
Expand Down Expand Up @@ -354,12 +354,12 @@ pub(super) mod encoding {

match self {
McCapability::Communal(_) => {
if self.access_mode() == &AccessMode::Write {
if self.access_mode() == AccessMode::Write {
header |= 0b0100_0000;
}
}
McCapability::Owned(_) => {
if self.access_mode() == &AccessMode::Read {
if self.access_mode() == AccessMode::Read {
header |= 0b1000_0000;
} else {
header |= 0b1100_0000;
Expand Down
Loading