feat: write invocations and receipts into ucan log #592

Gozala · 2023-03-21T21:15:00Z

Fixes #581

Overview

Integrates changes from feat: ucan invocation post handled car #579
- I have added undici to pnpm overrides
Gets rid of raw route in favor of root which now uses decoder/encoder based of content type
Defines UCANLog interface for sending invocations & receipts to kenisis
- As per feat: ucan invocation post handled car #579 we probably want to use CF Queue for failure tolerance which can be a followup improvement
Type fixes to unblock some changes
- You can't do @implements {import('./bindings').UCANLog} nor you can type alias and do implements because it no longer an interface. Solution is to do import * as API from "./bindings.js" in which case you can @implements {API.UCANLog}, which is why end up changing bindings.d.ts to bindings.ts + bindings.js.
- Turns out .d.ts files aren't type checked so ☝️ uncovered bunch of errors I had to fix

Notes

I'm not happy how this reaches deep into ucanto & I'd like to uplift changes around encode / decode / execute into ucanto. Once that is done I'll make a followup change here to replace some of these with ucanto exposed functionality
- It also would make it easier to integrate things in the upload-api
I do not seem to have privileges to add vars and secrets in this, so someone with enough privileges should add UCAN_LOG_URL or UCAN_LOG_BASIC_AUTH so someone needs to do it to make this work.
I end up sending invocations and receipts to the same endpoint but different content-type header which I think makes sense.
- Also unlike feat: ucan invocation post handled car #579 I've decided to decouple endpoint from upload endpoint as they're logically unrelated.

gobengo · 2023-03-21T21:37:37Z

packages/access-api/src/bindings.d.ts

@@ -73,6 +74,11 @@ export interface RouteContext {
  uploadApi: ConnectionView
 }

+interface UCANLog {
+  logInvocations: (car: Uint8Array) => Promise<void>
+  logReceipt: (receipt: Uint8Array) => Promise<void>


Is there a reason to allow logging more than one invocations at a time, but only one receipt at a time?

Any downsides to this being logReceipts? It seems nice to allow for logging them in batches

Invocations are run concurrently so in order to log receipts together we'd have to await for all of them to finish before we can log receipts. It seems like logging receipt right after invocation is complete is a better idea than having to await for the other invocations.

we'd have to await for all of them to finish before we can log receipts

Yikes. I definitely don't want to do that.

It seems like logging receipt right after invocation is complete is a better idea than having to await for the other invocations.

logReceipts could still be called several times as receipts are available (even one at a time). I'm only suggesting that the API afford for batching, not that we wait for all invocations to create only one batch.

…d-car' into feat/ucan-log

vasco-santos · 2023-03-22T08:08:20Z

I do not seem to have privileges to add vars and secrets in this, so someone with enough privileges should add UCAN_LOG_URL or UCAN_LOG_BASIC_AUTH so someone needs to do it to make this work.

@Gozala set those and made you org owner

vasco-santos

This is looking good! Thanks for tackling this @Gozala

I agree we should move this into ucanto as follow up. Let me work on the changes in w3infra side before approving to make sure everything fits nicely

vasco-santos · 2023-03-22T08:12:21Z

packages/access-api/src/bindings.ts

+export {}
+


This looks like it shouldn't be needed?

packages/access-api/src/routes/root.js

vasco-santos · 2023-03-22T11:15:25Z

packages/access-api/src/routes/root.js

+    ran: invocation.cid,
+    out,
+    fx: { fork: [] },
+    meta: {},


Currently there is no way to identify which invocation capability was the root for this receipt without opening the CAR file. This is not ideal from a stream of such events.

We should be able to filter out the events we want to handle per capability so that we do not need to read all the CARs to get to know about them. For example, let's consider a stream consumer (with batches of 20) that tracks number of store/add successful invocations. This consumer would be required to go through all the receipts received, read all the CAR files from the invocations and check the capability. If from these 20 CARs there are no store/add operations to address, all this processing was useless.

Thinking about adding capability from invocation in meta as a good way to flag this. We would read CARs mentioned in receipts with capability we care about (and could still validate that capability is really what is given in meta).

Thoughts?

Currently there is no way to identify which invocation capability was the root for this receipt without opening the CAR file. This is not ideal from a stream of such events.

It's not that bad...right? It's a local bucket so won't be costly. Can we see how it pans out and change if necessary? I'd rather not rely on something in a meta field...

...or maybe a better idea - the UCAN log service should validate that the receipt belongs to a stored invocation. It shoudl read the invocation and annotate what it sends to the stream with what you need.

...or maybe a better idea - the UCAN log service should validate that the receipt belongs to a stored invocation. It shoudl read the invocation and annotate what it sends to the stream with what you need.

I think this should be good yes, I will report back once I try it out

It's not that bad...right? It's a local bucket so won't be costly. Can we see how it pans out and change if necessary? I'd rather not rely on something in a meta field...

Depends, if we could use Kinesis filters already that would be a bumper if we could not filter out consumers right away. But your solution above should work well

alanshaw

LGTM - thanks @Gozala

alanshaw · 2023-03-22T11:28:07Z

packages/access-api/package.json

@@ -8,6 +8,7 @@
  "private": true,
  "scripts": {
    "lint": "tsc --build && eslint '**/*.{js,ts}' && prettier --check '**/*.{js,ts,yml,json}' --ignore-path ../../.gitignore",
+    "fix:lint": "eslint --fix '**/*.{js,ts}'",


Suggested change

"fix:lint": "eslint --fix '**/*.{js,ts}'",

"lint:fix": "eslint --fix '**/*.{js,ts}'",

packages/access-api/src/routes/root.js

alanshaw · 2023-03-22T11:46:42Z

packages/access-api/src/routes/root.js

+    ran: invocation.cid,
+    out,
+    fx: { fork: [] },
+    meta: {},


Currently there is no way to identify which invocation capability was the root for this receipt without opening the CAR file. This is not ideal from a stream of such events.

It's not that bad...right? It's a local bucket so won't be costly. Can we see how it pans out and change if necessary? I'd rather not rely on something in a meta field...

vasco-santos · 2023-03-22T14:18:29Z

packages/access-api/src/routes/root.js

+    headers['content-type'] === 'application/car' ? [CAR, CBOR] : [json, json]
+
+  const invocations = await decoder.decode({
+    body,
    headers: Object.fromEntries(request.headers.entries()),


Suggested change

headers: Object.fromEntries(request.headers.entries()),

headers,

gobengo · 2023-03-22T16:44:08Z

.github/workflows/reusable-deploy-api.yml

@@ -24,6 +24,8 @@ on:
        required: true
      LOGTAIL_TOKEN:
        required: true
+      UCAN_LOG_BASIC_AUTH:


can this include a description of the format? e.g. in the past I've wondered if it should be base64 encoded or not. If not commented, I assume yes? (i.e. I'd assume it's the value in an http header after 'Basic '.

Value after Basic encoded as base64

gobengo · 2023-03-22T16:50:38Z

packages/access-api/src/utils/ucan-log.js

+  async logInvocations(car) {
+    await pRetry(
+      () =>
+        fetch(this.url, {


do we want to retry on http response status 502 and/or 503?

I think those cases would not reject the fetch promise, so wouldn't retry here.

vasco-santos

LGTM

@Gozala feel free to merge. If so, please replace secrets for now to not perform the HTTP Requests until we merge storacha/w3infra#172

gobengo · 2023-03-22T16:52:01Z

packages/access-api/src/utils/ucan-log.js

+          method: 'POST',
+          headers: {
+            Authorization: `Basic ${this.auth}`,
+            'Content-Type': 'application/invocations+car',


Is this defined anywhere?
nit: If it's ad-hoc, application/vnd.... might be appropriate.

gobengo · 2023-03-22T16:52:56Z

packages/access-api/src/utils/ucan-log.js

+  /**
+   * @param {object} input
+   * @param {URL} input.url
+   * @param {string} [input.auth]


it could be helpful to include a comment on what kind of string this is. e.g. should it be base64, base64url, what kind of auth (http basic?)

gobengo · 2023-03-22T16:56:26Z

packages/access-api/src/utils/ucan-log.js

+  /**
+   * @param {Uint8Array} car
+   */
+  async logInvocations(car) {


do tests run in this implementation?

gobengo

none of my comments should block

@Gozala

…ts (#172) This PR includes two significant changes: - `upload-api` ucanto server now follows same pattern as `access-api` proposal from @Gozala's storacha/w3up#592 - saves CAR file with UCAN invocations before executing them - sends one receipt per invocation when they are executed - modifies `POST /ucan` endpoint to accept both receipts and CAR invocations as discussed sync yesterday - it identifies by ContentType wether is a CAR of invocations or a Receipt (`processUcanLogRequest`) - for CAR of invocations, everything is quite similar as before: - parse CAR from bytes, persist CAR and send a view of it to Kinesis - more than persisting the CARs, we also persist a map of invocation CIDs to CARs where they live in same fashion as we do with DUDEWHERE - kinesis now also receives a type property, which can either be Invocation, or receipt like the actual endpoint - NOTE: removed `proofs` as they can get quite big and we already talked about getting rid of it before - for receipts: - we parse the receipt CBOR, verify if given receipt is from an Invocation that we previously got. If so, we can persist the Receipt - Kinesis stream receives quite similar content to the invocation. Main differences are that a receipt type is used, and we also include the `inovcationCid`. In other words, we provide value from invocation so that Kinesis consumers can know about `invocation` and `nb` from invocation to filter out and account metrics Other notes: - `upload-api` hooks in the same functions for integrating with UCAN Stream as the `/ucan` endpoint, avoiding the HTTP calls - Storing receipts and invocations Bucket keys were decided yesterday. - Like storacha/w3up#592, we aim to get to ucanto part of the code that we currently inline to create the receipts - Will do a follow up PR for metric consumers to update to new world

🤖 I have created a release *beep* *boop* --- ## [5.0.0](access-api-v4.11.0...access-api-v5.0.0) (2023-03-23) ### ⚠ BREAKING CHANGES * implement new account-based multi-device flow ([#433](#433)) * upgrade capabilities to latest ucanto ([#463](#463)) ### Features * access-api handles provider/add invocations ([#462](#462)) ([5fb56f7](5fb56f7)) * access-api serves access/claim invocations ([#456](#456)) ([baacf35](baacf35)) * access/authorize confirmation email click results in a delegation back to the issuer did:key so that access/claim works ([#460](#460)) ([a466a7d](a466a7d)) * allow multiple providers ([#595](#595)) ([96c5a2e](96c5a2e)) * define `access/confirm` handler and use it in ucanto-test-utils registerSpaces + validate-email handler ([#530](#530)) ([b1bbc90](b1bbc90)) * handle access/delegate invocations without error ([#427](#427)) ([4f0bd1c](4f0bd1c)) * if POST /validate-email?mode=authorize catches error w/ too big qr code ([#516](#516)) ([d0df525](d0df525)) * implement new account-based multi-device flow ([#433](#433)) ([1ddc6a0](1ddc6a0)) * includes proofs chains in the delegated authorization chain ([#467](#467)) ([5144293](5144293)) * move access-api delegation bytes out of d1 and into r2 ([#578](#578)) ([4510c9a](4510c9a)) * move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ ([#449](#449)) ([02d7552](02d7552)) * provision provider type is now the DID of the w3s service ([#528](#528)) ([6a72855](6a72855)) * space/info will not error for spaces that have had storage provider added via provider/add ([#510](#510)) ([ea4e872](ea4e872)) * upgrade capabilities to latest ucanto ([#463](#463)) ([2d786ee](2d786ee)) * upgrade to new ucanto ([#498](#498)) ([dcb41a9](dcb41a9)) * write invocations and receipts into ucan log ([#592](#592)) ([754bf52](754bf52)) ### Bug Fixes * access/delegate checks hasStorageProvider(space) in a way that provider/add allows access/delegate ([#483](#483)) ([f4c640d](f4c640d)) * adjust migration 0005 to keep delegations table but create new used delegations_v2 ([#469](#469)) ([a205ad1](a205ad1)) * adjust migration 0005 to not do a drop table and instead rename delegations -> delegations_old and create a new delegations ([#468](#468)) ([6c8242d](6c8242d)) * allow injecting email ([#466](#466)) ([e19847f](e19847f)) * DbDelegationsStorage#find throws UnexpectedDelegation w/ { row } if failed bytesToDelegations ([#476](#476)) ([a6dafcb](a6dafcb)) * DbProvisionsStorage putMany doesnt error on cid col conflict ([#517](#517)) ([c1fea63](c1fea63)) * delegations model tries to handle if row.bytes is Array not Buffer (e.g. cloudflare) ([#478](#478)) ([030e7b7](030e7b7)) ### Miscellaneous Chores * **access-client:** release 11.0.0-rc.0 ([#573](#573)) ([be4386d](be4386d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

Fixes #581 ## Overview - Integrates changes from #579 - I have added `undici` to [pnpm overrides](https://pnpm.io/package_json) - Gets rid of `raw` route in favor of `root` which now uses decoder/encoder based of content type - Defines `UCANLog` interface for sending invocations & receipts to kenisis - As per #579 we probably want to use CF Queue for failure tolerance which can be a followup improvement - Type fixes to unblock some changes - You can't do `@implements {import('./bindings').UCANLog}` nor you can type alias and do implements because it no longer an interface. Solution is to do `import * as API from "./bindings.js"` in which case you can `@implements {API.UCANLog}`, which is why end up changing `bindings.d.ts` to `bindings.ts` + `bindings.js`. - Turns out `.d.ts` files aren't type checked so ☝️ uncovered bunch of errors I had to fix ## Notes - I'm not happy how this reaches deep into ucanto & I'd like to uplift changes around encode / decode / execute into ucanto. Once that is done I'll make a followup change here to replace some of these with ucanto exposed functionality - It also would make it easier to integrate things in the upload-api - I do not seem to have privileges to add vars and secrets in this, so someone with enough privileges should add `UCAN_LOG_URL` or` UCAN_LOG_BASIC_AUTH` so someone needs to do it to make this work. - I end up sending invocations and receipts to the same endpoint but different content-type header which I think makes sense. - Also unlike #579 I've decided to decouple endpoint from upload endpoint as they're logically unrelated.

🤖 I have created a release *beep* *boop* --- ## [5.0.0](access-api-v4.11.0...access-api-v5.0.0) (2023-03-23) ### ⚠ BREAKING CHANGES * implement new account-based multi-device flow ([#433](#433)) * upgrade capabilities to latest ucanto ([#463](#463)) ### Features * access-api handles provider/add invocations ([#462](#462)) ([46da0df](46da0df)) * access-api serves access/claim invocations ([#456](#456)) ([2ec16e9](2ec16e9)) * access/authorize confirmation email click results in a delegation back to the issuer did:key so that access/claim works ([#460](#460)) ([fc62691](fc62691)) * allow multiple providers ([#595](#595)) ([aba57b3](aba57b3)) * define `access/confirm` handler and use it in ucanto-test-utils registerSpaces + validate-email handler ([#530](#530)) ([a08b513](a08b513)) * handle access/delegate invocations without error ([#427](#427)) ([db01d07](db01d07)) * if POST /validate-email?mode=authorize catches error w/ too big qr code ([#516](#516)) ([ab83b19](ab83b19)) * implement new account-based multi-device flow ([#433](#433)) ([6152e55](6152e55)) * includes proofs chains in the delegated authorization chain ([#467](#467)) ([743a72f](743a72f)) * move access-api delegation bytes out of d1 and into r2 ([#578](#578)) ([3029e4a](3029e4a)) * move validation flow to a Durable Object to make it ⏩ fast ⏩ fast ⏩ fast ⏩ ([#449](#449)) ([3868d97](3868d97)) * provision provider type is now the DID of the w3s service ([#528](#528)) ([4cd6cd9](4cd6cd9)) * space/info will not error for spaces that have had storage provider added via provider/add ([#510](#510)) ([362024f](362024f)) * upgrade capabilities to latest ucanto ([#463](#463)) ([e375ae4](e375ae4)) * upgrade to new ucanto ([#498](#498)) ([790750d](790750d)) * write invocations and receipts into ucan log ([#592](#592)) ([d52a281](d52a281)) ### Bug Fixes * access/delegate checks hasStorageProvider(space) in a way that provider/add allows access/delegate ([#483](#483)) ([1d3d562](1d3d562)) * adjust migration 0005 to keep delegations table but create new used delegations_v2 ([#469](#469)) ([d90825a](d90825a)) * adjust migration 0005 to not do a drop table and instead rename delegations -> delegations_old and create a new delegations ([#468](#468)) ([89f2acd](89f2acd)) * allow injecting email ([#466](#466)) ([b4b0173](b4b0173)) * DbDelegationsStorage#find throws UnexpectedDelegation w/ { row } if failed bytesToDelegations ([#476](#476)) ([660f773](660f773)) * DbProvisionsStorage putMany doesnt error on cid col conflict ([#517](#517)) ([8c6dea8](8c6dea8)) * delegations model tries to handle if row.bytes is Array not Buffer (e.g. cloudflare) ([#478](#478)) ([02c0c28](02c0c28)) ### Miscellaneous Chores * **access-client:** release 11.0.0-rc.0 ([#573](#573)) ([29daa02](29daa02)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

vasco-santos and others added 4 commits March 21, 2023 15:39

feat: ucan invocation post handled car

50c6167

chore: wrap post with retry and rename post url var

5f3c1db

chore: update context

1d45995

feat: implement receipts

62a7a10

Gozala requested review from alanshaw, gobengo and vasco-santos March 21, 2023 21:15

gobengo reviewed Mar 21, 2023

View reviewed changes

Gozala added 6 commits March 21, 2023 15:06

chore: add some comments

9536da1

Merge remote-tracking branch 'origin/feat/ucan-invocation-post-handle…

9dfabe2

…d-car' into feat/ucan-log

integrate changes from #579

c5cf9ad

fix: lint issues

5c2ea98

chore: use pnpm overrides for undici

6388db3

chore: add tests

2f03ce8

Gozala marked this pull request as ready for review March 22, 2023 01:45

Gozala added 2 commits March 21, 2023 18:53

undo unintended changes

cf81fca

Merge branch 'main' into feat/ucan-log

7f140c9

vasco-santos reviewed Mar 22, 2023

View reviewed changes

alanshaw approved these changes Mar 22, 2023

View reviewed changes

vasco-santos reviewed Mar 22, 2023

View reviewed changes

vasco-santos mentioned this pull request Mar 22, 2023

feat: ucan stream endpoint and upload api with invocations and receipts storacha/w3infra#172

Merged

gobengo reviewed Mar 22, 2023

View reviewed changes

vasco-santos approved these changes Mar 22, 2023

View reviewed changes

gobengo reviewed Mar 22, 2023

View reviewed changes

gobengo self-requested a review March 22, 2023 16:57

gobengo approved these changes Mar 22, 2023

View reviewed changes

vasco-santos mentioned this pull request Mar 22, 2023

feat: ucan invocation post handled car #579

Closed

1 task

Alan Shaw added 6 commits March 23, 2023 14:06

Merge branch 'main' into feat/ucan-log

36b870f

fix: bindings types

9916589

fix: types

804d43a

chore: run prettier fix

846cf9b

fix: throw for adverse http status

0dd6462

chore: appease linter

a109735

alanshaw merged commit 754bf52 into main Mar 23, 2023

alanshaw deleted the feat/ucan-log branch March 23, 2023 14:40

it-dag-house mentioned this pull request Mar 23, 2023

chore(main): release access-api 5.0.0 #588

Merged

it-dag-house mentioned this pull request Mar 23, 2023

chore(main): release access-api 11.0.0-rc.0 #620

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: write invocations and receipts into ucan log #592

feat: write invocations and receipts into ucan log #592

Gozala commented Mar 21, 2023 •

edited

Loading

gobengo Mar 21, 2023

Gozala Mar 22, 2023

gobengo Mar 22, 2023

vasco-santos commented Mar 22, 2023

vasco-santos left a comment

vasco-santos Mar 22, 2023

vasco-santos Mar 22, 2023

alanshaw Mar 22, 2023

alanshaw Mar 22, 2023

vasco-santos Mar 22, 2023

vasco-santos Mar 22, 2023

alanshaw left a comment

alanshaw Mar 22, 2023

alanshaw Mar 22, 2023

vasco-santos Mar 22, 2023

gobengo Mar 22, 2023

vasco-santos Mar 22, 2023

gobengo Mar 22, 2023

vasco-santos left a comment

gobengo Mar 22, 2023

gobengo Mar 22, 2023

gobengo Mar 22, 2023

gobengo left a comment

	"fix:lint": "eslint --fix '*/.{js,ts}'",
	"lint:fix": "eslint --fix '*/.{js,ts}'",

	headers: Object.fromEntries(request.headers.entries()),
	headers,

feat: write invocations and receipts into ucan log #592

feat: write invocations and receipts into ucan log #592

Conversation

Gozala commented Mar 21, 2023 • edited Loading

Overview

Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vasco-santos commented Mar 22, 2023

vasco-santos left a 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

alanshaw left a 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

vasco-santos left a 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

gobengo left a comment

Choose a reason for hiding this comment

Gozala commented Mar 21, 2023 •

edited

Loading