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

feat(v2): use nested eip-712 approach for isValidSignature #36

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
208 changes: 208 additions & 0 deletions ext/solady/EIP712.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;

/// @notice Contract for EIP-712 typed structured data hashing and signing.
Copy link
Contributor

Choose a reason for hiding this comment

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

Request to add the solady commit hash this was pulled from.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will probably track this in the readme or something upstack.

/// @author Solady (https://github.com/vectorized/solady/blob/main/src/utils/EIP712.sol)
/// @author Modified from Solbase (https://github.com/Sol-DAO/solbase/blob/main/src/utils/EIP712.sol)
/// @author Modified from OpenZeppelin (https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/EIP712.sol)
///
/// @dev Note, this implementation:
/// - Uses `address(this)` for the `verifyingContract` field.
/// - Does NOT use the optional EIP-712 salt.
/// - Does NOT use any EIP-712 extensions.
/// This is for simplicity and to save gas.
/// If you need to customize, please fork / modify accordingly.
abstract contract EIP712 {
/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* CONSTANTS AND IMMUTABLES */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @dev `keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)")`.
bytes32 internal constant _DOMAIN_TYPEHASH =
0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f;

uint256 private immutable _cachedThis;
uint256 private immutable _cachedChainId;
bytes32 private immutable _cachedNameHash;
bytes32 private immutable _cachedVersionHash;
bytes32 private immutable _cachedDomainSeparator;

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* CONSTRUCTOR */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @dev Cache the hashes for cheaper runtime gas costs.
/// In the case of upgradeable contracts (i.e. proxies),
/// or if the chain id changes due to a hard fork,
/// the domain separator will be seamlessly calculated on-the-fly.
constructor() {
_cachedThis = uint256(uint160(address(this)));
_cachedChainId = block.chainid;

string memory name;
string memory version;
if (!_domainNameAndVersionMayChange()) (name, version) = _domainNameAndVersion();
bytes32 nameHash = _domainNameAndVersionMayChange() ? bytes32(0) : keccak256(bytes(name));
bytes32 versionHash =
_domainNameAndVersionMayChange() ? bytes32(0) : keccak256(bytes(version));
_cachedNameHash = nameHash;
_cachedVersionHash = versionHash;

bytes32 separator;
if (!_domainNameAndVersionMayChange()) {
/// @solidity memory-safe-assembly
assembly {
let m := mload(0x40) // Load the free memory pointer.
mstore(m, _DOMAIN_TYPEHASH)
mstore(add(m, 0x20), nameHash)
mstore(add(m, 0x40), versionHash)
mstore(add(m, 0x60), chainid())
mstore(add(m, 0x80), address())
separator := keccak256(m, 0xa0)
}
}
_cachedDomainSeparator = separator;
}

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* FUNCTIONS TO OVERRIDE */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @dev Please override this function to return the domain name and version.
/// ```
/// function _domainNameAndVersion()
/// internal
/// pure
/// virtual
/// returns (string memory name, string memory version)
/// {
/// name = "Solady";
/// version = "1";
/// }
/// ```
///
/// Note: If the returned result may change after the contract has been deployed,
/// you must override `_domainNameAndVersionMayChange()` to return true.
function _domainNameAndVersion()
internal
view
virtual
returns (string memory name, string memory version);

/// @dev Returns if `_domainNameAndVersion()` may change
/// after the contract has been deployed (i.e. after the constructor).
/// Default: false.
function _domainNameAndVersionMayChange() internal pure virtual returns (bool result) {}

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* HASHING OPERATIONS */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @dev Returns the EIP-712 domain separator.
function _domainSeparator() internal view virtual returns (bytes32 separator) {
if (_domainNameAndVersionMayChange()) {
separator = _buildDomainSeparator();
} else {
separator = _cachedDomainSeparator;
if (_cachedDomainSeparatorInvalidated()) separator = _buildDomainSeparator();
}
}

/// @dev Returns the hash of the fully encoded EIP-712 message for this domain,
/// given `structHash`, as defined in
/// https://eips.ethereum.org/EIPS/eip-712#definition-of-hashstruct.
///
/// The hash can be used together with {ECDSA-recover} to obtain the signer of a message:
/// ```
/// bytes32 digest = _hashTypedData(keccak256(abi.encode(
/// keccak256("Mail(address to,string contents)"),
/// mailTo,
/// keccak256(bytes(mailContents))
/// )));
/// address signer = ECDSA.recover(digest, signature);
/// ```
function _hashTypedData(bytes32 structHash) internal view virtual returns (bytes32 digest) {
// We will use `digest` to store the domain separator to save a bit of gas.
if (_domainNameAndVersionMayChange()) {
digest = _buildDomainSeparator();
} else {
digest = _cachedDomainSeparator;
if (_cachedDomainSeparatorInvalidated()) digest = _buildDomainSeparator();
}
/// @solidity memory-safe-assembly
assembly {
// Compute the digest.
mstore(0x00, 0x1901000000000000) // Store "\x19\x01".
mstore(0x1a, digest) // Store the domain separator.
mstore(0x3a, structHash) // Store the struct hash.
digest := keccak256(0x18, 0x42)
// Restore the part of the free memory slot that was overwritten.
mstore(0x3a, 0)
}
}

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* EIP-5267 OPERATIONS */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @dev See: https://eips.ethereum.org/EIPS/eip-5267
function eip712Domain()
public
view
virtual
returns (
bytes1 fields,
string memory name,
string memory version,
uint256 chainId,
address verifyingContract,
bytes32 salt,
uint256[] memory extensions
)
{
fields = hex"0f"; // `0b01111`.
(name, version) = _domainNameAndVersion();
chainId = block.chainid;
verifyingContract = address(this);
salt = salt; // `bytes32(0)`.
extensions = extensions; // `new uint256[](0)`.
}

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* PRIVATE HELPERS */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @dev Returns the EIP-712 domain separator.
function _buildDomainSeparator() private view returns (bytes32 separator) {
// We will use `separator` to store the name hash to save a bit of gas.
bytes32 versionHash;
if (_domainNameAndVersionMayChange()) {
(string memory name, string memory version) = _domainNameAndVersion();
separator = keccak256(bytes(name));
versionHash = keccak256(bytes(version));
} else {
separator = _cachedNameHash;
versionHash = _cachedVersionHash;
}
/// @solidity memory-safe-assembly
assembly {
let m := mload(0x40) // Load the free memory pointer.
mstore(m, _DOMAIN_TYPEHASH)
mstore(add(m, 0x20), separator) // Name hash.
mstore(add(m, 0x40), versionHash)
mstore(add(m, 0x60), chainid())
mstore(add(m, 0x80), address())
separator := keccak256(m, 0xa0)
}
}

/// @dev Returns if the cached domain separator has been invalidated.
function _cachedDomainSeparatorInvalidated() private view returns (bool result) {
uint256 cachedChainId = _cachedChainId;
uint256 cachedThis = _cachedThis;
/// @solidity memory-safe-assembly
assembly {
result := iszero(and(eq(chainid(), cachedChainId), eq(address(), cachedThis)))
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

newline for consistency

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The source files don't have it and these are ported untouched - I'll keep it this way but happy to revisit later!

77 changes: 26 additions & 51 deletions src/LightAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ pragma solidity ^0.8.23;
/* solhint-disable no-inline-assembly */
/* solhint-disable reason-string */

import {IERC1271} from "@openzeppelin/contracts/interfaces/IERC1271.sol";
import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";
import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
Expand Down Expand Up @@ -47,11 +46,6 @@ contract LightAccount is BaseLightAccount, CustomSlotInitializable {
// keccak256(abi.encode(uint256(keccak256("light_account_v1.initializable")) - 1)) & ~bytes32(uint256(0xff));
bytes32 internal constant _INITIALIZABLE_STORAGE_POSITION =
0x33e4b41198cc5b8053630ed667ea7c0c4c873f7fc8d9a478b5d7259cec0a4a00;
bytes32 internal constant _DOMAIN_SEPARATOR_TYPEHASH =
keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");
bytes32 internal constant _LA_MSG_TYPEHASH = keccak256("LightAccountMessage(bytes message)");
bytes32 internal constant _NAME_HASH = keccak256("LightAccount");
bytes32 internal constant _VERSION_HASH = keccak256("1");

struct LightAccountStorage {
address owner;
Expand Down Expand Up @@ -102,48 +96,6 @@ contract LightAccount is BaseLightAccount, CustomSlotInitializable {
return _getStorage().owner;
}

/// @notice Returns the domain separator for this contract, as defined in the EIP-712 standard.
/// @return bytes32 The domain separator hash.
function domainSeparator() public view returns (bytes32) {
return keccak256(
abi.encode(
_DOMAIN_SEPARATOR_TYPEHASH,
_NAME_HASH, // name
_VERSION_HASH, // version
block.chainid, // chainId
address(this) // verifying contract
)
);
}

/// @notice Returns the pre-image of the message hash.
/// @param message Message that should be encoded.
/// @return Encoded message.
function encodeMessageData(bytes memory message) public view returns (bytes memory) {
bytes32 messageHash = keccak256(abi.encode(_LA_MSG_TYPEHASH, keccak256(message)));
return abi.encodePacked("\x19\x01", domainSeparator(), messageHash);
}

/// @notice Returns hash of a message that can be signed by owners.
/// @param message Message that should be hashed.
/// @return Message hash.
function getMessageHash(bytes memory message) public view returns (bytes32) {
return keccak256(encodeMessageData(message));
}

/// @inheritdoc IERC1271
/// @dev The signature is valid if it is signed by the owner's private key (if the owner is an EOA) or if it is a
/// valid ERC-1271 signature from the owner (if the owner is a contract). Note that unlike the signature validation
/// used in `validateUserOp`, this does **not** wrap the digest in an "Ethereum Signed Message" envelope before
/// checking the signature in the EOA-owner case.
function isValidSignature(bytes32 hash, bytes memory signature) public view override returns (bytes4) {
bytes32 messageHash = getMessageHash(abi.encode(hash));
if (SignatureChecker.isValidSignatureNow(owner(), messageHash, signature)) {
return _1271_MAGIC_VALUE;
}
return 0xffffffff;
}

function _initialize(address owner_) internal virtual {
if (owner_ == address(0)) {
revert InvalidOwner(address(0));
Expand Down Expand Up @@ -172,19 +124,42 @@ contract LightAccount is BaseLightAccount, CustomSlotInitializable {
override
returns (uint256 validationData)
{
address _owner = owner();
address owner_ = owner();
bytes32 signedHash = userOpHash.toEthSignedMessageHash();
bytes memory signature = userOp.signature;
(address recovered, ECDSA.RecoverError error,) = signedHash.tryRecover(signature);
if (
(error == ECDSA.RecoverError.NoError && recovered == _owner)
|| SignatureChecker.isValidERC1271SignatureNow(_owner, userOpHash, signature)
(error == ECDSA.RecoverError.NoError && recovered == owner_)
|| SignatureChecker.isValidERC1271SignatureNow(owner_, userOpHash, signature)
) {
return 0;
}
return SIG_VALIDATION_FAILED;
}

/// @dev The signature is valid if it is signed by the owner's private key (if the owner is an EOA) or if it is a
/// valid ERC-1271 signature from the owner (if the owner is a contract).
function _isValidSignature(bytes32 derivedHash, bytes calldata trimmedSignature)
internal
view
virtual
override
returns (bool)
{
return SignatureChecker.isValidSignatureNow(owner(), derivedHash, trimmedSignature);
}

function _domainNameAndVersion()
internal
view
virtual
override
returns (string memory name, string memory version)
{
name = "LightAccount";
version = "2";
}

function _isFromOwner() internal view virtual override returns (bool) {
return msg.sender == owner();
}
Expand Down
Loading
Loading