-
Notifications
You must be signed in to change notification settings - Fork 801
Update ERC-7828: Updates to ERC-7828 to make it more clear and accessible. #1267
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
|
|
The commit bd717b6 (as a parent of 5ed4050) contains errors. |
ERCS/erc-7828.md
Outdated
|
|
||
| #### Step 4: Resolve alice.eth **for** the specified chain | ||
| 5. Extract the CAIP-2 `ChainReference` from the [ERC-7930] identifier returned in the previous step. | ||
| 6. Convert the `ChainReference` into an ENS `coinType` (see ENSIP-11). |
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.
For non-EVM chains, ENSIP-11 does not apply.
We could define how both EVM and non-EVM chains can expose their coin type/SLIP-44 constant through the chain registry for consistency.
Quick idea would be specifying a coin-type text key that returns the value?
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 would be equivalent to slip-44 in https://chainid.network/chains.json? It doesn't seem to be there for all chains in that list, so there would need to be some kind of fallback?
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.
ENSIP-11 can derive the coin type for EVM chains using their chain ID. For non-EVM chains, we use the SLIP-44 constant
The idea is to abstract that complexity away and surface the appropriate value for each chain using a coin-type text record. i.e resolving the text record coin-type for optimism.<namespace>.eth returns the ENSIP-11 coin type / SLIP-44 value
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 original spec had the following: "For addresses on chains that are both supported by BIP-44 registered coin type identifiers and the special scheme for the eip155 namespace defined in ENSIP-11, the latter should be used.".
I will readd this for clarity, and add additional commentary to outline what @ndeto has mentioned, namely:
"The idea is to abstract that complexity away and surface the appropriate value for each chain using a coin-type text record. i.e resolving the text record coin-type for optimism..eth returns the ENSIP-11 coin type / SLIP-44 value"
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.
Done
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.
let's keep the formal grammar definition (although I'm open to modify it), rely on the things already defined in ERC-7930, and clear out the confusion on using CAIP-2 directly
After that we can focus on what should be defined in this ERC vs the relevant ENSIP
| This proposal extends [ERC-7930](./eip-7930.md) (Interoperable Addresses & Names) by standardizing a human-readable format for chain-specific addresses in the form `<address>@<chain>#<checksum>`. It introduces: | ||
| This proposal extends the definition of an _Interoperable Name_ outlined in [ERC-7930](./eip-7930.md). | ||
|
|
||
| The `<chain>` component definition is extended to allow for a human-readable chain name to be used. This identifier is resolved to an [ERC-7930] _Interoperable Address_ using the Ethereum Name Service (ENS). This moves chain metadata discovery on-chain. |
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.
let's keep at least a mention of 7930's <address>@<chain>#<checksum> 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.
OK. Re-added.
ERCS/erc-7828.md
Outdated
| Moreover, the above can be leveraged to allow for a name to represent the same entity corresponding to different addresses on different chains, mitigating the risk of sending funds to an address the intended recipient doesn't actually control. | ||
| The flexibility of this specification and the underlying naming systems can be leveraged to allow for different addresses on different chains to be represented by the same human-readable string. This mitigates the risk of sending funds to an address the intended recipient doesn't actually control. | ||
|
|
||
| The usage of human-readable names provides additional clarity and confidence to 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 is vague and redundant
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. Removed.
| These components now have the following extended definitions: | ||
|
|
||
| ```bnf | ||
| <address> := <raw-address> | <name-service-domain> |
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 significantly more vague and relies on prose instead of standard BNF notation
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.
Are you requesting the definitions of <raw-address> and <name-service-domain> be in the bnf block too?
ERCS/erc-7828.md
Outdated
| <ens-name>: := (<label> .)+ <label> | ||
| Recall that [ERC-7930] defines an _Interoperable Name_ as a human readable representation of an underlying _Interoperable Address_. | ||
|
|
||
| It's format is `<address> @ <chain> # <checksum>` |
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 it should be 'its'
also, having this in the prose is good but let's please also keep it in the formal definition as well
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.
Correct. Fixed.
| `<name-service-domain>` is a domain name from a known, and supported naming service. For example an Ethereum Name Service (ENS) name like `example.eth`. These names should be resolved to a `<raw-address>` subject to the resolution specifications of the name service in question. | ||
|
|
||
| A few examples below: | ||
| `<caip-2-chain-id>` is the string representation of the [CAIP-2] blockchain identifier. For example, `eip155:1` or `bip122:000000000019d6689c085ae165831e93`. An ERC-7828 _Interoperable Name_ MUST specify both the namespace and reference elements of the CAIP-2 identifier as ENS resolves addresses on a chain specific basis. |
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 contradictory with 7930, why are you defining it like 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.
I think this only applies in the case where the address is an ENS name (i.e. vitalik.eth@eip155 is underspecified because vitalik.eth could resolve to different addresses on different chains). 0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045@eip155 is valid but that isn't a 7828 name (it's readable 7930)
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, as outlined by @0xrcinus , this was discussed with @skeletor-spaceman - the problem is that ENS name resolution is chain specific NOT namespace specific. I wouldn't say this contradicts, but rather modifies the syntactic restrictions outlined in ERC-7930
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.
Add some comments and corrections to make the ERC clearer.
ERCS/erc-7828.md
Outdated
| ``` | ||
|
|
||
| Wallets MAY display the checksum when formatting addresses, and MAY accept inputs without a checksum by computing it internally. | ||
| The checksum is optional and is calculated as outlined in [ERC-7930]. |
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.
To maintain consistency with ERC-7930, I would keep the previous sentence: "Clients MAY accept Interoperable Name inputs without a checksum."
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.
ERCS/erc-7828.md
Outdated
| This specification proposes that the `<chain>` component of the _Interoperable Name_ definition outlined in [ERC-7930] be extended to allow for a human-readable chain name resolved on-chain via ENS. This registry would provide a single source of truth for mapping a chain name to an [ERC-7930] _Interoperable Address_ and associated chain metadata. | ||
|
|
||
| In the same spirit, the address could be a human-readable name as well, which is already a use case for ENS. By coupling the TLD to the resolving method used, this standard could leverage current and future features of ENS as well as other naming registries. | ||
| In the same spirit, we propose that the `<address>` component of the definition allow for a human-readable name to be utilized. By coupling the TLD to the resolution method used, this standard could leverage current and future features of ENS as well as other naming systems. |
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.
In the same spirit, we propose...
Change to:
“This specification proposes…”
(EIPs prefer an impersonal tone)
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
ERCS/erc-7828.md
Outdated
|
|
||
| > [!NOTE] | ||
| > This standard explicitly defines the use of an agreed <namespace>.eth second-level domain for chain names. Beyond this, hierarchical name structures continue to be inherited from ENS, as exemplified in the ENS-in-rollup example. | ||
| When checksum validation fails clients MUST alert the user and require explicit user input to continue with the operation. |
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.
..and require explicit user input to continue with the operation.
This is not specified in ERC-7930. Should we add 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.
A similar statement is included in 7930. I've added to it for completeness: "If a user-provided Interoperable Name includes a checksum, 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 and require explicit user input to continue with the operation."
ERCS/erc-7828.md
Outdated
|
|
||
| > [!NOTE] | ||
| > This standard explicitly defines the use of an agreed <namespace>.eth second-level domain for chain names. Beyond this, hierarchical name structures continue to be inherited from ENS, as exemplified in the ENS-in-rollup example. | ||
| When checksum validation fails clients MUST alert the user and require explicit user input to continue with the operation. |
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 delegates parsing to CAIP-350, it would be ideal to repeat a line here that reinforces that requirement.
Similar: #1265 (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've added a block about this in the context of 7828.
ERCS/erc-7828.md
Outdated
| 5. Query the Universal Resolver: `result = UniversalResolver.resolve(dnsEncode(full), callData)`. | ||
| 6. ABI-decode `result` as `(bytes chainBytes)`. | ||
| • If `chainBytes` is empty → no mapping exists; treat the label as unknown. | ||
| If the `<chain>` is `ethereum`, you would resolve the `chain-id` data record for `ethereum.<namespace>.eth` subject to ENSIP-24: Arbitrary Data Resolution. |
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.
If the
<chain>isethereum, you would..
“You” is not used in standards. I propose:
If the <chain> component is ethereum, implementations MUST resolve the chain-id data record for ethereum.<namespace>.eth according to [ENSIP-24: Arbitrary Data Resolution].
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. I havn't linked to ENSIP-24 as ENSIPs are not in the allowed external links..
| ### Reverse resolution | ||
|
|
||
| ### Resolving address names | ||
| In the context of this specification 'Reverse Resolution' refers to the process of discerning the `<chain-label>` of a chain from its _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.
'Reverse Resolution'
change to:
Reverse Resolution
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-7828.md
Outdated
| 3. The underlying label for 'Optimism' is 'optimism'. Append `<namespace.eth>` to that label => `optimism.<namespace>.eth` | ||
|
|
||
| #### Step 3: Resolve the [ERC-7930] _Interoperable Address_ | ||
| 4. Discern the resolver for `optimism.<namespace>.eth` and resolve `data(bytes32 namehash, string key)` (ENSIP-24: Arbitrary Data Resolution) passing in the `namehash` (see ENSIP-1) of `optimism.<namespace>.eth` and the key, `chain-id`. |
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.
“Dicern” is ambiguous; I recommend using “Determine.”
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
| Consideration should be given to ENSIP-15: Name Normalization so as to avoid homograph attacks. | ||
|
|
||
| Wallets MUST surface whether reverse resolution was trust-minimized or trusted. | ||
| Wherever possible clients should utilize the optional checksum outlined in [ERC-7930]. It should be verified when a raw _Interoperable Name_ is pasted, and it should be appended when an _Interoperable Name_ is being generated based on user input. |
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 you define what happens when the resolver returns invalid 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.
Which aspect are you referring to here? Are you suggesting documenting that if (for example) a non existent chain name is resolved 0x will be returned?
Updates to ERC-7828 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: