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

Increase account_address and asset_reference max length #179

Merged
merged 4 commits into from
Feb 1, 2023

Conversation

delaaxe
Copy link
Contributor

@delaaxe delaaxe commented Nov 18, 2022

Many chains naturally use 32 bytes to represent addresses, which translates to 64 characters.

However, it's also common to prefix addresses with 0x, increasing the character length by two and going over this limit.

To avoid issues between dapps and wallets having to handle removing or adding this prefix, this proposal increases the max length 128 to accommodate more chains without introducing error-prone string manipulations in both dapps and wallets.

@ligi
Copy link
Member

ligi commented Nov 18, 2022

sounds reasonable - just out of curiosity - which chain does use 32 bytes?
Most chains I know (ethereum compatible use 20 bytes)

@delaaxe
Copy link
Contributor Author

delaaxe commented Nov 18, 2022

StarkNet uses 32 bytes, so their 0x-prefixed addresses have a length of 66

@ritave
Copy link
Contributor

ritave commented Nov 18, 2022

Is an Account ID with 0x prefix the same account without the 0x prefix? Should we normalize addresses instead?

@delaaxe
Copy link
Contributor Author

delaaxe commented Nov 18, 2022

Not sure what you mean. On Ethereum addresses are always prefixed with 0x and that is what's used in the CAIP for EIP-155. It's similar on StarkNet, except longer

@oed
Copy link
Collaborator

oed commented Nov 19, 2022

@pedrouid why was there a limit set here in the first place?

Should we normalize addresses instead?

@ritave it's up to each namespace to decide how to encode addresses.

@pedrouid
Copy link
Member

What about CAIP-19 Asset ID? @delaaxe

Will Starknet also need it to be expanded to 66?

@delaaxe
Copy link
Contributor Author

delaaxe commented Nov 28, 2022

Yes this PR was made for StarkNet

@pedrouid
Copy link
Member

pedrouid commented Dec 1, 2022

Sorry I wasn't explicit.

I meant if Starknet would also need AssetId (CAIP-19) to be increased to 66 chars

How do you currently represent Assets in StarkNet?

@pedrouid
Copy link
Member

pedrouid commented Dec 2, 2022

Hey @delaaxe I've taken into consideration this PR and also asked for some second opinions

Increasing this limit for a single namespace doesn't seem reasonable because it would affect the usage of CAIP-10

Also the PR #180 was a blocker because your ChainID's needed "_" to properly function

But this PR is not as critical as you can adapt your addresses by sanitizing the hex prefix (0x)

I would like to ask you and the StarkNet team to update your namespace PR to remove the hex prefix so that it would fit within the 64 char limit

Hope you understand my reasoning

@delaaxe delaaxe changed the title Increase max account_address to 66 Increase max account_address length Dec 2, 2022
@delaaxe
Copy link
Contributor Author

delaaxe commented Dec 2, 2022

Hi @pedrouid thanks for your response, I've resolved the merge conflict.

Hope you understand my reasoning

I don't understand your reasoning. To avoid making it appear like you're favoring a particular chain too much, I've increased the max length to 128 to avoid future such requests. As someone asked earlier, why is there such a strict limitation in the first place?

We're building wallets on iOS, Android and web as well as dapps. Asking all wallet and dapp devs to do unneeded string manipulations can only be error prone and the source of bugs and headaches.

Regarding StarkNet, it also has a decimal representation of addresses which are much longer.

As you asked about NFTs, those may also have much longer IDs too.

Finally, one of the goals of this format is to keep it user-readable and machine-searchable, and I believe keeping the canonical representations in the spec without artificial transformations is important.

I stay available if more clarifications are needed.

@bumblefudge
Copy link
Collaborator

bumblefudge commented Dec 2, 2022

hey there, namespaces editor here, let's see how I can help.

I believe keeping the canonical representations in the spec without artificial transformations is important

I'm not sure ^this is accurate-- most CAIP-2 profiles in the namespaces are NOT the canonical representations, but rather transformations. Namely, many CAIP-2s are 4- or 8-character substrings of the genesis block hash, which are far less helpful than the canonical full hash. This has been done historically to avoid size creep and keep compound URNs like CAIP-19 strings below 256 characters to avoid them getting truncated in some contexts. Arbitrary size limits on CAIP-2 and CAIP-10 are partial mitigations to keep CAIP-19 (and perhaps, some day, URLs with query parameters, my personal hobbyhorse) manageable. This is not an unalloyed good or even an explicit design goal of the CAIPs project as a whole, however, so we can definitely keep an open mind about counterveiling design goals per namespace.

Perhaps it would be helpful to flesh out the draft PR in namespaces to help me get situated in the problem space? In particular, any help explaining the decimal representations (and whether they can be deterministically roundtripped to the shorter form in particular) would help a lot in designing a namespace that minimizes or avoids changes to the rest of the system and breaks existing tooling and libraries. Over the next few weeks, I can edit that content into a full namespace and/or into ### Starknet sections eip155 CAIP-2, -10, and -19 sections as needed. But a little help goes a long way in making this a manageable amount of research for me!

@silverpill
Copy link

Monero uses 77-byte addresses (106 characters in base58 encoding): https://monerodocs.org/public-address/integrated-address/

Zcash shielded addresses are ~77 characters long: https://zcash.readthedocs.io/en/latest/rtd_pages/addresses.html. New unified addresses are longer (~106 characters).

I think we can expect that future private chains will have long addresses as well, so increasing character limit to 128 seems quite reasonable to me.

@kdenhartog
Copy link
Contributor

Since we're already having to adapt the size limitation here does it make sense to account for PQC algorithms that have been selected by NIST?

The three selected signing algorithms chosen with their largest key sizes are:

crystals-dilithium: 1760 bytes
falcon-1024: 1793 bytes
sphincs+: 64 bytes

@bumblefudge
Copy link
Collaborator

crystals-dilithium: 1760 bytes
falcon-1024: 1793 bytes

Seems a bit like a slippery slope to me-- or to put it another way, 128 feels as arbitrary as 64. Since we've got namespaces (of various lengths), chainIds (of various lengths) and 3 :s in the mix, maybe we should make more explicit what a reasonable functional limit is for the entire CAIP-10/19 and work backwards from there? How many characters are we leaving for query parameters? If 256 or 255 is the realistic limit for many of these strings to function as tokens, we're going to run into another limit when the longer account_address namespaces find themselves without enough characters left for query params (i.e. CAIP URLs), when the collision risk of substringing or sanitizing native/canonical account_address was not particularly high.

Another question-- is anyone interested in working on a Monero or ZCash namespace with me? 🥺👉👈

@oed
Copy link
Collaborator

oed commented Dec 5, 2022

ZCash namespace

This might fall within the bip122 namespace?

@bumblefudge
Copy link
Collaborator

This might fall within the bip122 namespace?

Good point-- I've always heard it referred to as a BTC fork, but it seems the crypto is relatively different and the addresses are different, so I could see it making sense either way. Ultimately it's a line in the sand between enough shared assumptions & tooling to be considered a variant versus different enough assumptions & tooling to justify a distinct namespace. Would have to dig into how diff the RPC commands are, assumptions about ChainID/forks, etc. to even have an opinion, so happy to defer to the group.

In any case who volunteering to help me write it? @paritytech seems to have written this, maybe someone at (or a grantee of) web3 foundation would come for the polkadot and stay for the zcash?

@bumblefudge
Copy link
Collaborator

(side note: zcash Z-addresses, which would be worth making representable in CAIP-10 form, are 78 characters long, and derived as such)

@silverpill
Copy link

@bumblefudge

Another question-- is anyone interested in working on a Monero or ZCash namespace with me? pleading_facepoint_rightpoint_left

I don't know much about Zcash ecosystem but I can help with Monero namespace. I already have gathered some relevant info so l'll create an issue in namespaces repo

@pedrouid
Copy link
Member

pedrouid commented Dec 7, 2022

At least there are now arguments to increase the char length for multiple namespaces:

  • StarkNet
  • Zcash
  • Monero

This completely changes the narrative of the PR IMO

@bumblefudge
Copy link
Collaborator

128 is making more sense given all these systems with long keys we're hearing about. As a privacy buff, I'm hoping at least one of these namespaces can get merged in Q1!

Kyle's examples of the mammoth postquantum keys (which came up on another standards call yesterday as a viable signing method for ZK-VCs!) also give me pause, tho-- if there has been an unwritten assumption until now that an entire CAIP-10/19 needs to fit under 256 characters for contexts where they're passed around as ASCII strings (e.g. API headers), it might be worth writing out that assumption as a SHOULD somewhere in the texts of CAIPs 10 and 19, if only as a non-normative reminder to consider it.

@pedrouid
Copy link
Member

pedrouid commented Dec 7, 2022

Also we should include this PR increasing CAIP-19 AssetId to also include 128 chars

@delaaxe delaaxe changed the title Increase max account_address length Increase account_address and asset_reference max length Dec 7, 2022
@delaaxe
Copy link
Contributor Author

delaaxe commented Dec 7, 2022

Also we should include this PR increasing CAIP-19 AssetId to also include 128 chars

done

@kdenhartog
Copy link
Contributor

128 feels like a reasonable max to me. SPHINCS+ could be used with this as well, but I doubt many chains will take that approach given the signature sizes are roughly 49KB each which would be a huge tradeoff there.

Realistically, I suspect most chains will go towards the FALCON for more efficient signature output sizes and faster verification times and the hardness problem it relies on (NTRU hardness problem) has been around quite awhile. With that said, we can update when we have a better understanding of chains that are actually heading in this direction rather than just astronaut architecting it IMO.

@bumblefudge bumblefudge added the 14-day merge hoping to merge in 14days or sooner if rough consensus in relevant subgroup label Jan 11, 2023
@bumblefudge bumblefudge requested a review from oed January 12, 2023 16:07
@bumblefudge
Copy link
Collaborator

Double-checking with @oed and @pedrouid that CAIP-2 underscore is right 🍡

@sneurlax
Copy link

sneurlax commented Jan 23, 2023

Monero's next major hard fork will be to a new address scheme which will be over 128 characters, more like 200

@silverpill
Copy link

silverpill commented Jan 26, 2023

A hard limit can be replaced with a soft limit. For example, account_address / asset_reference can look like this:

[-.%a-zA-Z0-9]{1,}

Then specification may say something like it is recommended to keep the length of asset_reference under 128 characters if it is intended for use in <describe situation where limit makes sense>

@bumblefudge
Copy link
Collaborator

TBH @silverpill that's been discussed in the past and rejected because hard limits are how existing implementations use the CAIPs, but I will discuss at next editorial meeting and take a temperature reading. I'm merging this for now but will open an issue to track and keep you posted there on where consensus is headed wrt to this

@bumblefudge bumblefudge merged commit 1263b57 into ChainAgnostic:master Feb 1, 2023
ntn-x2 added a commit to KILTprotocol/kilt-node that referenced this pull request Feb 22, 2023
Fixes KILTprotocol/ticket#2458.

- CAIP-19 asset reference max length was increased to 128:
ChainAgnostic/CAIPs#179
- CAIP-2 chain reference now supports underscores:
ChainAgnostic/CAIPs#180
- CAIP-19 asset reference and asset identifier now support `.` and `%`
symbols: ChainAgnostic/CAIPs#160

Not a breaking change since it just relaxes some requirements.
webguru9178 pushed a commit to webguru9178/kilt-node that referenced this pull request Jan 8, 2024
Fixes KILTprotocol/ticket#2458.

- CAIP-19 asset reference max length was increased to 128:
ChainAgnostic/CAIPs#179
- CAIP-2 chain reference now supports underscores:
ChainAgnostic/CAIPs#180
- CAIP-19 asset reference and asset identifier now support `.` and `%`
symbols: ChainAgnostic/CAIPs#160

Not a breaking change since it just relaxes some requirements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
14-day merge hoping to merge in 14days or sooner if rough consensus in relevant subgroup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants