airgap/dapp-wallet-messages

jsamol commented 1 year ago

This PR proposes a message standard that defines communication between dApps and wallets as described in RFP-7.

dfinity-droid-prod[bot] commented 1 year ago

Dear @jsamol,

In order to potentially merge your code in this open-source repository and therefore proceed with your contribution, we need to have your approval on DFINITY's CLA[^1].

If you decide to agree with it, please visit this issue and read the instructions there.

— The DFINITY Foundation [^1]: Contributor License Agreement

jsamol commented 1 year ago

The initial commit defines possible messages focusing on the content.

As already discussed in the last working group meeting, there are some concerns regarding the permission messages and whether they are needed in the IC ecosystem at all. This, however, requires further discussion as the initial conclusion was that such messages are redundant in the light of Internet Identity broad use, but some possible applications could be found.

jsamol commented 1 year ago

I've looked deeper into Internet Identity, and I'm not sure if it can be used to authorize requests to wallets as defined in RFP-7 🤔

Correct me if I'm wrong, but it seems like Internet Identity is the only source of its accounts, meaning a user can either create a new account/identity or import one that was previously created with Internet Identity. I can't see any options to import an account created in a wallet or loosely connect a wallet account to Internet Identity without revealing the passphrase used to create the account. This means that the only connection between Internet Identity and a wallet could be created only if the wallet imported the account from Internet Identity, which I don't think is possible in every case.

I believe that what we're defining here, through this wallet standard, is a secondary method of authentication in IC, an alternative to Internet Identity, where the source of the account can be any wallet that implements the standard. If that's the case, then the permission messages are indispensable as their main role is to obtain account related data from the wallet.

It would be great, though, to hear from someone with a broader knowledge of the whole IC ecosystem to validate my thoughts 😅

frederikrothenberger commented 1 year ago

I've looked deeper into Internet Identity, and I'm not sure if it can be used to authorize requests to wallets as defined in RFP-7 🤔

Correct me if I'm wrong, but it seems like Internet Identity is the only source of its accounts, meaning a user can either create a new account/identity or import one that was previously created with Internet Identity. I can't see any options to import an account created in a wallet or loosely connect a wallet account to Internet Identity without revealing the passphrase used to create the account. This means that the only connection between Internet Identity and a wallet could be created only if the wallet imported the account from Internet Identity, which I don't think is possible in every case.

Yes, Internet Identity is not a wallet (yet) but only an identity provider that issues delegations (which allow non-interactive calls using the delegation identity). As such, it currently is not possible to do transaction approval on a call-by-call basis using Internet Identity.

I believe that what we're defining here, through this wallet standard, is a secondary method of authentication in IC, an alternative to Internet Identity, where the source of the account can be any wallet that implements the standard. If that's the case, then the permission messages are indispensable as their main role is to obtain account related data from the wallet.

Yes, this is correct. I would not (yet) call the standard an alternative (as it has different goals and trade-offs) but rather a complement to it. The standards that we are defining here have transaction approval on a call-by-call basis as their core use-case. In the future, they can (and probably should) be extended to also incorporate additional use-cases such as issuing delegations. However, these extensions are definitely beyond the scope of RPF-7.

It would be great, though, to hear from someone with a broader knowledge of the whole IC ecosystem to validate my thoughts 😅

I'm the original author of the consent message spec and RFP-7. Now that I'm back from my PTO I'm happy to answer questions, do reviews, etc. I believe @marydwyer (planned to?) set up a shared slack channel as well... Feel free to reach out anytime. 🙂

jsamol commented 1 year ago

@frederikrothenberger I've updated the messages according to our discussion so far. I also added some technicalities not necessarily protocol specific, i.e. version and senderId. The version should allow us to seamlessly update the standard with new content in the future, and senderId is, well, the true identity of the source of the message that was discussed in one of the threads 😅 Additionally, I changed the form of the definitions from the original pseudocode to something more JSON-RPC suited.

One more thing regarding the certificate in the canister_call response. I've realized that the certificate returned by the read_state call is a CBOR encoded blob, not a raw object as I initially expected. Would it make sense, then, to change the type of the certificate field in the canister_call response to just blob?

frederikrothenberger commented 1 year ago

Thanks a lot @jsamol! I'll try to take a look before tomorrows session.

Would it make sense, then, to change the type of the certificate field in the canister_call response to just blob?

Yes, that is also an option. Given the complexity already required to verify the certificate, having it as a cbor blob would not make it much harder to deal with.

jsamol commented 1 year ago

@frederikrothenberger another draft update 😄

I've updated the title
I replaced the principal and ledger fields with one identity that holds, well, identity related details, including a DER-encoded publicKey + the crypto algorithm used to generate it (to make the challenge verification possible, as discussed) and an optional ledger (it felt like a good idea to group user-related data in the same object 🤔 )
the challenge field in the permission request has a proper definition of what it is expected to be
the certificate type in the canister_call response is blob
I've updated the use cases with more detailed descriptions of how to process the messages (how to verify the challenge and canister_call response - I hope I got this part right 🤞)
and finally, I've updated the examples and added a brief summary and terminology (since I use the same terminology as you do in ICRC-21, I took it from there)

sea-snake commented 12 months ago

including a DER-encoded publicKey + the crypto algorithm used to generate it (to make the challenge verification possible, as discussed)

The whole idea behind DER encoding is that it already includes the algorithm of the public key.

sea-snake commented 12 months ago

Here's how I verify challenges signed with either ECDSAKeyIdentity, Ed25519KeyIdentity or Secp256k1KeyIdentity with only the DER-encoded public key:

import { p256 } from '@noble/curves/p256';
import { secp256k1 } from '@noble/curves/secp256k1';
import * as ed from '@noble/ed25519';
import { sha512 } from '@noble/hashes/sha512';
import { sha256 } from '@noble/hashes/sha256';
import { ED25519_OID, unwrapDER } from '@dfinity/identity';
import { Buffer } from 'buffer';

ed.etc.sha512Sync = (...m) => sha512(ed.etc.concatBytes(...m));

const ECDSA_P256_OID = Uint8Array.from([
  ...[0x30, 0x13], // SEQUENCE
  ...[0x06, 0x07], // OID with 7 bytes
  ...[0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01], // OID ECDSA
  ...[0x06, 0x08], // OID with 8 bytes
  ...[0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07], // OID P-256
]);

const SECP256K1_OID = Uint8Array.from([
  ...[0x30, 0x10], // SEQUENCE
  ...[0x06, 0x07], // OID with 7 bytes
  ...[0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01], // OID ECDSA
  ...[0x06, 0x05], // OID with 5 bytes
  ...[0x2b, 0x81, 0x04, 0x00, 0x0a], // OID secp256k1
]);

export const verifyChallenge = async (
  publicKey: Uint8Array,
  signature: Uint8Array,
  data: Uint8Array,
): Promise<boolean> => {
  try {
    if (publicKey[0] !== 0x30 || publicKey[2] !== 0x30) {
      return false;
    }
    const oidSequenceLength = publicKey[3];
    const oid = publicKey.slice(2, oidSequenceLength + 4);
    if (oid.length !== oidSequenceLength + 2) {
      return false;
    }
    switch (Array.from(oid).join()) {
      case Array.from(ECDSA_P256_OID).join():
        return p256.verify(
          Buffer.from(signature).toString('hex'),
          Buffer.from(sha256(data)).toString('hex'),
          Buffer.from(unwrapDER(publicKey, ECDSA_P256_OID)).toString('hex'),
        );
      case Array.from(ED25519_OID).join():
        return ed.verify(
          Buffer.from(signature).toString('hex'),
          Buffer.from(data).toString('hex'),
          Buffer.from(unwrapDER(publicKey, ED25519_OID)).toString('hex'),
        );
      case Array.from(SECP256K1_OID).join():
        return secp256k1.verify(
          Buffer.from(signature).toString('hex'),
          Buffer.from(sha256(data)).toString('hex'),
          Buffer.from(unwrapDER(publicKey, SECP256K1_OID)).toString('hex'),
        );
      default:
        return false;
    }
  } catch (_) {
    return false;
  }
};

The DER-encoding actually standardizes the way the key and algorithm are communicated.

frederikrothenberger commented 12 months ago

The whole idea behind DER encoding is that it already includes the algorithm of the public key.

@sea-snake: Thanks! The latest version does no longer include the crypto algorithm separately exactly because of that. 👍

jsamol commented 12 months ago

Here's how I verify challenges signed with either ECDSAKeyIdentity, Ed25519KeyIdentity or Secp256k1KeyIdentity with only the DER-encoded public key: [...]

@sea-snake thanks for the snippet, it's very helpful! For the best development experience, however, I would love to see this functionality to be part of the @dfinity libraries, especially that there's already the PublicKey abstraction which could be extended with a factory function, which constructs a PublicKey implementation from a DER-encoded value, and with a verify method, which verifies the signature depending on the implementation.

sea-snake commented 12 months ago

@jsamol When I find the time, I can make an MR to make that happen. Not sure what the impact is on the package size, might need to be it's own lib.

sea-snake commented 12 months ago

The wallet continues to call read_state for the calculated request id until the status of the call indicates that the call has been processed (succesfully or not).

I don't see why the wallet should be making any requests and even poll for the result which can take some time. Can't the wallet just return the signed canister call and signed read state request so that the dapp can execute those however it intends?

This is basically how I've currently implemented things, the wallet is a popup that closes after returning a response over postMessage. Implementation: https://github.com/slide-computer/identity/blob/master/src/index.ts

The communication channels I'm currently using are opening a url in a popup/window and returning a postMessage (or opening a callback url for native apps). I can imagine standardizing that with this json rpc spec, the data transport layer can differ but that should not be an issue.

A challenge used for the wallet to sign in order to prove its access to the identity. The challenge should be an array of 32 cryptographically random bytes generated from a secure random source by the sender of the request.

If the wallet were to delegate, it could sign a delegation instead of a challenge, see example repo above.

Using delegations and having a rpc communication channel over url/postMessage/callback besides browser extensions that inject an rpc service, enables wallets that are hosted on the IC without a browser extension to function like NFID.

frederikrothenberger commented 12 months ago

The wallet continues to call read_state for the calculated request id until the status of the call indicates that the call has been processed (succesfully or not).

I don't see why the wallet should be making any requests and even poll for the result which can take some time. Can't the wallet just return the signed canister call and signed read state request so that the dapp can execute those however it intends? This is basically how I've currently implemented things, the wallet is a popup that closes after returning a response over postMessage.

This was the initial design. However, a DFINITY internal security review found that it would be safer to do it this way. The reason was the following attack scenario:

The canister call is not idempotent
The result of the canister call is not publicly observable

In that case a malicious relying party can request a transaction from the wallet. Given it is the relying party that submits the call, the wallet will not observe the result of the call. A malicious relying party could successfully submit the call but then show some made up error to the user tricking them into repeating the transaction. Having the wallet submit the call (and thus is able to observe the result) makes this type of attack more difficult to pull off (as the wallet would already show a success message to the user).

frederikrothenberger commented 12 months ago

A challenge used for the wallet to sign in order to prove its access to the identity. The challenge should be an array of 32 cryptographically random bytes generated from a secure random source by the sender of the request.

If the wallet were to delegate, it could sign a delegation instead of a challenge, see example repo above.

Using delegations and having a rpc communication channel over url/postMessage/callback besides browser extensions that inject an rpc service, enables wallets that are hosted on the IC without a browser extension to function like NFID.

I think there is a use-case to exchange delegation over such a channel using JSON rpc. However, I would make this an extension of this standard by extending it by introducing an additional scope (e.g. "delegation"), alongside with a delegation request and response.

The reason why I would split it into a separate standard is that the currently proposed feature set is save over any channel, whereas exchanging delegations usually requires an authenticated channel.

sea-snake commented 12 months ago

The canister call is not idempotent

I remember asking about this a few times before, as far as I've been told, you can't submit a canister call with the same request id more than once. So I don't see a security issue here.

The result of the canister call is not publicly observable That's a good point, sadly that means the UX for the end user isn't that great (waiting for a popup to close since the call is still ongoing).

sea-snake commented 12 months ago

The reason why I would split it into a separate standard is that the currently proposed feature set is save over any channel, whereas exchanging delegations usually requires an authenticated channel.

What do you mean with authenticated? As in data could be intercepted if it's not? A delegation itself isn't something that needs to be secured as long the as private key the delegation is created for isn't communicated over any channel (which it shouldn't in the first place).

Also, delegations are crucial for dapps that want to fetch private user specific information from their backend canisters with query calls.

frederikrothenberger commented 12 months ago

The reason why I would split it into a separate standard is that the currently proposed feature set is save over any channel, whereas exchanging delegations usually requires an authenticated channel.

What do you mean with authenticated? As in data could be intercepted if it's not? A delegation itself isn't something that needs to be secured as long the as private key the delegation is created for isn't communicated over any channel (which it shouldn't in the first place).

Authenticated as in the sender of the request can be validated. For window post messages this is the case as the message origin is set by the browser. This means evil.com cannot request delegations on behalf of nice-dapp.com. All security models around delegations that I have seen so far require this property, because either the delegation principal or the target list is dependent on the sender of the delegation request.

frederikrothenberger commented 12 months ago

That's a good point, sadly that means the UX for the end user isn't that great (waiting for a popup to close since the call is still ongoing).

I don't think it is that bad. If the user has to look at a spinner for 2 seconds, that's still very manageable. But I see the point. However, for transaction approval I would really err on the side of security / caution.

sea-snake commented 12 months ago

All security models around delegations that I have seen so far require this property, because either the delegation principal or the target list is dependent on the sender of the delegation request.

Yeah that's why I require a public key in the initial connection that looks at the origin. But subsequent calls can be made by signing a challenge over the request id, the origin doesn't need to be checked anymore. So that the dapp can for example connect at http://account.dapp.com while making calls from http://market.dapp.com by simply sharing the private key with it's own domains.

sea-snake commented 12 months ago

Upon receiving the request, the wallet validates whether the relying party has the permission to request the action and processes the message following the [ICRC-21]() specification.

This would limit the dapp from only being able to make calls to canisters that have this implemented. Is it fair to assume this is optional, the wallet can also decide to decode the canister call arguments and show these to the user? Currently I'm doing this:

ICRC-21 would definitely be helpful for calls that aren't standardized like ICRC1 calls where the wallet cannot infer the intent from the method and args.

sea-snake commented 12 months ago

I'll implement an Agent following this spec and link it here :D

frederikrothenberger commented 12 months ago

Upon receiving the request, the wallet validates whether the relying party has the permission to request the action and processes the message following the ICRC-21 specification.

This would limit the dapp from only being able to make calls to canisters that have this implemented. Is it fair to assume this is optional, the wallet can also decide to decode the canister call arguments and show these to the user?

Yes, of course. However, it is recommended that some sort of warning is shown to the user to make them aware that this target does not fully support transaction approval.

frederikrothenberger commented 12 months ago

All security models around delegations that I have seen so far require this property, because either the delegation principal or the target list is dependent on the sender of the delegation request.

Yeah that's why I require a public key in the initial connection that looks at the origin. But subsequent calls can be made by signing a challenge over the request id, the origin doesn't need to be checked anymore. So that the dapp can for example connect at http://account.dapp.com while making calls from http://market.dapp.com by simply sharing the private key with it's own domains.

Ah, I see. I think this model of sharing the "sender identity" with another domain could be an extension too, right? I'd like to keep the standards small & focused in order to not burden devs too much that don't need the full functionality. Or do you see a fundamental issue with ICRC-25 that would break with what you are trying to do?

sea-snake commented 12 months ago

All security models around delegations that I have seen so far require this property, because either the delegation principal or the target list is dependent on the sender of the delegation request.

Yeah that's why I require a public key in the initial connection that looks at the origin. But subsequent calls can be made by signing a challenge over the request id, the origin doesn't need to be checked anymore. So that the dapp can for example connect at http://account.dapp.com while making calls from http://market.dapp.com by simply sharing the private key with it's own domains.

Ah, I see. I think this model of sharing the "sender identity" with another domain could be an extension too, right? I'd like to keep the standards small & focused in order to not burden devs too much that don't need the full functionality. Or do you see a fundamental issue with ICRC-25 that would break with what you are trying to do?

I think making it part of the spec to send a challenge (e.g. signed request id in a canister call) would make the spec compatible with data channels that can't always communicate the origin like native <-> browser. Making only the initial connection require the data channel to be something that communicates the origin like a browser, would make it for a dapp developer of a native app only required to open a special dapp specific proxy page in a browser once for the connection request but not for every subsequent canister call request.

But indeed, this could still be an extension of the spec, where an additional pubkey is sent in the initial request and in subsequent canister call requests an additional signed challenge is sent to let the wallet know it can verify the challenge instead of the origin.

I'll add it to the example Agent implementation as optional field.

On another note, after reading both specs in more detail, I'm happy with the decisions made and glad to see the work that's put into it :D

sea-snake commented 12 months ago

I've ran into an issue while I was implementing an Agent for the spec.

The relying party retrieves the publicKey from identity, determines the signature scheme and verifies whether it was generated by signing the concatenation of the domain separator \x0Aic-wallet-challenge (UTF-8 encoded) and the challenge from the request with the private key associated with the publicKey.

This isn't possible if the wallet doesn't have access to the private key of the identity, which actually is the case with Internet Identity and wallets that rely on delegations to secure their private key behind biometrics/password/non-extractable-browser-key. Both is the case for the wallet I'm currently implementing, and there's no way around it 😅

Why does the relying party need to verify that the wallet has the private key for the public key? Only reason I can think of would be to verify it for e.g. user registration, but even then I would probably rely on a canister call instead.

One workaround I could think of would be to also return a delegation besides the signed challenge. If a delegation is returned, the challenge can be verified with the public key at the end of the delegation chain and then the delegation chain itself can be validated to be valid & start with the identity public key.

Adding an optional delegation chain to the identity object in the response would make above doable, though it makes things more complex, it also makes it possible to work with identities beyond the ones that directly come from a seed phrase.

frederikrothenberger commented 12 months ago

Why does the relying party need to verify that the wallet has the private key for the public key? Only reason I can think of would be to verify it for e.g. user registration, but even then I would probably rely on a canister call instead.

The main reason is to prevent wallets being able to claim control of a key they don't actually have access to. I expect quite a few applications building a poor mans sign-in flow where the signed challenge serves as some sort of session token. Having the challenge response flow at least makes sure that the wallet cannot lie about the keys it controls.

Having the option to provide a delegation chain seems sensible to me. While it does add complexity, it is also a largely solved problem (there are libraries for that) and consistent with the ecosystem.

What do you think?

PS: It would be really nice if you could join the working group session on September 5 to discuss your findings and feedback. 🙂

sea-snake commented 12 months ago

The main reason is to prevent wallets being able to claim control of a key they don't actually have access to. I expect quite a few applications building a poor mans sign-in flow where the signed challenge serves as some sort of session token. Having the challenge response flow at least makes sure that the wallet cannot lie about the keys it controls.

That's not something I would ever consider but totally something that I would see happen indeed, especially with web2 sites that use a web3 wallet.

Having the option to provide a delegation chain seems sensible to me. While it does add complexity, it is also a largely solved problem (there are libraries for that) and consistent with the ecosystem.

What do you think?

I'll add the delegation_chain as an optional response value within the identity object in my implementation.

PS: It would be really nice if you could join the working group session on September 3 to discuss your findings and feedback. 🙂

I'll be there.

sea-snake commented 12 months ago

@frederikrothenberger In the @dfinity/agent lib the delegation chain is converted to JSON in a different way than this spec, public key blobs are encoded as hex instead of base64 and targets (principals) are also encoded as hex instead base32 dash separated string.

https://github.com/dfinity/agent-js/blob/main/packages/identity/src/identity/delegation.ts#L48

So should I ignore that and follow the encodings in this spec? Probably makes sense anyway since the public key within the delegation chain is redundant (already defined within identity).

frederikrothenberger commented 12 months ago

@frederikrothenberger In the @dfinity/agent lib the delegation chain is converted to JSON in a different way than this spec, public key blobs are encoded as hex instead of base64 and targets (principals) are also encoded as hex instead base32 dash separated string.

https://github.com/dfinity/agent-js/blob/main/packages/identity/src/identity/delegation.ts#L48

So should I ignore that and follow the encodings in this spec? Probably makes sense anyway since the public key within the delegation chain is redundant (already defined within identity).

Hm, that's true. Yeah, it is probably better to be consistent with existing code. Let's pick this up as a discussion point in the next session...

sea-snake commented 12 months ago

Took a bit longer than expected, but here's the WalletAgent implementation that follows this spec: https://github.com/slide-computer/identity/blob/master/src/index.ts

(Ignore the git repository name that says "identity", this was pushed into a WIP repo)

It's definitely a bit of a chaotic code trying to wrap things within the constraints of the Agent interface, while trying to support the pollForResponse implementation, upgrading query requests to calls and adding support for delegation chains.

This agent should be interchangeable with HttpAgent when you create an Actor.

As for the data transport, this is a configurable option when you create the agent, the following transports have been implemented: LinkTransport, PostMessageTransport, PromiseTransport and MixedTransport and more implementation could be added

Maybe a standard that lets you lookup the recommended transport and its options for a specific wallet and device environment, could simplify things here further into something like a LookupTransport implementation that only requires the wallet url to get things working.

Example usage:

// Links and post message are used here together as transport with `MixedTransport`
const walletOrigin = 'https://example.wallet.com';
const transport = new MixedTransport({
    incoming: new PostMessageTransport({
        origin: walletOrigin
    }),
    outgoing: new LinkTransport({
        origin: walletOrigin,
        open: (link: string) => {
            window.open(link, 'exampleWalletWindow');
        }
    })
});
const agent = new WalletAgent({ transport });
const result = await agent.permission({
    version: 1,
    appMetaData: {
        name: 'Example Dapp',
    },
    scopes: ['canister_call'],
});

// Required: pick the identity to use for the canister calls and set it as sender
agent.sender = Principal.selfAuthenticating(result.identities[0].publicKey.toDer());

// Now you can make calls with the wallet like you normally would with HttpAgent
const ledgerActor = createLedgerActor(agent);
const txId = await ledgerActor.transfer({ to: johnDoe, amount: 99999999});

These transports can also be used standalone from the wallet side to receive incoming RPC calls.

frederikrothenberger commented 12 months ago

@sea-snake: That's fantastic! 😄 Do you have a deployment of your code somewhere, so that I can try it out in a live example?

Maybe a standard that lets you lookup the recommended transport and its options for a specific wallet and device environment, could simplify things here further into something like a LookupTransport implementation that only requires the wallet url to get things working.

This is absolutely something we should work on once the standard has been established. As I see it, the next steps are as follows:

Standardize ICRC-21 and ICRC-25
Outline possible extensions / further work. Current candidates are:
- Standardize transports & transport selection
- Support exchanging delegations by introducing the delegation scope (and another request / response pair)
- Extend ICRC-25 to support batch approvals
- Standardize target canister metadata (i.e. is the call going to a well-known target like the ICP ledger canister or not)
Work on extensions

sea-snake commented 12 months ago

Do you have a deployment of your code somewhere, so that I can try it out in a live example?

Working on that :D

frederikrothenberger commented 12 months ago

PS: It would be really nice if you could join the working group session on September 3 to discuss your findings and feedback. 🙂

I'll be there.

@sea-snake: I just realized that I mistyped the date of the working group session. It is on September 5. Sorry about that.

sea-snake commented 12 months ago

PS: It would be really nice if you could join the working group session on September 3 to discuss your findings and feedback. 🙂

I'll be there.

@sea-snake: I just realized that I mistyped the date of the working group session. It is on September 5. Sorry about that.

That's fine, I've got the whole DFINITY agenda here to see all events and WG meetings.

neeboo commented 11 months ago

Do you have a deployment of your code somewhere, so that I can try it out in a live example?

Working on that :D

Please provide benchmark btw :D

frederikrothenberger commented 11 months ago

Do you have a deployment of your code somewhere, so that I can try it out in a live example?

Working on that :D

Please provide benchmark btw :D

@neeboo: could you please clarify what metrics you are interested in?

neeboo commented 11 months ago

Do you have a deployment of your code somewhere, so that I can try it out in a live example?

Working on that :D

Please provide benchmark btw :D

@neeboo: could you please clarify what metrics you are interested in?

mainly latency.

in the code, agent.permission is an await, and ledger.transfer is another.

And also bulk transfer is something dapp devs really want, would it be considered as well?

frederikrothenberger commented 11 months ago

And also bulk transfer is something dapp devs really want, would it be considered as well?

Yes, batching / bulk transfer is probably a natural extension of this standard. See my comment here.

frederikrothenberger commented 11 months ago

mainly latency.

in the code, agent.permission is an await, and ledger.transfer is another.

Note that in both of these interactions a user approval is required. It is generally expected that the user interaction with the wallet will make up the bulk of any latency measurement.

neeboo commented 11 months ago

mainly latency. in the code, agent.permission is an await, and ledger.transfer is another.

Note that in both of these interactions a user approval is required. It is generally expected that the user interaction with the wallet will make up the bulk of any latency measurement.

yep, i think it really depends on wallet to handle interface and etc.

sea-snake commented 11 months ago

Do you have a deployment of your code somewhere, so that I can try it out in a live example?

Working on that :D

Please provide benchmark btw :D

@neeboo: could you please clarify what metrics you are interested in?

mainly latency.

in the code, agent.permission is an await, and ledger.transfer is another.

And also bulk transfer is something dapp devs really want, would it be considered as well?

Permission is a one time operation, the response data can be stored in the browser so the dapp knows it already has permission and reuses this data until it receives a no permissions error when the user manually revoked the permission in the wallet.

Bulk is primarily useful for multiple operations that the user needs to approve towards ledgers and other dapps.

More common are bulk calls from a dapp frontent to the dapp backend canister(s), with delegations you can avoid going through the wallet altogether and directly make those calls from the dapp frontend.

neeboo commented 11 months ago

Do you have a deployment of your code somewhere, so that I can try it out in a live example?

Working on that :D

Please provide benchmark btw :D

@neeboo: could you please clarify what metrics you are interested in?

mainly latency. in the code, agent.permission is an await, and ledger.transfer is another. And also bulk transfer is something dapp devs really want, would it be considered as well?

Permission is a one time operation, the response data can be stored in the browser so the dapp knows it already has permission and reuses this data until it receives a no permissions error when the user manually revoked the permission in the wallet.

Bulk is primarily useful for multiple operations that the user needs to approve towards ledgers and other dapps.

More common are bulk calls from a dapp frontent to the dapp backend canister(s), with delegations you can avoid going through the wallet altogether and directly make those calls from the dapp frontend.

Thanks for the reply. Personally, I really want to see how UI handles those messages and make permission smoothly. Can't wait to see your example.

sea-snake commented 11 months ago

Based on forum discussion there's basically one main point of discussion I would like to bring up, the choice of JSON RPC for the spec.

As I mentioned in a forum post, maybe Candid IDL makes more sense to be platform agnostic and then a spec could be defined for Candid -> JSON RPC for data transport channels where it makes more sense then Candid. This would allow this spec or an extension of this spec to also apply for e.g. web/canister to wallet canister messages.

sea-snake commented 11 months ago

Here's a working demo: https://osu4t-5aaaa-aaaak-ae5za-cai.icp0.io

Make sure to create a wallet first at: https://etk52-fqaaa-aaaak-ae4ca-cai.icp0.io

Edit: Fixed mistake with verifying ed25519 signatures, demo should work also for mnemonic phrase now.

The wallet and this demo are still WIP:

The wallet UI still says "Signature request" and "Sign" even though we're sending the request from the wallet as defined in the spec
Error handling is not fully implemented, it's basically missing in the demo
Make sure to be signed in to the wallet, if you're not, things will break in this demo
Secure signing and locking settings do not work yet in this demo flow
The wallet does not remember the permission, you'll have to reconnect every time (not implemented yet)
Delegations work but is currently hardcoded in the wallet to a cowsay canister for the purpose of this demo
Internet Identity works in the wallet and this demo, but if it expires, the wallet won't let you know yet 😅

Functionality in this demo:

Connect to wallet by permission request
Transfer ICP by canister_call request
Query cowsay canister by delegation (no interaction or communication with wallet required)

Data transports used in this demo:

LinkTransport to open popup window for outgoing messages
PostMessageTransport to receive incoming messages from popup window

jsamol commented 11 months ago

PoC (up to date with the standard definition at a03d16d6400abc18666995009785e56cd1235c52):

DApp: https://weutz-riaaa-aaaao-a2hna-cai.icp0.io/
Wallet: https://wdvvn-4qaaa-aaaao-a2hnq-cai.icp0.io/
ICRC-21 canister: https://dashboard.internetcomputer.org/canister/xhy27-fqaaa-aaaao-a2hlq-cai

frederikrothenberger commented 11 months ago

The PR should be merged so that the draft can be worked on in smaller, focused PRs.

dfinity / wg-identity-authentication

airgap/dapp-wallet-messages #32