feat: access-api forwards store/ and upload/ invocations to upload-api

[gobengo]

Motivation:

• make it so access-api can proxy store and upload invocations to upload-api #331

[gobengo]

some of this extra stuff is in here because I wanted to support running the tests with the production private key, which works with a non-error result. But if you don't pass that in, the assertions shift slightly to ensuring that it's an error result of the kind that would happen only if the server signer is wrong

Note how this test was run with WEB3_STORAGE_PRIVATE_KEY:

bengo@bengo ~/protocol.ai/w3protocol/packages/access-api ⚡  DID="$PROD_DID_KEY" WEB3_STORAGE_PRIVATE_KEY="$PROD_PRIVATE_KEY" pnpm test -- -f 'upload-api'

> @web3-storage/access-api@4.2.0 test /Users/bengo/protocol.ai/w3protocol/packages/access-api
> pnpm build && mocha --bail --timeout 10s -n no-warnings -n experimental-vm-modules "-f" "upload-api"


> @web3-storage/access-api@4.2.0 build /Users/bengo/protocol.ai/w3protocol/packages/access-api
> scripts/cli.js build



  proxy store/list invocations to upload-api
    ✔ forwards store/list invocations with aud=did:key (648ms)
    ✔ forwards invocations with aud=did:web:web3.storage (460ms)
    ✔ errors when a bad delegation is given as proof (320ms)

  Store.all
    ✔ proxies store/add to upload-api (331ms)
    ✔ proxies store/remove to upload-api (306ms)
    ✔ proxies store/list to upload-api (385ms)

  Upload.all
    ✔ proxies upload/add to upload-api (303ms)
    ✔ proxies upload/remove to upload-api (287ms)
    ✔ proxies upload/list to upload-api (342ms)


  9 passing (3s)

[Gozala]

Added bunch of Nits, which you can completely ignore or address before or after landing. Only thing I do wish is that we had a test(s) where we spin up local store and upload services and route to those in test so that we have passing cases that don't require actual w3up service and all worked in offline mode.

That said it's probably tricky to do it without moving bunch of code from w3infra here so I think it's good enough for now.

[Gozala]

Nit: We re-export all of the types from the dag-ucan in the ucanto interface & it would be better to use those instead, so we don't have a chance of having two diff versions of dag-ucan in the tree.

[gobengo]

Thanks, I updated to that, and will prefer importing from ucanto libs in the future instead of dag-ucan.

import { UCAN } from '@ucanto/interface';
import { DID } from '@ucanto/core';

[gobengo]

fcc80ea
9559d0e

[Gozala]

Nit: I think ...uploadProvider(), ...storeProvider() would be more aligned with other stuff already in use. Also I think it would be a good idea to make fetch optional so we only need to pass it when overriding default.

[gobengo]

i agree and will do as fast follow #338

[Gozala]

Nit: This whole class seems like incidental complexity, things would be a whole lot simpler with just config object and static function for get method.

P.S.: I know that's not a popular opinion, yet I like to share it in a hope to bring peers to a true side :P

[gobengo]

Is this like what you had in mind? b46dd85

P.S.: I know that's not a popular opinion, yet I like to share it in a hope to bring peers to a true side :P

Eh, I don't disagree necessarily. good suggestion.

[Gozala]

Not exactly, sorry for not been very clear. In linked version you still have audienceConnections which has get function. When I criticize classes what I mean is:

Separate data part which is basically a state from the behavior which are all the methods

That way it does not matter how you constructed state, you can just pass it into a function and it will compute whatever. Separating data and behavior de-tangles two allowing you to store data in e.g. indexDB and then apply function after reading it from indexDB without having to built a class first. Not the case here but complex class heirerchies also often entangle initialization order.

I think there are only two valid reasons to resort to a classes with methods:

1. When you have to deal with open-ended variant space. In other sometimes you have many different state options but only one of them will be used in practice. In this scenario dispatching from one function introduces unnecessary coordination and defining interface is better option.
2. When your methods are simply a convenience sugar for the static functions. I do it all the time to allow method chaining and remove need for importing lot of dependencies. In this case they don't tangle anything they are nothing but a sugar

I would argue use of classes with methods for any other reason just introduces incidental complexity and gets between you and data nothing else.

[gobengo]

@Gozala ah I understand what you mean now.

Separating data and behavior de-tangles two allowing you to store data in e.g. indexDB and then apply function after reading it from indexDB without having to built a class first.

def have run into that. That helped me understand.

[Gozala]

That said it's probably tricky to do it without moving bunch of code from w3infra here so I think it's good enough for now.

Created this issue to that effect storacha/w3infra#127

[gobengo]

cc @hugomrdias

[hugomrdias]

i was hoping for another round of review

[hugomrdias]

Upon for testing this should work as expected

/**
 * @template {API.Ability} A
 * @template {API.URI} R
 * @template {API.Caveats} C
 * @param {API.CapabilityParser<API.Match<API.ParsedCapability<A, R, API.InferCaveats<C>>>>} capability
 * @param {import('../bindings').RouteContext} ctx
 */
function createProxyProvider(capability, ctx) {
  const provider = Server.provide(
    capability,
    async ({ capability, invocation }) => {
      /** @type {import('@ucanto/interface').ConnectionView<import('@web3-storage/access/types').Service>} */
      const connection = connect({
        // This should be the same did:web as access api
        id: ctx.signer,
        encoder: CAR,
        decoder: CBOR,
        channel: HTTP.open({
          // this is the only new global env var
          url: new URL('https://staging.up.web3.storage'),
          method: 'POST',
        }),
      })

      const inv = ucanto.invoke({
        issuer: ctx.signer,
        // This should be the same did:web as access api
        audience: ctx.signer,
        capability,
        proofs: [invocation],
        expiration: invocation.expiration,
        facts: invocation.facts,
        nonce: invocation.nonce,
        notBefore: invocation.notBefore,
      })
      const [result] = await connection.execute(
        /** @type {API.ServiceInvocation<{can: A, with: R, nb: C}, import('@web3-storage/access/types').Service>} */ (
          inv
        )
      )
      return result
    }
  )

  return provider
}

In the service definition (src/service/index.js) we only need:

/**
 * @param {import('../bindings').RouteContext} ctx
 * @returns {import('@web3-storage/access/types').Service}
 */
export function service(ctx) {
  return {
    store: {
      add: createProxyProvider(Store.add, ctx),
    },
    .....
  }
}

and here https://github.com/web3-storage/w3protocol/blob/b77337692d9e4580031c429c429d4055d6f6ebff/packages/access-client/src/types.ts#L67 we need to add

store: {
    add: ServiceMethod<StoreAdd, StoreAddResponse, Failure>
  }