-
Notifications
You must be signed in to change notification settings - Fork 809
Update ERC-7730: Remove embedded ABI; keys-as-schema #1289
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
Conversation
|
🛑 Auto merge failed. Please see logs for more details, and report this issue at the |
|
I'm actually thinking if a full ABI JSON in this spec is an overkill. In a way this information is redundant because we are already providing it in the format spec of each function: "formats": {
"myFunction(address _address,uint256 _amount,MyParamType _param)": {The key here contains all the information needed to identify and ABI-decode the parameters. This is the preferred format in ethers.js and is fully translatable to other formats. We can even fully remove the "abi" field and require proper formatting of Right now the spec allows 3 possibilities on this key:
With that the spec would only allow the last one, "full Solidity signature", or maybe better defined as the ether.js Human-readable ABI The ABI itself requires a verification/validation step anyway so I don't see the benefit of including the full ABI. |
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.
Some comments. Otherwise lgtm thank you! I guess we need to update the JSON schema as well
Co-authored-by: Kaan Uzdoğan <[email protected]>
Remove the abi from ERC7730 json schema and examples
| "interpolatedIntent": "Approve {spender} to spend up to {amount} on your behalf until {deadline}", | ||
| "fields": [ | ||
| {"path": "spender", "label": "Spender", "format": "addressName"}, | ||
| {"path": "amount", "label": "Amount", "format": "tokenAmount", "params": {"tokenPath": "@.to"}}, |
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.
params should not be dropped here and in next line
| "formats": { | ||
| "title": "List of field formats", | ||
| "description": "The list includes formatting info for each field of a structure. This list is indexed by a key identifying uniquely the message's type in the abi. For smartcontracts, it is the selector of the function or its signature; and for EIP712 messages it is the primaryType of the message.", | ||
| "description": "The list includes formatting info for each field of a structure. This list is indexed by the full function signature with parameter names.", |
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.
Missing the index type for EIP-712 (since they do not have an equivalent of human readable ABI)
| "description": "The contract binding context is a set constraints that are used to bind the ERC7730 file to a specific smart contract.", | ||
|
|
||
| "properties": { | ||
| "abi": { |
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.
Can we leave the ABI key and mark it as deprecated? To avoid breaking a lot of existing files?
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.
added a short mention
| "title": "List of field formats", | ||
| "description": "The list includes formatting info for each field of a structure. This list is indexed by a key identifying uniquely the message's type in the abi. For smartcontracts, it is the selector of the function or its signature; and for EIP712 messages it is the primaryType of the message.", | ||
| "description": "The list includes formatting info for each field of a structure. This list is indexed by the full function signature with parameter names.", | ||
| "type": "object", |
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 we're making the key itself a reference of the schema, might be useful to validate the format itself using the schema. Something like this should work:
"propertyNames": {
"pattern": "^[A-Za-z_][A-Za-z0-9_]*\\([A-Za-z0-9_\\[\\]()]+ [A-Za-z_][A-Za-z0-9_]*(?:,[A-Za-z0-9_\\[\\]()]+ [A-Za-z_][A-Za-z0-9_]*)*\\)$"
}
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 surfacing that. I’ve reintroduced a stricter propertyNames guard so schema validation enforces the “full function signature with parameter names” contract. The pattern now allows:
- Named parameters with optional array suffixes and memory|calldata|storage qualifiers.
- Tuple/struct parameters, including nested tuples (tuple((uint256,address) info)).
- Optional whitespace and the empty-parameter case (pause()).
Here’s what landed:
"propertyNames": {
"pattern": "^[A-Za-z_][A-Za-z0-9_]*\\(\\s*(?:(?:(?:tuple(?:\\s*\\((?:[^()]|\\([^()]*\\))*\\))?|[A-Za-z0-9_]+)(?:\\[[0-9]*\\])*(?:\\s+(?:memory|calldata|
storage))?\\s+[A-Za-z_][A-Za-z0-9_]*)(?:\\s*,\\s*(?:(?:tuple(?:\\s*\\((?:[^()]|\\([^()]*\\))*\\))?|[A-Za-z0-9_]+)(?:\\[[0-9]*\\])*(?:\\s+(?:memory|
calldata|storage))?\\s+[A-Za-z_][A-Za-z0-9_]*))*\\s*)?\\)$"
}
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.
Thanx for all the help
Summary
This PR simplifies ERC-7730 by removing embedded ABIs and making display.formats the single source of truth for routing, decoding, and labeling.
Functions only: scope limited to calldata clear-signing; events/errors out of scope.
Keys = schema & router: each display.formats key is a Human-Readable ABI function with parameter names (e.g., transfer(address to,uint256 value)).
Selector matching: wallets compute keccak256()[:4] from the key to match calldata.
Decoding from key: wallets parse the key’s canonical types to decode; names from the key populate paths.
Why: reduces duplication and drift, shrinks descriptors, and aligns with existing wallet UX while keeping overloads and complex types (tuples/arrays) precise via canonical types.