v1.0-rc.1 #6

expede · 2024-01-18T17:56:35Z

FIXMEs
TODOs

expede · 2024-01-24T00:54:25Z

flake.nix

+              name = "docs:build";
+              help = "Refresh the docs";
+              category = "dev";
+              command = "${cargo} doc --features=mermaid_docs";


The features is an annoying workaround since it's duplicated in the Cargo.toml, but AFAICT the pkgs.cargo here and cargo in the shell are different cargos... my guess is that the shell one probably one comes from the toolchain, but I haven't debugged it much yet

avivash · 2024-01-24T00:58:46Z

Cargo.toml

 documentation = "https://docs.rs/rs-ucan"
 repository = "https://github.com/ucan-wg/rs-ucan"
-authors = ["Quinn Wilton <quinn@quinnwilton.com>"]
+authors = ["Quinn Wilton <quinn@quinnwilton.com>", "Brooklyn Zelenka <hello@brooklynzelenka.com"]


we have matching websites 🙈

.github/workflows/bench.yml

QuinnWilton · 2024-01-25T23:59:30Z

src/ability/msg.rs

+
+impl From<MsgReceive> for Ipld {
+    fn from(msg: MsgReceive) -> Self {
+        let mut map = BTreeMap::new();


Nitpick: for cases like this where the set of keys is statically known, it can sometimes be a little less verbose to use the FromIterator trait to construct maps, and avoid needing to mutate the map key-by-key:

BTreeMap::from_iter([ ("to".to_string(), msg.to.to_string().into()), ("from".to_string(), msg.from.to_string().into()) ]).into()

Great tip! Thanks :)

I'd call this more of a protoip than a nitpick, even 😁

QuinnWilton · 2024-01-26T00:12:49Z

src/ability/msg.rs

+    }
+}
+
+impl TryFrom<&Ipld> for MsgReceiveBuilder {


Rather than manually writing this implementation, if you'd like, you can probably use libipld_core with the serde-codec feature to derive Deserialize on MsgReceive, and then use IntoDeserializer to do MsgReceive::deserialize(some_ipld.into_deserializer()).

Something like this:

use serde::de::{Deserialize, IntoDeserializer}; use serde_derive::Deserialize; #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] pub struct MsgReceive { to: Url, from: Url, } impl TryFrom<&Ipld> for MsgReceiveBuilder { type Error = (); fn try_from(ipld: &Ipld) -> Result<Self, ()> { MsgReceive::deserialize(some_ipld.into_deserializer()).map_err(|_| ()) } }

the trait Deserialize<'_> is not implemented for Url

Womp womp

But good tip in general! I guess I can newtype Url, but it's some code either way

UPDATE

27 | pub struct MyUrl(pub Url);
| ^^^ the trait Deserialize<'_> is not implemented for Url

Welp. I guess I'm learning how to write a deserializer 😛

@expede are you importing the url lib w/ the serde feature?

@zeeshanlakhani good point! Ugh, I really dislike how invisible those are in tooling.

What I learned:

libipld-core = { version = "0.16", features = ["serde-codec"] }

use libipld_core::{ipld::Ipld, serde as ipld_serde}; // ... #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] pub struct MsgReceive { to: Url, from: Url, } #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] pub struct MsgReceiveBuilder { to: Option<Url>, from: Option<Url>, } // ... impl TryFrom<Ipld> for MsgReceiveBuilder { type Error = (); fn try_from(ipld: Ipld) -> Result<Self, ()> { ipld_serde::from_ipld(ipld).map_err(|_| ()) // ^^^^^^^^^^^^^^^^^^^^^ // this exists, but only on owned Ipld } }

zeeshanlakhani · 2024-01-26T02:19:17Z

src/invocation.rs

+    pub subject: DID,
+    pub audience: Option<DID>,
+
+    pub ability: B,


So without a task/instruction (not being steadfast about layers here per se, but Task in 0.2 spec), this structure, as is shown and implemented, makes it difficult to have an index to match on a reusable key for replayability. Going off previous versions, this reusable key was composed of:

rsc

input/args/etc

op/(old) ability string, i.e. wasm/run, crud/etc

nonce (nnc)

The nonce as part of this was handy b/c it could separate one instruction/task from the other if you so chose to. With 1.0, rsc is now inside ability/run/arguments, which is fine. But, calculating this generic task "index" is key, and nonce should be a part of it?

As per the 1.0 spec, A Task MUST be uniquely defined by the following fields:

Subject
Command
Arguments
Nonce

For reusability, the sub subject may not be handy here, but the rest are. If we don't want this represented within the structure, then it needs to be something that can be rendered from it (function with a known name quantity that we use).

makes it difficult to have an index to match on a reusable key for replayability.

Yeah, we're going to have to calculate our own. This is better for a few reasons, but the two bigs ones are:

We can pick the serialisation

Getting people to respect the nonce parameter for idempotent actions was going to be hard

Previously, if two people submitted the same invocation, but one was DAG-CBOR and the other was DAG-JSON, we wouldn't have the same index. We can also determine if we want to include the nonce or not based on metadata about the task being run.

For reusability, the sub subject may not be handy here, but the rest are

It depends. It doesn't matter for pure Wasm, but it does as soon as effects show up (i.e. calling the same function on two agents can yield different results)

As per the 1.0 spec, A Task MUST be uniquely defined by the following fields:

Sorry, that's probably my bad. All (100%) of the indexing stuff should be removed from the invocation spec. It's an IPVM concern, not a UCAN one.

For reusability, the sub subject may not be handy here, but the rest are

It depends. It doesn't matter for pure Wasm, but it does as soon as effects show up (i.e. calling the same function on two agents can yield different results)

Right. I guess, it's not something you always want in the calculation is what I want to say.

makes it difficult to have an index to match on a reusable key for replayability.

Yeah, we're going to have to calculate our own. This is better for a few reasons, but the two bigs ones are:

We can pick the serialisation

Getting people to respect the nonce parameter for idempotent actions was going to be hard

Previously, if two people submitted the same invocation, but one was DAG-CBOR and the other was DAG-JSON, we wouldn't have the same index. We can also determine if we want to include the nonce or not based on metadata about the task being run.

yeah, I think the key is dealing with duplicate tasks once again, and that nonce has meaning there if you want to run the same thing (for whichever reason).

I'm trying to remember back to when this was written, but we probably usually (always?) do want the subject in there, because ultimately they're the one that set the semantics

Yeah. It wasn't part of the original task/instruction in 0.1/0.2. It was there, but not included. If it's the case that sub (in like the wasm case) will typically be the same across runs (and machines), then that's fine. Mainly, what works for reusability when we want to allow that.

zeeshanlakhani · 2024-01-26T02:24:31Z

src/ability/msg.rs

+    from: Url,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]


as per your video, we can remove these via https://crates.io/crates/optional_struct, so you can generate optional versions.

There's also builder_macro... any reason to prefer optional_struct?

You mean https://docs.rs/derive_builder/latest/derive_builder/? Yeah, I'd say the latter is more explicit about the Option version. derive_builder actually has different use cases. So, I'd say it's about intention.

zeeshanlakhani · 2024-01-26T10:23:52Z

tests/conformance.rs

@@ -2,7 +2,7 @@ use libipld_core::{ipld::Ipld, raw::RawCodec};
 use serde::{Deserialize, Serialize};
 use std::{collections::HashMap, fs::File, io::BufReader, str::FromStr};

-use rs_ucan::{
+use rs-ucan::{


minor: I can't find @matheus23's comments now, but while the internal use of crate names remains unclear, snake_case still seems to be the convention (dash for dep of crate, _ for internal use).

FWIW the original rs-ucan crate uses the ucan crates.io name.
Also rs-wnfs uses the wnfs crate name.
Not sure why Quinn changed the ucan crate name into rs_ucan.

Yep. I was also going to say we shouldn't rs everything anyhow.

Oh yeah, it's against the community guidelines

Crate names should not use -rs or -rust as a suffix or prefix. Every crate is Rust! It serves no purpose to remind users of this constantly.

Not sure why Quinn changed the ucan crate name into rs_ucan.

It was me being annoyed by mistyping, and the highlighted line is me pushing a global find-and-replace to revert that

It was me being annoyed by mistyping, and the highlighted line is me pushing a global find-and-replace to revert that

I was referring to the fact that Quinn changed the crate name from ucan to rs_ucan, not the fact that you changed rs_ucan to rs-ucan :)

matheus23

Thanks for opening up the discussion on this so early 🤗 It's certainly not the most 'fun' thing to have everyone give their thoughts on something that's in an early phase.
But I think that was a really good decision. It's going to be a fairly central piece of the stack, most likely making its way into the server, homestar, possibly bound into frontend libs, possibly a future dependency of wnfs (submarine sealing), etc.

benches/a_benchmark.rs

matheus23 · 2024-01-26T13:13:05Z

src/prove.rs

+pub trait TryProve<'a, T> {
+    type Error;
+    type Proven;
+
+    fn try_prove(&'a self, candidate: &'a T) -> Result<&'a Self::Proven, Self::Error>;
+}


I suspect that this version of the trait won't quite work, or at least I'm confused about how it would.

To write a function that takes an Invocation and verifies that some A: Ability can be proven, that function will need to take the proofs: Vec<Cid> in the Invocation and start fetching and parsing them to valid Delegations.
But what type would you use for the abilities in these Delegations?
Likely it'll be the same ability type for all elements in the Vec, because it needs to be homogenous.
However, trait TryProve<'a, T> allows self, the candidate and return type Self::Proven to be different types.

Grepping all impl TryProves it seems like in most cases you're doing impl TryProve<X> for X with type Proven = X.
So perhaps it should just be fn try_prove(&self, candidate: &Self) -> Result<Self, Self::Error>?
The only exception is the Crud ability. In that case it looks like there exist e.g. impl TryProve<Crud> for CrudRead with type Proven = CrudRead. I'm not sure exactly what you're trying to solve, but my gut reaction would be to simply have an enum Crud { CrudAll, CrudRead, CrudWrite, ... }.

Ah, it does not take an invocation! It only takes Abilities (command + args). The validation around that just depends on this being implemented for the T in Invocation<T> / Delegation<T> / Receipt<T>

Hmm. I'm confused. Let me try to get clarification:

There will be a Invocation::<T>::check(&self, ability: impl TryProve<...>, ...) -> Result<...> function that verifies the given ability is valid in the invocation, right? Or perhaps that's even just Invocation::<T>::check(&self, ...) -> Result<...>, and it'll just return the ability in the invocation or sth.

The invocation checking code will call the TryProve implementations of your abilities, right?

To be clear, I wrote:

To write a function that takes an Invocation and verifies that some A: Ability can be proven

But I wasn't talking about try_prove as being "that function". I was talking about a hypothetical Invocation::check function

matheus23 · 2024-01-26T13:25:03Z

src/ability/crud.rs

+pub struct Crud {
+    uri: Field<Url>,
+}
+
+pub struct CrudRead {
+    pub uri: Field<Url>,
+}
+
+pub struct CrudMutate {
+    uri: Field<Url>,
+}
+
+pub struct CrudCreate {
+    pub uri: Field<Url>,
+    pub args: BTreeMap<Box<str>, Field<String>>,
+}
+
+pub struct CrudUpdate {
+    pub uri: Field<Url>,
+    pub args: BTreeMap<Box<str>, Field<String>>,
+}
+
+pub struct CrudDestroy {
+    pub uri: Field<Url>,
+}


This is setting a really high bar for type safety, which I commend, but could possibly be hard to realize.

I'm trying to categorize the 'needs' for these different types:

in Delegations: Needs the ability to "name" the ability (kind of like a function name), and needs to know about which abilities can be derived from others. E.g. account/info can be derived from account/noncritical, but account/delete cannot. No need to know about arguments yet (unless we try to integrate conditions here, too).

in Invocations: Needs not only the ability name, but also values for all arguments. Which abilities derive others is irrelevant. Stuff like account/noncritical doesn't even have a defined set of arguments and shouldn't even be usable in an invocation.

in Workflows/Tasks(? I'm fuzzy on the terminology here): Needs ability arguments to have a representation for "deferred". Otherwise fairly similar needs to "in Invocations".

in Receipts: Needs all arguments to be there, none of them optional nor deferred.

Maybe for now, we'll separate these into let's say two? I.e. require uses to define two structs for each , one for use within Delegation and one for use within Invocation/Tasks/Receipts.

I can see a future where we improve the situation with a derive proc-macro like in the future then.

Yep, this is the "I wish I had HKTs" stuff that we talked about. You'll see the expanded code hopefully later today, but we can also macro-ize it. If it turns out to be too much of a pain, we can drop to a dynamic version, but so far writing this feels more straightforward than perhaps it feels?

This is also all standard lib — I think 99% of of people will just use the stuff off the shelf because they're general enough for most use cases.

But hey, we'll see as this comes together :)

expede · 2024-01-26T17:40:46Z

@matheus23

It's certainly not the most 'fun' thing to have everyone give their thoughts on something that's in an early phase.

haha yes, I have been somewhat descended upon... thanks for noting that!

I also kind of want to model the pattern. It's something that we used to do at Fission, but seems to have stopped (because it's uncomfortable!), buuuuuut it also lets people pipe in with "oh shit I have something for that over here!" or "uh, you probably don't want to do that", which is happening in this thread :) Working in the open!

pre-commit.nix

zeeshanlakhani · 2024-01-26T20:34:24Z

src/ability/wasm.rs

+}
+
+#[derive(Debug, Clone, PartialEq)]
+pub enum Module {


@expede is more of a spec question. Can't this also be a URL? Or should there be a tag where it may be fetched from, as we do not always know it's a Cid from IPFS for example? So, there needs to be some info somewhere on how to retrieve it. Is it elsewhere and I'm not seeing it?

Can't this also be a URL?

There's no spec for the wasm ability yet, so we'll define it together!

The way I was thinking about this is that resolving a module for $SOMEWHERE_ELSE would be captured as an effect so that we can cache the output using the normal machinery. I think the higher level DSL will do things like construct the correct workflow for this.

The other option is to special-case URL resolution specifically for Wasm at this layer, which I don't love — because we're already going to have that functionality in the system with our first-class effects — but I'm open to talking about it

Yeah. We should discuss this, as it just needs to be defined somewhere. It's fine if it's outside of the execution (which is how it's done today in the runtime anyway but just pulled from what was rsc of course).

matheus23 · 2024-03-19T13:33:40Z

src/delegation/payload.rs

+    /// given as a [Unix timestamp].
+    ///
+    /// [Unix timestamp]: https://en.wikipedia.org/wiki/Unix_time
+    pub expiration: Timestamp,


Suggested change

pub expiration: Timestamp,

pub expiration: Option<Timestamp>,

Shouldn't this be optional? I know it needs to be present, but it can also be set to null, and Timestamp doesn't have a representation for null.

The exp field MUST be set. Following the principle of least authority, it is RECOMMENDED to give a timestamp expiry for UCANs. If the token explicitly never expires, the exp field MUST be set to null. If the time is in the past at validation time, the token MUST be treated as expired and invalid.

* refactor: Make delegation::Store::get return Option * refactor: Make delegation::Store::insert take &self instead of &mut self * refactor: Make invocation::Store take &self instead of &mut self * refactor: Make `delegation::Agent` not take `&mut self` in methods * refactor: Make `Agent` take `DID` by value * refactor: Take `DID` by value in `delegation::Agent::new` * refactor: Change generic order in `delegation::Agent` and add defaults

Test delegation store

Wrap auto CID Insert error

…x-chain Remove unrestricted powerbox chain

…chain`

This reverts commit c06e6de.

More bugfixes & some suggested changes

Small change to get this up in a PR

692a4d4

expede changed the title ~~Small change to get this up in a PR~~ v1.0-rc.1 Jan 18, 2024

expede added 8 commits January 18, 2024 16:40

feat: move flake.nix to numtide/devshell

c6e9e17

Update README to use new GitHub markdown features

3177cae

Trim whitespace

a31445a

feat: frustration

3cda14f

fix: typo

d6a62fb

Adding some TODOs and FIXMEs to come back to

0e0efa2

Exploratory programming

77dafd9

Formatter

9c38b9b

expede commented Jan 24, 2024

View reviewed changes

avivash reviewed Jan 24, 2024

View reviewed changes

expede added 5 commits January 23, 2024 22:22

Splitting out separate files

a5b5434

Add Signature envelope and receipts

6ec904c

More exploration

fb8336f

WIP

aaa2a79

More exploratory programming, cleanup, and conds

ad94b97

expede commented Jan 25, 2024

View reviewed changes

.github/workflows/bench.yml Outdated Show resolved Hide resolved

QuinnWilton reviewed Jan 25, 2024

View reviewed changes

QuinnWilton reviewed Jan 26, 2024

View reviewed changes

zeeshanlakhani reviewed Jan 26, 2024

View reviewed changes

WIP

e72b6d8

zeeshanlakhani reviewed Jan 26, 2024

View reviewed changes

matheus23 reviewed Jan 26, 2024

View reviewed changes

Remove -rs from package name

9cb4126

zeeshanlakhani reviewed Jan 26, 2024

View reviewed changes

pre-commit.nix Outdated Show resolved Hide resolved

zeeshanlakhani reviewed Jan 26, 2024

View reviewed changes

Thanks for the suggestions everyone! 🎉

7a5452f

Working on edge cases in selector parser

b965982

matheus23 reviewed Mar 19, 2024

View reviewed changes

matheus23 and others added 28 commits March 22, 2024 00:54

Fairly complete predicate tests

3503a32

WIP rebasing

deee2bc

Revert so that can push

93cce69

Correct the FIXME for command

643b466

Test delegation store

faba286

WIP testing

246578b

Save WIP writng more tests

279f17c

Not pretty, but got that last selector passing

6affc01

Add automatic CID conversaion error

bc0a275

Receipts as invocations

96b0fe1

Require Subject on get_chain

4913ad0

Merge pull request #12 from fission-codes/test-deleg-store

13510c5

Test delegation store

Merge pull request #13 from fission-codes/wrap_auto_cid_insert

ff87859

Wrap auto CID Insert error

Merge pull request #15 from fission-codes/remove-unrestricted-powerbo…

d854438

…x-chain Remove unrestricted powerbox chain

fix: Error early in Agent::invoke if there's no valid delegation chain

2172b21

fix: Check for delegations for the specific command, not for "/"

f894e69

refactor: Rename MissingDelegation -> DelegationNotFound

5d26366

refactor: Use Mutex instead of RwLock

4bd583a

fix: Check for the appropriate command in Agent::delegate

fd0922f

refactor: Take command: &str instead of by String in `Store::get_…

0dfb2a6

…chain`

refactor: Turn try_fold into for loop in Invocation::check

8cc6ac5

Revert "refactor: Turn try_fold into for loop in Invocation::check"

d0fab39

This reverts commit c06e6de.

Merge pull request #11 from fission-codes/matheus23/v1.0-2

5dbd296

More bugfixes & some suggested changes

refactor: Remove unused constraints (#17)

7f680d3

Nonce simplification (#19)

53c49d2

Rename CannotCidOr, API for creating powerlines & more (#18)

a00db2d

Cleanup unused imports & bump Nix Flake (#16)

a8d1b1d

	pub expiration: Timestamp,
	pub expiration: Option<Timestamp>,

v1.0-rc.1 #6

Are you sure you want to change the base?

v1.0-rc.1 #6

Conversation

expede commented Jan 18, 2024 • edited

expede Jan 24, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

expede Jan 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

expede Jan 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

expede Jan 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

expede Jan 26, 2024 • edited

Choose a reason for hiding this comment

expede Jan 26, 2024 • edited

Choose a reason for hiding this comment

expede Jan 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeeshanlakhani Jan 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

expede Jan 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matheus23 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matheus23 Jan 29, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

expede commented Jan 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matheus23 Mar 19, 2024 • edited

Choose a reason for hiding this comment

expede commented Jan 18, 2024 •

edited

expede Jan 24, 2024 •

edited

expede Jan 26, 2024 •

edited

expede Jan 26, 2024 •

edited

expede Jan 26, 2024 •

edited

expede Jan 26, 2024 •

edited

expede Jan 26, 2024 •

edited

expede Jan 26, 2024 •

edited

zeeshanlakhani Jan 26, 2024 •

edited

expede Jan 26, 2024 •

edited

matheus23 Jan 29, 2024 •

edited

matheus23 Mar 19, 2024 •

edited