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

Add EIP-712 support for Account v2 #21

Open
wants to merge 9 commits into
base: jw/account-v2
Choose a base branch
from
Open
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
85 changes: 77 additions & 8 deletions src/Account.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "erc6551/interfaces/IERC6551Account.sol";
import "erc6551/lib/ERC6551AccountLib.sol";

Expand All @@ -12,6 +11,7 @@ import "openzeppelin-contracts/token/ERC1155/IERC1155Receiver.sol";
import "openzeppelin-contracts/interfaces/IERC1271.sol";
import "openzeppelin-contracts/utils/cryptography/SignatureChecker.sol";
import "openzeppelin-contracts/proxy/utils/UUPSUpgradeable.sol";
import "openzeppelin-contracts/utils/cryptography/EIP712.sol";

import {BaseAccount as BaseERC4337Account, IEntryPoint, UserOperation} from "account-abstraction/core/BaseAccount.sol";

Expand All @@ -34,10 +34,20 @@ contract Account is
IERC721Receiver,
IERC1155Receiver,
UUPSUpgradeable,
BaseERC4337Account
BaseERC4337Account,
EIP712
{
using ECDSA for bytes32;

/// @dev EIP-712 name
string public constant NAME = "ERC6551-Account";

/// @dev EIP-712 version
string public constant VERSION = "1";

/// @dev EIP-712 Type for UserOperation
bytes32 public immutable userOperationType;

/// @dev ERC-4337 entry point address
address public immutable _entryPoint;

Expand Down Expand Up @@ -81,12 +91,18 @@ contract Account is
_;
}

constructor(address _guardian, address entryPoint_) {
if (_guardian == address(0) || entryPoint_ == address(0))
constructor(address _guardian, address entryPoint_)
EIP712(NAME, VERSION)
{
if (_guardian == address(0) || entryPoint_ == address(0)) {
revert InvalidInput();
}

_entryPoint = entryPoint_;
guardian = _guardian;
userOperationType = keccak256(
"UserOperation(address sender,uint256 nonce,bytes initCode,bytes callData,uint256 callGasLimit,uint256 verificationGasLimit,uint256 preVerificationGas,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,bytes paymasterAndData,bytes32 userOpHash)"
);
}

/// @dev allows eth transfers by default, but allows account owner to override
Expand Down Expand Up @@ -333,6 +349,43 @@ contract Account is
return this.onERC1155BatchReceived.selector;
}

/// @dev
function buildUserOp712Hash(
UserOperation memory op,
bytes32 opHash
) public view returns (bytes32 op712Hash) {
bytes32 domainSeparator = keccak256(
abi.encode(
keccak256(
"EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
),
keccak256(bytes(NAME)),
keccak256(bytes(VERSION)),
block.chainid,
op.sender
)
);
bytes32 hashStruct = keccak256(
abi.encode(
keccak256(
"UserOperation(address sender,uint256 nonce,bytes initCode,bytes callData,uint256 callGasLimit,uint256 verificationGasLimit,uint256 preVerificationGas,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,bytes paymasterAndData,bytes32 userOpHash)"
),
op.sender,
op.nonce,
op.initCode,
op.callData,
op.callGasLimit,
op.verificationGasLimit,
op.preVerificationGas,
op.maxFeePerGas,
op.maxPriorityFeePerGas,
op.paymasterAndData,
opHash
)
);
op712Hash = ECDSA.toTypedDataHash(domainSeparator, hashStruct);
}

/// @dev Contract upgrades can only be performed by the owner and the new implementation must
/// be trusted
function _authorizeUpgrade(address newImplementation)
Expand All @@ -352,10 +405,26 @@ contract Account is
UserOperation calldata userOp,
bytes32 userOpHash
) internal view override returns (uint256 validationData) {
bool isValid = this.isValidSignature(
userOpHash.toEthSignedMessageHash(),
userOp.signature
) == IERC1271.isValidSignature.selector;
UserOperation memory userOpMemory = userOp;
bytes32 hashStruct = keccak256(
abi.encode(
userOperationType,
userOpMemory.sender,
userOpMemory.nonce,
userOpMemory.initCode,
userOpMemory.callData,
userOpMemory.callGasLimit,
userOpMemory.verificationGasLimit,
userOpMemory.preVerificationGas,
userOpMemory.maxFeePerGas,
userOpMemory.maxPriorityFeePerGas,
userOpMemory.paymasterAndData,
userOpHash
)
);

bool isValid =
this.isValidSignature(_hashTypedDataV4(hashStruct), userOp.signature) == IERC1271.isValidSignature.selector;

if (isValid) {
return 0;
Expand Down
6 changes: 4 additions & 2 deletions test/Account.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,9 @@ contract AccountTest is Test {

MockAccount upgradedImplementation = new MockAccount(
address(guardian),
address(entryPoint)
address(entryPoint),
"ERC6551-Account",
"1"
);

vm.expectRevert(UntrustedImplementation.selector);
Expand All @@ -562,4 +564,4 @@ contract AccountTest is Test {

assertEq(returnValue, 12345);
}
}
}
22 changes: 7 additions & 15 deletions test/AccountERC4337.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
pragma solidity ^0.8.13;

import "forge-std/Test.sol";

import "openzeppelin-contracts/utils/cryptography/ECDSA.sol";
import "openzeppelin-contracts/token/ERC20/ERC20.sol";
import "openzeppelin-contracts/proxy/Clones.sol";
Expand Down Expand Up @@ -31,6 +30,7 @@ contract AccountERC4337Test is Test {
entryPoint = new EntryPoint();
guardian = new AccountGuardian();
implementation = new Account(address(guardian), address(entryPoint));

registry = new ERC6551Registry();

tokenCollection = new MockERC721();
Expand Down Expand Up @@ -135,12 +135,9 @@ contract AccountERC4337Test is Test {
paymasterAndData: "",
signature: ""
});

bytes32 opHash = entryPoint.getUserOpHash(op);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(
1,
opHash.toEthSignedMessageHash()
);
bytes32 op712Hash = implementation.buildUserOp712Hash(op, opHash);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(1, op712Hash);

bytes memory signature = abi.encodePacked(r, s, v);
op.signature = signature;
Expand Down Expand Up @@ -195,12 +192,9 @@ contract AccountERC4337Test is Test {
paymasterAndData: "",
signature: ""
});

bytes32 opHash = entryPoint.getUserOpHash(op);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(
1,
opHash.toEthSignedMessageHash()
);
bytes32 op712Hash = implementation.buildUserOp712Hash(op, opHash);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(1, op712Hash);

bytes memory signature = abi.encodePacked(r, s, v);
op.signature = signature;
Expand Down Expand Up @@ -257,10 +251,8 @@ contract AccountERC4337Test is Test {
});

bytes32 opHash = entryPoint.getUserOpHash(op);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(
1,
opHash.toEthSignedMessageHash()
);
bytes32 op712Hash = implementation.buildUserOp712Hash(op, opHash);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(1, op712Hash);

// invalidate signature
bytes memory signature = abi.encodePacked(r, s, v + 1);
Expand Down
2 changes: 1 addition & 1 deletion test/mocks/MockAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity ^0.8.13;
import "../../src/Account.sol";

contract MockAccount is Account {
constructor(address _guardian, address entryPoint_)
constructor(address _guardian, address entryPoint_, string memory _name, string memory _version)
Account(_guardian, entryPoint_)
{}

Expand Down