CAIPs icon indicating copy to clipboard operation
CAIPs copied to clipboard

Implementer Feedback: CAIP-122<>EIP-4361 mismatch

Open bumblefudge opened this issue 1 year ago • 12 comments

resources is optional in EIP-4361 and required in CAIP-122 (although nothing stops it from being present but empty). both are still in review, but weight-bearing. presumably the path of least resistance/breakage is to keep it as-is, but add non-normative guidance warning people about this possibility (i.e. make "present but empty" explicitly allowed by 4361 to make sure implementers cover that case).

bumblefudge avatar Feb 20 '24 17:02 bumblefudge

As of now, just checked some stuff, some kind of PR on CAIP-122 is probably necessary. pinging @ukstv to see if the message format in #236 was implemented by ceramic, not seeing it reflected here which makes me wonder if we need an alignment [meeting]

property EIP CAIP in CAIP spec examples?
scheme opt n/a no
domain req req yes
address req req yes
statement opt opt yes
uri req req yes
version req req yes
chain-id req n/a - see PR#236 no
nonce req optional yes
issued-at req optional yes
expiration-time opt opt no
not-before opt opt no
request-id opt opt no
resources opt opt yes

bumblefudge avatar Feb 20 '24 19:02 bumblefudge

I will link this here as your table also mentions the attribute "address": https://github.com/ChainAgnostic/CAIPs/issues/266

jdsika avatar Feb 28 '24 12:02 jdsika

I think #236 does not make much sense as the CAIP-10 clearly defines an account ID including the namespace and chain ID as prefix. This makes the chain ID obsolete and I think the account ID is a good solution and well readable

jdsika avatar Feb 28 '24 12:02 jdsika

Maybe I am just confused why it is split up....? And why is it not also: blockchain == ${namespace(address)}

jdsika avatar Feb 28 '24 13:02 jdsika

@oed @haardikk21 @chris13524 you guys have opinions on reconciling these? i'm happy to open a PR and get a review from @jdsika off the top of my head but I've never implemented SIWX and have no idea what i'd be breaking!

bumblefudge avatar Mar 22 '24 22:03 bumblefudge

@oed @haardikk21 you guys have opinions on reconciling these? i'm happy to open a PR and get a review from @jdsika off the top of my head but I've never implemented SIWX and have no idea what i'd be breaking!

Also paging @0xproflupin for any thoughts here as well as it relates to SIWS compatibility.

obstropolos avatar Mar 22 '24 22:03 obstropolos

So I am the "fresh pair of eyes" on the whole thing but I may not know the backstory to some decisions. Suming up some things I think are actually inconsistencies: #266

  • rename address to account-id
  • Styleguide: give conventions if snakecase camlcase etc should be used etc. CAIP-10: account_id and CAIP-122: account-id
  • replace blockchain with ${namespace(account-id)}

Important as I realized while doing tezos-caip-122:

  • Depending on the cryptographic curve and hashing function to derive the pkh from the public key it is NOT possible to get the public key from the pkh
  • In Ethereum it apparently is but in Tezos the wallet interaction TZIP defines a handshake to provide the public key from wallet to the application
  • Recommendation: Introduce a public-key in the CAIP-122 data model as optional and/or clearly define the need to provide this information to validate the signature
  • Give a recommendation to define the signature enums like $namespace as prefix + : + some curve specific suffix

jdsika avatar Mar 25 '24 18:03 jdsika

Hi guys, I found a couple of typos in the spec:

  • The Chain ID is not presented in the following table: image

  • The above table says the address must be CAIP-10 compliant but this is not shown in the Ethereum or Solana examples: image

  • I'm not familiar with Solana tbh, but I see it's example has Chain ID: 1 is this correct for Solana?


Joining a little bit the discussion here:

I think https://github.com/ChainAgnostic/CAIPs/pull/236 does not make much sense as the CAIP-10 clearly defines an account ID including the namespace and chain ID as prefix. This makes the chain ID obsolete and I think the account ID is a good solution and well readable

This seems like a conflict to me between making the protocol "efficient" against avoiding deprecating EIP-4361, seems like there's intention to avoid breaking changes in Ethereum. I think if we think of introducing breaking changes into EIP-4361 then would the following statement from the spec be correct?

This work is meant to generalize and abstract the Sign in With Ethereum specification, thereby making EIP-4361 a specific implementation of this specification, to work with all blockchains.

glitch-txs avatar Apr 04 '24 02:04 glitch-txs

This seems like a conflict to me between making the protocol "efficient" against avoiding deprecating EIP-4361, seems like there's intention to avoid breaking changes in Ethereum. I think if we think of introducing breaking changes into EIP-4361 then would the following statement from the spec be correct?

This work is meant to generalize and abstract the Sign in With Ethereum specification, thereby making EIP-4361 a specific implementation of this specification, to work with all blockchains.

Agreed, I added a commit wordsmithing this here - reapprove if that works for you?

I'm not familiar with Solana tbh, but I see it's example has Chain ID: 1 is this correct for Solana?

Technically, this would be a nonconformant message, but since the EIP-4361 examples and tutorials and SDKs out there in the wild all use chainId: 1 for mainnet, and the Solana environment doesn't have a native numeric chainId system (they organize networks more by stable RPC endpoints, as many L1s do), a Solana dev might be "forgiven", à la Postel's Law, for assuming that 1, and not 5eykt4UsFv8P8NJdTREpY1vzqKqZKvdpKuc147dw2N9d, is the CAIP-2 identifier for Solana mainnet. It's up to the implementation if they want to accept reasonable "aliases" for chainId or be strict in the Postel sense-- they're conformant with the spec either way. There is also an example of "aliases" in the recently-merged tezos CAIP-2 profile, on the subject of aliases. Note that the EIP 4361 original gives an ABNF for translating to user-readable text deterministically; showing an end-user a prompt that reads ChainId: 5eykt4UsFv8P8NJdTREpY1vzqKqZKvdpKuc147dw2N9d is a little cryptic, but the authors of EIP-4361 decided that the onus of translating chainIds to user-recognizable names was a UX extension EIP for a later day and the wallet's obligation/option in the meantime.

bumblefudge avatar Apr 04 '24 03:04 bumblefudge

I see, in the Solana case would the use of 5eykt4UsFv8P8NJdTREpY1vzqKqZKvdpKuc147dw2N9d be valid as a Chain ID parameter?

glitch-txs avatar Apr 04 '24 03:04 glitch-txs

valid yes; user-friendly, not so much. Designating official aliases could be done in a PR to solana/caip2.md in the namespaces project, if it had demonstrable support from the Solana community. Presumably "1" or "mainnet" would be the official alias?

bumblefudge avatar Apr 04 '24 03:04 bumblefudge

user-friendly, not so much.

In the case Chain ID was optional and CAIP-10 compliance required for address, devs would still be required to use 5eykt4UsFv8P8NJdTREpY1vzqKqZKvdpKuc147dw2N9d right?

Also "official aliases" seems to have a different description/meaning compared to "Chain ID"

Or do you mean to replace it in CAIP-2?

glitch-txs avatar Apr 04 '24 03:04 glitch-txs