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

Update ERC-7739: Update ERC-7739 #687

Merged
merged 16 commits into from
Oct 28, 2024
91 changes: 46 additions & 45 deletions ERCS/erc-7739.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
eip: 7739
title: Readable Typed Signatures for Smart Accounts
description: A defensive rehashing scheme which prevents signature replays across smart accounts and preserves the readability of the signed contents
author: vectorized (@vectorized), Sihoon Lee (@push0ebp), Francisco Giordano (@frangio), Im Juno (@junomonster), howydev (@howydev), 0xcuriousapple (@0xcuriousapple)
author: vectorized (@vectorized), Sihoon Lee (@push0ebp), Francisco Giordano (@frangio), Hadrien Croubois (@Amxx), Ernesto García (@ernestognw), Im Juno (@junomonster), howydev (@howydev), Atarpara (@Atarpara), 0xcuriousapple (@0xcuriousapple)
discussions-to: https://ethereum-magicians.org/t/erc-7739-readable-typed-signatures-for-smart-accounts/20513
status: Draft
type: Standards Track
Expand Down Expand Up @@ -55,7 +55,7 @@ The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "S

### Overview

Compliant smart accounts MUST implement the following dependencies:
The following dependencies are REQUIRED:

- [EIP-712](./eip-712.md) Typed structured data hashing and signing.
Provides the relevant typed data hashing logic internally, which is required to construct the final hashes.
Expand Down Expand Up @@ -84,8 +84,7 @@ keccak256(\x19\x01 ‖ APP_DOMAIN_SEPARATOR ‖
version: eip712Domain().version,
chainId: eip712Domain().chainId,
verifyingContract: eip712Domain().verifyingContract,
salt: eip712Domain().salt,
extensions: eip712Domain().extensions
salt: eip712Domain().salt
}))
)
```
Expand All @@ -112,8 +111,7 @@ finalTypedDataSignHash =
keccak256(bytes(eip712Domain().version)),
uint256(eip712Domain().chainId),
uint256(uint160(eip712Domain().verifyingContract)),
bytes32(eip712Domain().salt),
keccak256(abi.encodePacked(eip712Domain().extensions))
bytes32(eip712Domain().salt)
)
)
)
Expand All @@ -127,23 +125,21 @@ typedDataSignTypehash =
keccak256(
abi.encodePacked(
"TypedDataSign(",
contentsTypeName, " contents",
"bytes1 fields,",
contentsName, " contents,",
"string name,",
"string version,",
"uint256 chainId,",
"address verifyingContract,",
"bytes32 salt,",
"uint256[] extensions",
"bytes32 salt"
")",
contentsType
)
);
```

If `contentsType` is `"Mail(address from,address to,string message)"`, then `contentsTypeName` will be `"Mail"`.
If `contentsType` is `"Mail(address from,address to,string message)"`, then `contentsName` will be `"Mail"`.

The `contentsTypeName` is the substring of `contentsType` up to (excluding) the first instance of `"("`:
The `contentsName` is the substring of `contentsType` up to (excluding) the first instance of `"("`:

In Solidity, this can be written as:

Expand All @@ -155,31 +151,38 @@ In Solidity, this can be written as:
// `indexOf(string memory subject, string memory search)`
// Returns the byte index of the first location of `search` in `subject`,
// searching from left to right. Returns `2**256 - 1` if `search` is not found.
contentsTypeName =
contentsName =
LibString.slice(
contentsType,
0, // Start byte index.
LibString.indexOf(contentsType, "(") // End byte index (exclusive).
);
```

A copy of the `LibString` Solidity library is provided for reference in [`/assets/eip-7739/contracts/utils/LibString.sol`].
[A copy of the `LibString` Solidity library is provided for completeness](../assets/eip-7739/contracts/utils/LibString.sol).

For safety, smart accounts MUST treat the signature as invalid if any of the following is true:
For safety, it is RECOMMENDED to treat the signature as invalid if any of the following is true:

- `contentsTypeName` is the empty string (i.e. `bytes(contentsTypeName).length == 0`).
- `contentsTypeName` starts with any of the following bytes `abcdefghijklmnopqrstuvwxyz(`.
- `contentsTypeName` contains any of the following bytes `, )\x00`.
- `contentsName` is the empty string (i.e. `bytes(contentsName).length == 0`).
- `contentsName` starts with any of the following bytes `abcdefghijklmnopqrstuvwxyz(`.
- `contentsName` contains any of the following bytes `, )\x00`.

#### `TypedDataSign` signature

The `signature` passed into `isValidSignature` will be changed to:

```
originalSignature ‖ APP_DOMAIN_SEPARATOR ‖ contents ‖ contentsType ‖ uint16(contentsType.length)
originalSignature ‖ APP_DOMAIN_SEPARATOR ‖ contents ‖ contentsDescription ‖ uint16(contentsDescription.length)
```

where `contents` is the bytes32 struct hash of the original struct.
where:

- `contents` is the bytes32 struct hash of the original struct.
- `contentsDescription` is either:
- `contentsType` (implicit mode),
where `contentsType` starts with `contentsName`.
- `contentsType ‖ contentsName` (explicit mode),
where `contentsType` may not necessarily start with `contentsName`.

In Solidity, this can be written as:

Expand All @@ -189,8 +192,8 @@ signature =
bytes(originalSignature),
bytes32(APP_DOMAIN_SEPARATOR),
bytes32(contents),
bytes(contentsType),
uint16(contentsType.length)
bytes(contentsDescription),
uint16(contentsDescription.length)
);
```

Expand Down Expand Up @@ -268,21 +271,11 @@ hash =

The `PersonalSign` workflow does not require additional data to be appended to the `signature` passed into `isValidSignature`.

### `supportsNestedTypedDataSign` function for detection
### Support detection

To facilitate automatic detection, smart accounts SHOULD implement the following function:
Smart accounts SHOULD return `bytes4(0x77390001)` for `isValidSignature(0x7739773977397739773977397739773977397739773977397739773977397739, "")` to indicate support for this standard.

```solidity
/// @dev For automatic detection that the smart account supports the nested EIP-712 workflow.
/// By default, it returns `bytes32(bytes4(keccak256("supportsNestedTypedDataSign()")))`,
/// denoting support for the default behavior, as implemented in
/// `_erc1271IsValidSignatureViaNestedEIP712`, which is called in `isValidSignature`.
/// Future extensions should return a different non-zero `result` to denote different behavior.
/// This method intentionally returns bytes32 to allow freedom for future extensions.
function supportsNestedTypedDataSign() public view virtual returns (bytes32 result) {
result = bytes4(0xd620c85a);
}
```
The magic number `bytes4(0x77390001)` MAY be incremented if this standard gets updated.

### Signature verification workflow deduction

Expand Down Expand Up @@ -322,37 +315,45 @@ As many developers may not update their applications to support the nested EIP-7

The `typedDataSignTypehash` must be constructed on-the-fly on-chain. This is to enforce that the signed contents will be visible in the signature request, by requiring that `contents` be a user defined type.

The structure is intentionally made flat with the fields of `eip712Domain` to make implementation feasible. Otherwise, smart accounts must implement on-chain lexographical sorting of strings for the struct type names when constructing `typedDataSignTypehash`.
The fields of `eip712Domain` are flattened into the `TypedDataSign` structure instead of being included as a field of type `EIP712Domain` in order to to avoid a conflict with the domain type of the verifying contract in case it's different.

### `supportsNestedTypedDataSign` for detection
The `bytes1 fields` bitmap and `uint256[] extensions` array in [ERC-5267](./eip-5267.md) have been omitted. Differentiating between an absent field versus a zero field (e.g. `bytes32(0)`) offers no additional security benefits for on-chain defensive rehashing. The `extensions` parameter is a list of EIP numbers used for off-chain signaling.

Without this function, this standard will not change the interface of the smart account, as it defines the behavior of `isValidSignature` without adding any new functions. As such, [ERC-165](./eip-165.md) cannot be used.
### `contentsDescription` with implicit and explicit modes

For future extendability, `supportsNestedTypedDataSign` is defined to return a bytes32 as the first word of its returned data. For bytecode compactness and to leave space for bit packing, only the leftmost 4 bytes are set to the function selector of `supportsNestedTypedDataSign`.
When the `contents` structure contains nested types, EIP-712 lexicographical sorting can result in the `contentsName` not being positioned exactly at the start of the `contentsType`. As such, we need the explicit mode.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the rationale behind these two modes but I feel the ERC should provide a general guideline for when to use each other. So far I've found the main benefit of using implicit mode when possible is that it reduces the calldata costs. If that's the case, I'd suggest to include it in this section

Suggested change
When the `contents` structure contains nested types, EIP-712 lexicographical sorting can result in the `contentsName` not being positioned exactly at the start of the `contentsType`. As such, we need the explicit mode.
When the `contents` structure contains nested types, EIP-712 lexicographical sorting can result in the `contentsName` not being positioned exactly at the start of the `contentsType`. As such, we need the explicit mode.
Signing UIs may use implicit mode whenever possible to reduce the calldata size. Although the `explicit` mode works as a generalized approach.


The `supportsNestedTypedDataSign` function may be extended to return multiple values (e.g. `bytes32 result, bytes memory data`), as long as the first word of the returned data is a bytes32 identifier. This will not change the function selector.
### Support detection with `isValidSignature`

For easier implementation in modular smart accounts, we have decided to utilize the `isValidSignature` method to return a magic number instead of defining new functions.

### Rejecting `contentsName` beginning with any lowercase 7-bit ASCII character

This recommendation is to keep the standard language agnostic and future-proof. Atomic types such as `uint256` may be named differently in other languages (e.g. `u256`).

## Backwards Compatibility

No backwards compatibility issues.
### Detection of previous draft

In an earlier draft, we have designated a `supportsNestedTypedDataSign()` function for support detection, which returns `bytes4(0xd620c85a)`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add something like:

"To ensure false negative detections, it is RECOMMENDED that dApps use supportsNestedTypedDataSign() as a fallback detection method in addition to the one described earlier in the spec."

to ensure dApps consider using supportsNestedTypedDataSign() and not just ignore it


## Reference Implementation

A production ready and optimized reference implementation is provided at [`/assets/eip-7739/contracts/accounts/ERC1271.sol`].
[A production ready and optimized implementation is provided for reference](../assets/eip-7739/contracts/accounts/ERC1271.sol).

It includes relevant complementary features required for safety, flexibility, developer experience, and user experience.

The reference implementation is intentionally not minimalistic. This is to avoid repeating the mistake of [ERC-1271](./eip-1271.md), where a minimalist reference implementation is wrongly assumed to be safe for production use.

## Security Considerations

### Rejecting invalid `contentsTypeName`
### Rejecting invalid `contentsName`

Current major implementations of `eth_signTypedData` do not sanitize the names of custom types.

A phishing website can craft a `contentsTypeName` with control characters to break out of the `PersonalSign` type encoding, resulting in the wallet client asking the user to sign an opaque hash.
A phishing website can craft a `contentsName` with control characters to break out of the `PersonalSign` type encoding, resulting in the wallet client asking the user to sign an opaque hash.

Requiring on-chain sanitization of `contentsTypeName` will block this phishing attack vector.
Requiring on-chain sanitization of `contentsName` will block this phishing attack vector.

## Copyright

Expand Down
Loading
Loading