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

feat: support execution of delegations #199

Merged
merged 2 commits into from
Jan 20, 2023
Merged

Conversation

Gozala
Copy link
Collaborator

@Gozala Gozala commented Jan 20, 2023

Fixes #195

@Gozala Gozala requested review from gobengo and hugomrdias January 20, 2023 23:15
@hugomrdias
Copy link
Contributor

@Gozala where is this https://github.com/web3-storage/ucanto/blob/e84c659268d104e24bfc0d0354ef436b865b12ca/packages/core/src/delegation.js#L192 function called now ? This is where the ucan is signed and it does not seems to be called.

@gobengo
Copy link
Contributor

gobengo commented Jan 23, 2023

@hugomrdias that delegate fn is called by IssuedInvocation#delegate https://github.com/web3-storage/ucanto/blob/e84c659268d104e24bfc0d0354ef436b865b12ca/packages/core/src/invocation.js#L52 which, after this PR, is called by the transport encoders eg here

in adopting this in w3protocol access-api, I noticed the client-codec.ts file was using the delegate fn you inquired about. Here's what I changed it to: https://github.com/web3-storage/w3protocol/pull/390/files#diff-992aa509308f8ee138534fd4dadef526c3ee5cf920b669c995316b2754b34f41L9

@hugomrdias
Copy link
Contributor

wait but that does not solve the problem, we still signing in the codecs. connection.execute will call the codec and fail

gobengo added a commit to storacha/w3up that referenced this pull request Jan 30, 2023
… and not sign proxyInvocation. using features coming in ucanto 4.2.0

Motivation:
* #325
* simplify access-api ucanto proxy using features added to ucanto in
storacha/ucanto#199
* previously, the technique used to proxy the invocation was to issue a
new invocation (i.e. `proxyInvocation`) in the proxy server, and then
send that to the upstream. This had at least two limitations:
1. required the proxy server to be configured with a `options.signer` to
sign the `proxyInvocation`
2. for functional use in access-api and proxying upload-api, this proxy
`options.signer` also had to be configured pretty much identically to
the ucanto verifier with same did on the upstream, including requiring
both to have the same private key
  * now
* you don't need an `options.signer` at all! so you definitely don't
need one creating signatures with the same private key as the upstream

Steps
* [x] release ucanto 4.2.0
storacha/ucanto#200
* [x] update this source branch package.json + pnpm locks to upgrade
ucanto to 4.2.0
* [x] ensure `tsc` + tests pass here
gobengo added a commit to storacha/w3up that referenced this pull request Apr 11, 2023
… and not sign proxyInvocation. using features coming in ucanto 4.2.0

Motivation:
* #325
* simplify access-api ucanto proxy using features added to ucanto in
storacha/ucanto#199
* previously, the technique used to proxy the invocation was to issue a
new invocation (i.e. `proxyInvocation`) in the proxy server, and then
send that to the upstream. This had at least two limitations:
1. required the proxy server to be configured with a `options.signer` to
sign the `proxyInvocation`
2. for functional use in access-api and proxying upload-api, this proxy
`options.signer` also had to be configured pretty much identically to
the ucanto verifier with same did on the upstream, including requiring
both to have the same private key
  * now
* you don't need an `options.signer` at all! so you definitely don't
need one creating signatures with the same private key as the upstream

Steps
* [x] release ucanto 4.2.0
storacha/ucanto#200
* [x] update this source branch package.json + pnpm locks to upgrade
ucanto to 4.2.0
* [x] ensure `tsc` + tests pass here
This was referenced Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connection should be able to take materialized invocations
3 participants