-
Notifications
You must be signed in to change notification settings - Fork 814
Update ERC-7930: Move to Review #1265
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
base: master
Are you sure you want to change the base?
Conversation
…munity feedback and discussions within the Interop working group.
File
|
0xrcinus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small notes (one I think is just formatting that tripped CI). Thanks for these updates
ERCS/erc-7930.md
Outdated
| - Any future Interoperable Address will be trivially convertible to Interoperable Address v1 (specified below), which, together with the point below, makes them a canonical representation for users who need to treat them opaquely. | ||
| - Two Interoperable Addresses, converted to this canonical version, will be bitwise-equal if and only if they represent the same _target address_. | ||
| - Checksums are short enough to be compared by humans, and allow for easy differentiation of _target addresses_ independently of any extra data the Interoperable Address may contain. | ||
| ## Comparisons with other standards |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## Comparisons with other standards | |
| ### Comparisons with other standards |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
ERCS/erc-7930.md
Outdated
| ## Comparisons with other standards | ||
|
|
||
| ### Comparisons with other standards | ||
| ### CAIP-10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ### CAIP-10 | |
| #### CAIP-10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
| [ERC-7785]: ./eip-7785.md | ||
| [CAIP-2]: https://github.com/ChainAgnostic/CAIPs/blob/2a7d42aebaffa42d1017c702974395ff5c1b3636/CAIPs/caip-2.md | ||
| [CAIP-10]: https://github.com/ChainAgnostic/CAIPs/blob/2a7d42aebaffa42d1017c702974395ff5c1b3636/CAIPs/caip-10.md | ||
| [CAIP-50]: https://github.com/ChainAgnostic/CAIPs/blob/2a7d42aebaffa42d1017c702974395ff5c1b3636/CAIPs/caip-50.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add CAIP-350 (referenced in the text)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 CAIP-350 is linked.
0xteddybear
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for iterating on this!
Some of the edits do make the document easier to read, however some sections lost a great deal of normative power (e.g. the definition of the interoperable names) and I'd like for this to be as clear-cut of a spec as possible. The better-flowing prose is very well suited for an explainer-type document, though.
Also, some editorial concerns:
- the reordering breaks automated checks on the document structure (e.g. no extraneous sections, required sections, etc)
- the status should be moved to Review, and then we should ask an editor to move it to Last Call
ERCS/erc-7930.md
Outdated
| Furthermore, this proposal adds a few features to make addresses interoperable with infrastructure at the edges of and beyond the ethereum ecosystem, by supporting the representation of addresses of non-evm chains as well, for the benefit of cross-chain liquidity movements and cross-chain message passing. | ||
| - Binds chain identification to the raw address. | ||
| - Is compact for usage with cross-chain message passing and intent declaration. | ||
| - Includes checksums to protect users. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this paragraph loses a great deal of specificity when rewritten. While I get that the previous iteration could be needlessly verbose, this no longer differentiates between binary and text formats (which like half of the value proposition of this ERC) and doesn't say what or how it protects users from
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've iterated on this further to address these concerns.
|
|
||
| ### Interoperable Address V1 binary format definition | ||
| In binary format, addresses MUST have the following encoding: | ||
| An _Interoperable Address_ as defined by this standard MUST have the following binary format: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good fix on using the name whenever applicable
ERCS/erc-7930.md
Outdated
| Interoperable Name | ||
| : A string representation of an interoperable address, meant to be used by humans. | ||
| **Interoperable Name** | ||
| : A string representation of an _Interoperable Address_. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the human-readability of it is a crucial clarification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated for clarity.
ERCS/erc-7930.md
Outdated
| ChainType | ||
| : A 2-byte value as defined in [CAIP-350], corresponding to a [CAIP-2] _namespace_, which allows users to know how to interpret & display the other two fields. | ||
| **ChainType** | ||
| : A 2-byte value as defined in [CAIP-350], corresponding to a [CAIP-2] namespace, which allows users to know how to interpret and display the `ChainReference` and the `Address`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's standard practice to have new concepts highlighted in italics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted. Updated.
ERCS/erc-7930.md
Outdated
| Address | ||
| : Variable length field containing the binary format of the address component. For serialization details of different types of addresses see the [CAIP-350] profile for the chain type. | ||
| **Address** | ||
| : Variable length field containing the binary encoding of the address. To discern how to serialize an address for a specific chain type, implementors MUST reference the respective [CAIP-350] profile. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think it makes sense to normative-MUST the algorithm complying with the CAIP profile, and not the implementor referencing the spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha. Valid point. Updated.
ERCS/erc-7930.md
Outdated
| Clients MAY include the checksum when displaying an _Interoperable Name_ within their interface. | ||
| Clients MAY accept _Interoperable Name_ inputs without a checksum. | ||
|
|
||
| ## Examples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iirc EIP-1 requires this is called test cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EIP 1 states "Test cases for an implementation are mandatory for EIPs that are affecting consensus changes."
Because this ERC isn't related to consensus, and 'Test cases' is an optional heading I opted to call it 'Examples' because it's semantically clearer IMO..
ERCS/erc-7930.md
Outdated
| ## Rationale | ||
|
|
||
| It is therefore advised for implementers requiring canonicity of addresses (e.g by using them as keys in smart contract mappings or other key-value stores), to thoroughly review the [CAIP-350] profile of a chain namespace for the possibility of a lack of canonicity of addresses (which should be noted in the profile's 'Extra Considerations' section) as well as collisions with other already-supported namespaces. | ||
| - The `@` symbol is used as a separator as it is clear, easy for software to parse, and avoids confusion with the colon (`:`) symbol utilized in [CAIP-2] identifiers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel the rewrite of this section is more vague
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specifically as it pertains to canonicity as a security consideration? Happy to flesh out - is there an obvious example of a collision with another namespace?
I've updated, and commented as appropriate. I've moved it to 'Review' and fixed the validation checks. I think my fundamental issue was that on first read I was unable to follow exactly what was going on. The separation of the Interoperable Address and Interoperable Name definitions in my opinion makes this significantly more accessible. I do agree that the spec should be as clear-cut as possible, so if I've inadvertently removed meaning I apologise, and am happy to iterate to make sure it is all in there. This is my first foray into contributing to specs, and my perception is that a spec should be easy to understand independent of any external resources or explainers so as to incentivize people to actually read and contribute. |
0xteddybear
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kudos on complying with the editorial requirements, but there is still a significant loss of specificity
ERCS/erc-7930.md
Outdated
| - Any future Interoperable Address will be trivially convertible to Interoperable Address v1 (specified below), which, together with the point below, makes them a canonical representation for users who need to treat them opaquely. | ||
| - Two Interoperable Addresses, converted to this canonical version, will be bitwise-equal if and only if they represent the same _target address_. | ||
| - Checksums are short enough to be compared by humans, and allow for easy differentiation of _target addresses_ independently of any extra data the Interoperable Address may contain. | ||
| - Bind together chain identification and the raw address. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should be 'binds' ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
|
|
||
| ## Motivation | ||
| Current Ethereum addresses ([ERC-55]) lack chain specificity and have optional checksums, creating several challenges: | ||
| The address format utilized on Ethereum mainnet is shared by a large number of other blockchains. The format does not include details of the chain on which an interaction should occur. This introduces risk if, for example, a transaction is mistakenly executed on a chain where the address is inaccesible. This risk is particularly pronounced for addresses that represent smart contracts or smart accounts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this introduction is clearly better, thanks
ERCS/erc-7930.md
Outdated
| - Is compact for usage with cross-chain message passing and intent declaration. | ||
| - Includes short, easily verififiable checksums to protect users. | ||
| - Extends beyond EVM blockchains. | ||
| - Is flexible and extensible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is a bit easier to follow on what this standard provides, but not why/how. I wrote it like this to address actual questions from people in the ecosystem (with @frangio in particular being the guinea pig on how to sell a smart contract implementor on this). My impression is that every standard sells itself as flexible and extensible, and it's crucial to explain what it means in this context
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair. I have referenced the original and modified this section accordingly to add some more 'why'. I think that the 'how' is pushed in the Specification.
ERCS/erc-7930.md
Outdated
| It is trivial to add support for chains to [CAIP-10], but the standard has its drawbacks: | ||
|
|
||
| - There is no canonical identifier for a specific address on a specific chain - there could be multiple valid [CAIP-10] addresses pointing to the same target address. | ||
| - The format is not optimized for usage within smart contracts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo not saying how it is unoptimized is a loss for readability and convince-people-to-use-this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair. Added.
| **Interoperable Name** | ||
| : A string representation of an _Interoperable Address_, meant to be used by humans. | ||
|
|
||
| Target address |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is still used throughout the document, it's worth it to define it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defined, how it is used within this updated spec.
ERCS/erc-7930.md
Outdated
| - MUST support defining an address, a chain, or both | ||
| - MUST allow the client to discern the data necessary to calculate the checksum as outlined in this standard | ||
| - MAY only be able to represent a subset of the CAIP namespaces | ||
| - MAY expand upon the _Interoperable Name_ definition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'expand' is much less specific than 'assign extra syntactic restrictions', what do you mean precisely by 'expand'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'assign extra syntactic restrictions' feels constraining in that 7828 for example actually loosens restrictions - it allows you to use ENS names. My usage of 'expand' was I guess intentionally less specific as we don't know how people may build upon this in the future.
I've gone for 'MAY modify the syntactic restrictions of the Interoperable Name definition' for now..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'assign extra syntactic restrictions' feels constraining in that 7828 for example actually loosens restrictions - it allows you to use ENS names.
This is not correct. A grammar is defined as a set of strings (some grammars can also be described by things such as regular expressions of BNF notation). The grammar defined in 7930 is less restrictive than 7828's. As an example, 0x00@... would be a syntactically valid 7930 name (albeit semantically meaningless), but invalid 7828, since AFAICT the ENS grammar specifies you got to have labels in between dots.
My usage of 'expand' was I guess intentionally less specific as we don't know how people may build upon this in the future. I've gone for 'MAY modify the syntactic restrictions of the Interoperable Name definition' for now..
I'm against this. While we should not make standards maximally restrictive (as in, disallowing future usecases), we should make them maximally specific (as in, being easy to figure out if something fits the standard or doesn't). People will write parsers for this; they'll want to be able to figure out if a string they receive is a 7828 or just 7930, and the standard should ensure that decision is computable.
Another way to think about the specificity requirements for this document is that it should be possible to develop two independent implementations of libraries that are compatible with one another, even if the teams developing them have an adversarial relationship between them
| This standard aims to be a foundational standard to allow for canonical representations of chain specific addresses. In the real world it can not be guaranteed that there will only be one valid representation of a chain specific address. For example, a particular chain namespace might define a serialization scheme that can't guarantee canonicity of addresses, or a given network might have two valid [CAIP-2] IDs referring to it. | ||
|
|
||
| --> | ||
| It is therefore advised that implementers requiring canonicity of addresses (if for example you are using them as keys in smart contract mappings), to thoroughly review the [CAIP-350] profile of a chain namespace for the possibility of a lack of canonicity. These considerations should be noted in the profile's 'Extra Considerations' section. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is repeated with the 'rationale' section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what happened there. Removed from 'Rationale' as more appropriate in 'Security Considerations'.
ERCS/erc-7930.md
Outdated
|
|
||
| ## Copyright | ||
| Copyright and related rights waived via [CC0](../LICENSE.md). | ||
| - The `@` symbol is used as a separator as it is clear, easy for software to parse, and avoids confusion with the colon (`:`) symbol utilized in [CAIP-2] identifiers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
software parsing is not particularly hard in the case of using another separator, but might be less obvious for humans
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Still left in 'easy for software to parse' because it's still true, even if other separators are just as easy.
ERCS/erc-7930.md
Outdated
| ## Copyright | ||
| Copyright and related rights waived via [CC0](../LICENSE.md). | ||
| - The `@` symbol is used as a separator as it is clear, easy for software to parse, and avoids confusion with the colon (`:`) symbol utilized in [CAIP-2] identifiers. | ||
| - No length restriction is placed on addresses, allowing for chains with longer address formats to be represented. This is flexible and future proof. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sold on the 'This is flexible and future proof', feels too chatgpt-y while being less specific than the previous wording
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed. I think the flexibility is implied anyway
ERCS/erc-7930.md
Outdated
| - No length restriction is placed on addresses, allowing for chains with longer address formats to be represented. This is flexible and future proof. | ||
| - The address field includes `%` as a valid character to allow for URL-encoded address formats. | ||
| - We chose to support zero-length addresses and chain IDs to make this standard flexible and to allow developers to use a single, uniform standard for many different jobs. | ||
| - We chose **not** to utilize alternate encoding formats (`base58` or `base64` for example) to reduce the size of the _Interoperable Address_ and/or to abstract away namespace specific address serialization standards. We opted not to noting that readability and familiarity are important user experience considerations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'we opted not to noting that readability...' not sure I follow this sentence
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to explain our justification - that the space savings are not worth making it less familiar or readable for users. I have clarified further.
ERCS/erc-7930.md
Outdated
| | **CAIP-2 Namespace** | `solana` | | ||
| | **CAIP-2 Chain ID** | `5eykt4UsFv8P8NJdTREpY1vzqKqZKvdpKuc147dw2N9d` | | ||
| | **Address** | `MJKqp326RZCHnAAbew9MDdui3iCKWco7fsK9sVuZTX2` | | ||
| | **Checksum Input** | `0x00000100x00022045296998a6f8e2a784db5d9f95e18fc23f70441a1039446801089879b08c7ef02005333498d5aea4ae009585c43f7b8c30df8e70187d4a713d134f977fc8dfe0b5114D8DA6BF26964AF9D7EED9E03E53415D37AA96045` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this input is incorrect, it includes the address from the example above. But the interoperable names's checksum hasn't changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure whats happened here. These were from the original. Possibly miscopied or something by me. Regardless, fixed.
0xMonoAx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some comments and corrections to improve the clarity and consistency of the ERC
|
|
||
| ## Abstract | ||
| Interoperable Addresses is a binary format to describe an address specific to one chain with a companion 'Interoperable Names' specification for human-readable display. | ||
| This proposal introduces a **binary format** to describe a chain specific address. Additionally, it defines a human-readable version of that identifier for improved user experience for user facing interactions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since ERC-7930 is quite dependent on CAIP-350, do you think it would make sense to clarify this in the Abstract?
I was thinking of something like:
"This ERC defines a generic, versioned binary container format for representing an address bound to a specific blockchain. The interpretation and serialization rules for the chain reference and address components are delegated to complementary standards, namely, CAIP-350 profiles."
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added the section you outlined below. I also added explicit reference to CAIP-350 here.
| : Variable length field containing the binary encoding of the address. The serialization for a specific _ChainType_ MUST follow the rules of its corresponding [CAIP-350] profile. | ||
|
|
||
| These rules ensure that future versions of Interoperable Addresses maintain backwards compatibility and consistent behavior across implementations: | ||
| ### Interoperable Name Definition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be helpful to clarify the relationship between this standard and [ERC-7828].
We could add something like the following:
"Resolution of Interoperable Names MAY be delegated to name service standards such as ENS (see [ERC-7828]) or other equivalent systems. Implementations MUST ensure that any binary data derived from such name resolution strictly conforms to the Interoperable Address encoding defined in this specification."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than add this here, I have added the final statement to the 'Future versions' sections.
My view is that highlighting 7828 and name services is too concrete, as specs can build upon this one in a multitude of ways as long as they follow the rules outlined.
ERCS/erc-7930.md
Outdated
| - It makes the Interoperable Address <-> raw address relationship evident to the user, which will make the cases where the former are not (yet) supported by a dapp or hardware wallet easier to reason with, as users will evidently see the address is the same, but it's the chain&checksum that is being trimmed. | ||
| - Its extra human-readability will make it more useful as a a stopgap solution until a standard focused on chain and address names is finalized. | ||
| - We chose to support zero-length addresses and chainids to be able to use just one standard to represent both potentially foreign _target addresses_ (e.g. the recipient of a cross-chain message) and also plain addresses or chainids (e.g. to specify the origin-chain hook address of a cross-chain message), for greater uniformity across implementations. | ||
| `<checksum>` is a 4-byte checksum calculated by taking the first 4-bytes of the hexadecimal (as defined in [RFC 4648]) representation of the keccak256 hash of the concatenated _ChainType_, _ChainReferenceLength_, _ChainReference_, _AddressLength_ and _Address_ fields of the binary _Interoperable Address_. The `Version` field is **not** included. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it typed like this? What do you think about it?
The checksum is a 4-byte value calculated by taking the first 4 bytes of the hexadecimal (as defined in [RFC 4648] string representing the keccak256 hash.
The hash is computed over the concatenation of the following binary fields of the Interoperable Address: ChainType, ChainReferenceLength, ChainReference, AddressLength, and Address.
The Version field MUST NOT be included in the hashed data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reworded based on your suggestion.
ERCS/erc-7930.md
Outdated
|
|
||
| A 4-byte checksum MUST be computed and included when sharing an _Interoperable Name_. | ||
|
|
||
| If a user-provided _Interoperable Name_ includes a checksum, clients MUST discern the underlying _Interoperable Address_ recompute the checksum and compare it to the provided value. On mismatch, clients MUST alert the user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about adding this section? I think it would improve clarity regarding the relationship with CAIP-350 (if I add this, I would remove the clarification from the Abstract)
Parsing and Serialization:
When parsing an Interoperable Name to generate its binary Interoperable Address, clients MUST follow the normalization and serialization rules defined in the relevant CAIP-350 profile for the given <chain> and <address>. This ensures that different valid text representations (e.g., case variations in an address) resolve to a single, canonical binary form, which is essential for consistent checksum calculation and data integrity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I like this a lot. Added verbatim.
ERCS/erc-7930.md
Outdated
| : A binary payload which unambiguously identifies a target address and allows conversion to a human-readable name. | ||
| ### Terminology | ||
| **Target Address** | ||
| : The target address being represented. Serialized subject to the rules outlined in the [CAIP-350] profile for the namespace of the chain being targetted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo “targetted” -> “targeted”
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. (You added a typo in your comment XD)
ERCS/erc-7930.md
Outdated
|
|
||
| ## Copyright | ||
| Copyright and related rights waived via [CC0](../LICENSE.md). | ||
| Future versions.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo "Future versions.." -> "Future versions:"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
ERCS/erc-7930.md
Outdated
|
|
||
| A 4-byte checksum MUST be computed and included when sharing an _Interoperable Name_. | ||
|
|
||
| If a user-provided _Interoperable Name_ includes a checksum, clients MUST discern the underlying _Interoperable Address_ recompute the checksum and compare it to the provided value. On mismatch, clients MUST alert the user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clients MUST discern the underlying Interoperable Address recompute the checksum and compare it to the provided value. On mismatch, clients MUST alert the user.
I think “dicern” is a bit ambiguous. What do you think of this phrase:
clients MUST derive the underlying Interoperable Address, recalculate the checksum, and compare it to the provided value. In case of a mismatch, clients MUST warn the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
|
The commit 1cdfe30 (as a parent of 2bcfeba) contains errors. |
| title: Interoperable Addresses | ||
| description: An extensible binary format to refer to an address specific to one chain. | ||
| author: Teddy (@0xteddybear), Joxes (@0xJoxess), Nick Johnson (@Arachnid), Francisco Giordano (@frangio), Skeletor Spaceman (@skeletor-spaceman), Racu (@0xRacoon), TiTi (@0xtiti), Gori (@0xGorilla), Ardy (@0xArdy), Onizuka (@onizuka-wl), Sam Kaufman (@SampkaML), Marco Stronati (@paracetamolo), Yuliya Alexiev (@yuliyaalexiev), Jeff Lau (@jefflau), Sam Wilson (@samwilsn), Vitalik Buterin (@vbuterin) | ||
| author: Teddy (@0xteddybear), Joxes (@0xJoxess), Nick Johnson (@Arachnid), Francisco Giordano (@frangio), Skeletor Spaceman (@skeletor-spaceman), Racu (@0xRacoon), TiTi (@0xtiti), Gori (@0xGorilla), Ardy (@0xArdy), Onizuka (@onizuka-wl), Sam Kaufman (@SampkaML), Marco Stronati (@paracetamolo), Yuliya Alexiev (@yuliyaalexiev), Jeff Lau (@jefflau), Sam Wilson (@samwilsn), Vitalik Buterin (@vbuterin), Thomas Clowes (@clowestab), Mono (0xMonoAx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mono's username is missing the @
| : A binary payload which unambiguously identifies a target address and allows conversion to a human-readable name. | ||
| ### Terminology | ||
| **Target Address** | ||
| : The target address being represented. Serialized subject to the rules outlined in the [CAIP-350] profile for the namespace of the chain being targeted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so far we have used 'target address' to mean an address already scoped to a given chain, aka the object of this proposal, and we haven't used the 'target chain' concept. Also, this definition is recursive 😁
| It's format is `<address> @ <chain> # <checksum>` where the components match the following regular expressions: | ||
|
|
||
| #### Syntax | ||
| ```bnf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the rationale behind removing the element we're actually trying to define from the grammar definition? keep in mind this is a kinda standard format: https://en.wikipedia.org/wiki/Backus%E2%80%93Naur_form
|
|
||
| Address | ||
| : Chain namespace specific text representation of the address from the binary representation. Mapping between the two described in the [CAIP-350] profile for the chain type. In the case where `AddressLength` is zero, it should be the empty string. | ||
| `<chain>` is the string representation of a specific blockchain as defined in [CAIP-2]. For example `eip155:1` for Ethereum Mainnet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is way less specific than the current wording, and even explicitly discards the handling of an edge case 😭
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, it should not be CAIP-2 but CAIP-350. They can be different when e.g. the chain reference is truncated in CAIP-2 but used in full in CAIP-350
0xteddybear
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure this process is moving forward. Am open to other ways of making this iteration more effective (e.g. splitting this up into a few PRs or working on a feature branch where we modify one section at a time)
|
|
||
| Address | ||
| : Chain namespace specific text representation of the address from the binary representation. Mapping between the two described in the [CAIP-350] profile for the chain type. In the case where `AddressLength` is zero, it should be the empty string. | ||
| `<chain>` is the string representation of a specific blockchain as defined in [CAIP-2]. For example `eip155:1` for Ethereum Mainnet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, it should not be CAIP-2 but CAIP-350. They can be different when e.g. the chain reference is truncated in CAIP-2 but used in full in CAIP-350
| - It makes the Interoperable Address <-> raw address relationship evident to the user, which will make the cases where the former are not (yet) supported by a dapp or hardware wallet easier to reason with, as users will evidently see the address is the same, but it's the chain&checksum that is being trimmed. | ||
| - Its extra human-readability will make it more useful as a a stopgap solution until a standard focused on chain and address names is finalized. | ||
| - We chose to support zero-length addresses and chainids to be able to use just one standard to represent both potentially foreign _target addresses_ (e.g. the recipient of a cross-chain message) and also plain addresses or chainids (e.g. to specify the origin-chain hook address of a cross-chain message), for greater uniformity across implementations. | ||
| The 4-byte `<checksum>` is the first 4 bytes of the hexadecimal ([RFC 4648]) string representation of a keccak256 hash. This hash is computed over the concatenation of the _Interoperable Address_'s binary fields: `ChainType`, `ChainReferenceLength`, `ChainReference`, `AddressLength`, and `Address`. The `Version` field MUST NOT be included in the hashed data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: 'the 4byte checksum is the first 4 bytes...' feels like could be improved writing-wise, but can't think of an actually better alternative. It does not compromise clarity though, so not a blocker
| --> | ||
| - The `@` symbol is used as a separator as it provides visual clarity to humans, is easy for software to parse, and avoids confusion with the colon (`:`) symbol utilized in [CAIP-2] identifiers. | ||
| - No length restriction is placed on addresses, allowing for chains with longer address formats to be represented. | ||
| - The address field includes `%` as a valid character to allow for URL-encoded address formats. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the previous expression was clearer than this one:
Similarly, the address field includes
%as a valid character to allow for url-encoding of any other characters.
it clearly states that the purpose of using % is to url-encode characters not natively supported by the standard. On the other hand, it's not clear what 'URL-encoded' address formats' means, precisely. Are there address formats out there which are in some way URLs?
| ## Rationale | ||
|
|
||
| --> | ||
| - The `@` symbol is used as a separator as it provides visual clarity to humans, is easy for software to parse, and avoids confusion with the colon (`:`) symbol utilized in [CAIP-2] identifiers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the previous wording is a more comprehensive explaination of why we chose to use the @ character. In particular:
- it makes it trivial to distinguish CAIP-10 ids from Interoperable Names
- it also makes sure we can fit into this format anything that can fit in a CAIP-10
| - No length restriction is placed on addresses, allowing for chains with longer address formats to be represented. | ||
| - The address field includes `%` as a valid character to allow for URL-encoded address formats. | ||
| - We chose to support zero-length addresses and chain IDs to make this standard flexible and to allow developers to use a single, uniform standard for many different jobs. | ||
| - We chose **not** to utilize alternate encoding formats (`base58` or `base64` for example) to reduce the size of the _Interoperable Address_ and/or to abstract away namespace specific address serialization standards. We decided that using the chain specific address formats that users are used to provides a better user experience. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rewrite includes much less on why we went for this approach, which is the purpose of the rationale section
Updates to ERC-7930 to make it more clear and accessible based on community feedback and discussions within the Interop working group.
When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md
We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met: