From 18cc6ff60ce62e4de7d080618487ad0c95c4a2d0 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Fri, 26 Apr 2024 18:53:24 +0400 Subject: [PATCH 01/21] add eip712 and eip1271 methods --- contracts/Nexus.sol | 46 ++++++++++++++++++- test/foundry/mocks/MockValidator.sol | 18 ++++---- .../TestAccountExecution_ExecuteBatch.t.sol | 10 ++++ 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/contracts/Nexus.sol b/contracts/Nexus.sol index 5e35221b..6037a3f0 100644 --- a/contracts/Nexus.sol +++ b/contracts/Nexus.sol @@ -13,6 +13,7 @@ pragma solidity ^0.8.24; // Learn more at https://biconomy.io/ import { UUPSUpgradeable } from "solady/src/utils/UUPSUpgradeable.sol"; +import { EIP712 } from "solady/src/utils/EIP712.sol"; import { PackedUserOperation } from "account-abstraction/contracts/interfaces/PackedUserOperation.sol"; import { ExecLib } from "./lib/ExecLib.sol"; @@ -33,10 +34,18 @@ import { ModeLib, ExecutionMode, ExecType, CallType, CALLTYPE_BATCH, CALLTYPE_SI /// @author @filmakarov | Biconomy | filipp.makarov@biconomy.io /// @author @zeroknots | Rhinestone.wtf | zeroknots.eth /// Special thanks to the Solady team for foundational contributions: https://github.com/Vectorized/solady -contract Nexus is INexus, BaseAccount, ExecutionHelper, ModuleManager, UUPSUpgradeable { +contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, UUPSUpgradeable { using ModeLib for ExecutionMode; using ExecLib for bytes; + /// @dev Precomputed `typeHash` used to produce EIP-712 compliant hash when applying the anti + /// cross-account-replay layer. + /// + /// The original hash must either be: + /// - An EIP-191 hash: keccak256("\x19Ethereum Signed Message:\n" || len(someMessage) || someMessage) + /// - An EIP-712 hash: keccak256("\x19\x01" || someDomainSeparator || hashStruct(someStruct)) + bytes32 private constant _MESSAGE_TYPEHASH = keccak256("BiconomyNexusMessage(bytes32 hash)"); + /// @notice Initializes the smart account by setting up the module manager and state. constructor() { _initModuleManager(); @@ -279,6 +288,22 @@ contract Nexus is INexus, BaseAccount, ExecutionHelper, ModuleManager, UUPSUpgra UUPSUpgradeable.upgradeToAndCall(newImplementation, data); } + /// @notice Wrapper around `_eip712Hash()` to produce a replay-safe hash fron the given `hash`. + /// + /// @dev The returned EIP-712 compliant replay-safe hash is the result of: + /// keccak256( + /// \x19\x01 || + /// this.domainSeparator || + /// hashStruct(CoinbaseSmartWalletMessage({ hash: `hash`})) + /// ) + /// + /// @param hash The original hash. + /// + /// @return The corresponding replay-safe hash. + function replaySafeHash(bytes32 hash) public view virtual returns (bytes32) { + return _eip712Hash(hash); + } + /// @dev Ensures that only authorized callers can upgrade the smart contract implementation. /// This is part of the UUPS (Universal Upgradeable Proxy Standard) pattern. /// @param newImplementation The address of the new implementation to upgrade to. @@ -286,6 +311,25 @@ contract Nexus is INexus, BaseAccount, ExecutionHelper, ModuleManager, UUPSUpgra newImplementation; } + /// @notice Returns the EIP-712 typed hash of the `BiconomyNexusMessage(bytes32 hash)` data structure. + /// + /// @dev Implements encode(domainSeparator : ๐”นยฒโตโถ, message : ๐•Š) = "\x19\x01" || domainSeparator || + /// hashStruct(message). + /// @dev See https://eips.ethereum.org/EIPS/eip-712#specification. + /// + /// @param hash The `BiconomyNexusMessage.hash` field to hash. + //// + /// @return The resulting EIP-712 hash. + function _eip712Hash(bytes32 hash) internal view virtual returns (bytes32) { + return + keccak256(abi.encodePacked("\x19\x01", _domainSeparator(), keccak256(abi.encode(_MESSAGE_TYPEHASH, hash)))); + } + + function _domainNameAndVersion() internal pure override returns (string memory name, string memory version) { + name = "Nexus"; + version = "0.0.1"; + } + /// @dev Executes a single transaction based on the specified execution type. /// @param executionCalldata The calldata containing the transaction details (target address, value, and data). /// @param execType The execution type, which can be DEFAULT (revert on failure) or TRY (return on failure). diff --git a/test/foundry/mocks/MockValidator.sol b/test/foundry/mocks/MockValidator.sol index 20acddee..9ba10a27 100644 --- a/test/foundry/mocks/MockValidator.sol +++ b/test/foundry/mocks/MockValidator.sol @@ -3,10 +3,10 @@ pragma solidity ^0.8.24; import { IModule } from "../../../contracts/interfaces/modules/IModule.sol"; import { IValidator } from "../../../contracts/interfaces/modules/IValidator.sol"; -import { VALIDATION_SUCCESS, VALIDATION_FAILED, MODULE_TYPE_VALIDATOR } from "../../../contracts/types/Constants.sol"; -import { EncodedModuleTypes } from "../../../contracts/lib/ModuleTypeLib.sol"; +import { VALIDATION_SUCCESS, VALIDATION_FAILED, MODULE_TYPE_VALIDATOR, ERC1271_MAGICVALUE, ERC1271_INVALID } from "../../../contracts/types/Constants.sol"; import { PackedUserOperation } from "account-abstraction/contracts/interfaces/PackedUserOperation.sol"; import { ECDSA } from "solady/src/utils/ECDSA.sol"; +import { SignatureCheckerLib } from "solady/src/utils/SignatureCheckerLib.sol"; import { MessageHashUtils } from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; contract MockValidator is IValidator{ @@ -25,18 +25,20 @@ contract MockValidator is IValidator{ } function isValidSignatureWithSender( - address sender, + address, bytes32 hash, bytes calldata signature ) external - pure + view returns (bytes4) { - sender; - hash; - signature; - return 0xffffffff; + address owner = smartAccountOwners[msg.sender]; + // MAYBE SHOULD PREPARE REPLAY RESISTANT HASH BY APPENDING MSG.SENDER + // SEE: https://github.com/bcnmy/scw-contracts/blob/3362262dab34fa0f57e2fbe0e57a4bdbd5318165/contracts/smart-account/modules/EcdsaOwnershipRegistryModule.sol#L122-L132 + // OR USE EIP-712 + return + SignatureCheckerLib.isValidSignatureNowCalldata(owner, hash, signature) ? ERC1271_MAGICVALUE : ERC1271_INVALID; } function onInstall(bytes calldata data) external { diff --git a/test/foundry/unit/concrete/accountexecution/TestAccountExecution_ExecuteBatch.t.sol b/test/foundry/unit/concrete/accountexecution/TestAccountExecution_ExecuteBatch.t.sol index 2dee7269..f04be833 100644 --- a/test/foundry/unit/concrete/accountexecution/TestAccountExecution_ExecuteBatch.t.sol +++ b/test/foundry/unit/concrete/accountexecution/TestAccountExecution_ExecuteBatch.t.sol @@ -141,6 +141,16 @@ contract TestAccountExecution_ExecuteBatch is TestAccountExecution_Base { assertEq(aliceBalanceAfter, aliceBalanceBefore + transferAmount, "Alice should receive tokens via transferFrom"); } + function test_isValidSignature_MockValidator() public { + string memory message = "Some Message"; + bytes32 hash = keccak256(abi.encodePacked(message)); + bytes32 toSign = ALICE_ACCOUNT.replaySafeHash(hash); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(ALICE.privateKey, toSign); + bytes memory signature = abi.encodePacked(r, s, v); + bytes4 ret = ALICE_ACCOUNT.isValidSignature(hash, abi.encodePacked(address(VALIDATOR_MODULE), signature)); + assertEq(ret, bytes4(0x1626ba7e)); + } + function test_ExecuteBatch_ApproveAndTransfer_SingleOp() public { uint256 approvalAmount = 1000 * 10 ** token.decimals(); uint256 transferAmount = 500 * 10 ** token.decimals(); From 6523994a3eab66456349f7fbaba77b387bdbd889 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Sat, 27 Apr 2024 01:41:38 +0400 Subject: [PATCH 02/21] fix sig verification problem --- contracts/Nexus.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/Nexus.sol b/contracts/Nexus.sol index 6037a3f0..ad82cc61 100644 --- a/contracts/Nexus.sol +++ b/contracts/Nexus.sol @@ -229,7 +229,7 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U function isValidSignature(bytes32 hash, bytes calldata data) external view virtual override returns (bytes4) { address validator = address(bytes20(data[0:20])); if (!_isValidatorInstalled(validator)) revert InvalidModule(validator); - return IValidator(validator).isValidSignatureWithSender(msg.sender, hash, data[20:]); + return IValidator(validator).isValidSignatureWithSender(msg.sender, _eip712Hash(hash), data[20:]); } /// @notice Retrieves the address of the current implementation from the EIP-1967 slot. From c7480de6ea35490338772c9def69e64774526eda Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Sat, 27 Apr 2024 01:42:22 +0400 Subject: [PATCH 03/21] lint --- contracts/Nexus.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/Nexus.sol b/contracts/Nexus.sol index ad82cc61..ee7dd69c 100644 --- a/contracts/Nexus.sol +++ b/contracts/Nexus.sol @@ -321,8 +321,7 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U //// /// @return The resulting EIP-712 hash. function _eip712Hash(bytes32 hash) internal view virtual returns (bytes32) { - return - keccak256(abi.encodePacked("\x19\x01", _domainSeparator(), keccak256(abi.encode(_MESSAGE_TYPEHASH, hash)))); + return keccak256(abi.encodePacked("\x19\x01", _domainSeparator(), keccak256(abi.encode(_MESSAGE_TYPEHASH, hash)))); } function _domainNameAndVersion() internal pure override returns (string memory name, string memory version) { From 6f3938bf1229459cd856fd1fac68e72b6a004f56 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Mon, 29 Apr 2024 01:10:34 +0400 Subject: [PATCH 04/21] move 1271 tests to separate file + negative test --- .../TestAccountExecution_ExecuteBatch.t.sol | 10 ------ .../TestERC1271Account_IsValidSignature.sol | 35 +++++++++++++++++++ 2 files changed, 35 insertions(+), 10 deletions(-) create mode 100644 test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol diff --git a/test/foundry/unit/concrete/accountexecution/TestAccountExecution_ExecuteBatch.t.sol b/test/foundry/unit/concrete/accountexecution/TestAccountExecution_ExecuteBatch.t.sol index f04be833..2dee7269 100644 --- a/test/foundry/unit/concrete/accountexecution/TestAccountExecution_ExecuteBatch.t.sol +++ b/test/foundry/unit/concrete/accountexecution/TestAccountExecution_ExecuteBatch.t.sol @@ -141,16 +141,6 @@ contract TestAccountExecution_ExecuteBatch is TestAccountExecution_Base { assertEq(aliceBalanceAfter, aliceBalanceBefore + transferAmount, "Alice should receive tokens via transferFrom"); } - function test_isValidSignature_MockValidator() public { - string memory message = "Some Message"; - bytes32 hash = keccak256(abi.encodePacked(message)); - bytes32 toSign = ALICE_ACCOUNT.replaySafeHash(hash); - (uint8 v, bytes32 r, bytes32 s) = vm.sign(ALICE.privateKey, toSign); - bytes memory signature = abi.encodePacked(r, s, v); - bytes4 ret = ALICE_ACCOUNT.isValidSignature(hash, abi.encodePacked(address(VALIDATOR_MODULE), signature)); - assertEq(ret, bytes4(0x1626ba7e)); - } - function test_ExecuteBatch_ApproveAndTransfer_SingleOp() public { uint256 approvalAmount = 1000 * 10 ** token.decimals(); uint256 transferAmount = 500 * 10 ** token.decimals(); diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol new file mode 100644 index 00000000..8ab7fe4a --- /dev/null +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import "../../../utils/Imports.sol"; +import "../../../utils/SmartAccountTestLab.t.sol"; + +contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { + + function setUp() public { + init(); + } + + function test_isValidSignature_MockValidator_Success() public { + string memory message = "Some Message"; + bytes32 hash = keccak256(abi.encodePacked(message)); + bytes32 toSign = ALICE_ACCOUNT.replaySafeHash(hash); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(ALICE.privateKey, toSign); + bytes memory signature = abi.encodePacked(r, s, v); + bytes4 ret = ALICE_ACCOUNT.isValidSignature(hash, abi.encodePacked(address(VALIDATOR_MODULE), signature)); + assertEq(ret, bytes4(0x1626ba7e)); + } + + function test_isValidSignature_MockValidator_Wrong1271Signer_Fail() public { + string memory message = "Some Message"; + bytes32 hash = keccak256(abi.encodePacked(message)); + bytes32 toSign = ALICE_ACCOUNT.replaySafeHash(hash); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(BOB.privateKey, toSign); + bytes memory signature = abi.encodePacked(r, s, v); + bytes4 ret = ALICE_ACCOUNT.isValidSignature(hash, abi.encodePacked(address(VALIDATOR_MODULE), signature)); + assertEq(ret, bytes4(0xFFFFFFFF)); + } + + // @ TODO + // other test scenarios +} \ No newline at end of file From 22616270d06a7f2a65638345993925d777a86143 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Mon, 29 Apr 2024 13:58:01 +0400 Subject: [PATCH 05/21] respond to review comments --- contracts/Nexus.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/Nexus.sol b/contracts/Nexus.sol index ee7dd69c..75db7608 100644 --- a/contracts/Nexus.sol +++ b/contracts/Nexus.sol @@ -294,7 +294,7 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U /// keccak256( /// \x19\x01 || /// this.domainSeparator || - /// hashStruct(CoinbaseSmartWalletMessage({ hash: `hash`})) + /// hashStruct(BiconomyNexusMessage({ hash: `hash`})) /// ) /// /// @param hash The original hash. From a8f9df9628bd7d4bc53da4fd4f722d0bab09d587 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Tue, 30 Apr 2024 02:12:46 +0400 Subject: [PATCH 06/21] v3 trial --- contracts/Nexus.sol | 105 +++++++++++++++++- .../TestERC1271Account_IsValidSignature.sol | 104 ++++++++++++++--- 2 files changed, 194 insertions(+), 15 deletions(-) diff --git a/contracts/Nexus.sol b/contracts/Nexus.sol index 75db7608..50bc5263 100644 --- a/contracts/Nexus.sol +++ b/contracts/Nexus.sol @@ -46,6 +46,10 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U /// - An EIP-712 hash: keccak256("\x19\x01" || someDomainSeparator || hashStruct(someStruct)) bytes32 private constant _MESSAGE_TYPEHASH = keccak256("BiconomyNexusMessage(bytes32 hash)"); + /// @dev `keccak256("PersonalSign(bytes prefixed)")`. + bytes32 internal constant _PERSONAL_SIGN_TYPEHASH = + 0x983e65e5148e570cd828ead231ee759a8d7958721a768f93bc4483ba005c32de; + /// @notice Initializes the smart account by setting up the module manager and state. constructor() { _initModuleManager(); @@ -229,7 +233,7 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U function isValidSignature(bytes32 hash, bytes calldata data) external view virtual override returns (bytes4) { address validator = address(bytes20(data[0:20])); if (!_isValidatorInstalled(validator)) revert InvalidModule(validator); - return IValidator(validator).isValidSignatureWithSender(msg.sender, _eip712Hash(hash), data[20:]); + return IValidator(validator).isValidSignatureWithSender(msg.sender, _erc1271HashForIsValidSignatureViaNestedEIP712(hash, data[20:]), data[20:]); } /// @notice Retrieves the address of the current implementation from the EIP-1967 slot. @@ -324,11 +328,84 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U return keccak256(abi.encodePacked("\x19\x01", _domainSeparator(), keccak256(abi.encode(_MESSAGE_TYPEHASH, hash)))); } + function _erc1271HashForIsValidSignatureViaNestedEIP712(bytes32 hash, bytes calldata signature) + internal + view + virtual + returns (bytes32) + { + bool result; + bytes32 t = _typedDataSignFields(); + /// @solidity memory-safe-assembly + assembly { + let m := mload(0x40) // Cache the free memory pointer. + // Length of the contents type. + let c := and(0xffff, calldataload(add(signature.offset, sub(signature.length, 0x20)))) + for {} 1 {} { + let l := add(0x42, c) // Total length of appended data (32 + 32 + c + 2). + let o := add(signature.offset, sub(signature.length, l)) + calldatacopy(0x20, o, 0x40) // Copy the `DOMAIN_SEP_B` and contents struct hash. + mstore(0x00, 0x1901) // Store the "\x19\x01" prefix. + // Use the `PersonalSign` workflow if the reconstructed contents hash doesn't match, + // or if the appended data is invalid (length too long, or empty contents type). + if or(xor(keccak256(0x1e, 0x42), hash), or(lt(signature.length, l), iszero(c))) { + mstore(0x00, _PERSONAL_SIGN_TYPEHASH) + mstore(0x20, hash) // Store the `prefixed`. + hash := keccak256(0x00, 0x40) // Compute the `PersonalSign` struct hash. + break + } + // Else, use the `TypedDataSign` workflow. + // Construct `TYPED_DATA_SIGN_TYPEHASH` on-the-fly. + mstore(m, "TypedDataSign(bytes32 hash,") + let p := add(m, 0x1b) // Advance 27 bytes. + calldatacopy(p, add(o, 0x40), c) // Copy the contents type. + // Whether the contents name is invalid. Starts with lowercase or '('. + let d := byte(0, mload(p)) + d := or(gt(26, sub(d, 97)), eq(d, 40)) + // Store the end sentinel '(', and advance `p` until we encounter a '(' byte. + for { mstore(add(p, c), 40) } 1 { p := add(p, 1) } { + let b := byte(0, mload(p)) + if eq(b, 40) { break } + d := or(d, and(1, shr(b, 0x120100000001))) // Has a byte in ", )\x00". + } + mstore(p, " contents,bytes1 fields,string n") + mstore(add(p, 0x20), "ame,string version,uint256 chain") + mstore(add(p, 0x40), "Id,address verifyingContract,byt") + mstore(add(p, 0x60), "es32 salt,uint256[] extensions)") + p := add(p, 0x7f) // Advance 127 bytes. + calldatacopy(p, add(o, 0x40), c) // Copy the contents type. + // Fill in the missing fields of the `TypedDataSign`. + mstore(t, keccak256(m, sub(add(p, c), m))) // `TYPED_DATA_SIGN_TYPEHASH`. + mstore(add(t, 0x20), hash) // `hash`. + mstore(add(t, 0x40), calldataload(add(o, 0x20))) // `contents`. + // The "\x19\x01" prefix is already at 0x00. + // `DOMAIN_SEP_B` is already at 0x20. + mstore(0x40, keccak256(t, 0x140)) // `hashStruct(typedDataSign)`. + // Compute the final hash, corrupted if the contents name is invalid. + hash := keccak256(0x1e, add(0x42, d)) + result := 1 // Use `result` to temporarily denote if we will use `DOMAIN_SEP_B`. + signature.length := sub(signature.length, l) // Truncate the signature. + break + } + mstore(0x40, m) // Restore the free memory pointer. + } + if (!result) hash = _hashTypedData(hash); + return hash; + } + function _domainNameAndVersion() internal pure override returns (string memory name, string memory version) { name = "Nexus"; version = "0.0.1"; } + function hashTypedData(bytes32 structHash) external view returns (bytes32) { + return _hashTypedData(structHash); + } + + function DOMAIN_SEPARATOR() external view returns (bytes32) { + return _domainSeparator(); + } + /// @dev Executes a single transaction based on the specified execution type. /// @param executionCalldata The calldata containing the transaction details (target address, value, and data). /// @param execType The execution type, which can be DEFAULT (revert on failure) or TRY (return on failure). @@ -362,4 +439,30 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U else if (moduleTypeId == MODULE_TYPE_HOOK) return _isHookInstalled(module); else return false; } + + /// @dev For use in `_erc1271HashForIsValidSignatureViaNestedEIP712`, + function _typedDataSignFields() private view returns (bytes32 m) { + ( + bytes1 fields, + string memory name, + string memory version, + uint256 chainId, + address verifyingContract, + bytes32 salt, + uint256[] memory extensions + ) = eip712Domain(); + /// @solidity memory-safe-assembly + assembly { + m := mload(0x40) // Grab the free memory pointer. + mstore(0x40, add(m, 0x140)) // Allocate the memory. + // Skip 3 words: `TYPED_DATA_SIGN_TYPEHASH, hash, contents`. + mstore(add(m, 0x60), shl(248, byte(0, fields))) + mstore(add(m, 0x80), keccak256(add(name, 0x20), mload(name))) + mstore(add(m, 0xa0), keccak256(add(version, 0x20), mload(version))) + mstore(add(m, 0xc0), chainId) + mstore(add(m, 0xe0), shr(96, shl(96, verifyingContract))) + mstore(add(m, 0x100), salt) + mstore(add(m, 0x120), keccak256(add(extensions, 0x20), shl(5, mload(extensions)))) + } + } } diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol index 8ab7fe4a..70a80993 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol @@ -6,28 +6,104 @@ import "../../../utils/SmartAccountTestLab.t.sol"; contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { + struct _TestTemps { + bytes32 userOpHash; + bytes32 contents; + address signer; + uint256 privateKey; + uint8 v; + bytes32 r; + bytes32 s; + uint256 missingAccountFunds; + } + + + bytes32 internal constant _PARENT_TYPEHASH = + 0xd61db970ec8a2edc5f9fd31d876abe01b785909acb16dcd4baaf3b434b4c439b; + + bytes32 internal constant _DOMAIN_SEP_B = + 0xa1a044077d7677adbbfa892ded5390979b33993e0e2a457e3f974bbcda53821b; + function setUp() public { init(); } function test_isValidSignature_MockValidator_Success() public { - string memory message = "Some Message"; - bytes32 hash = keccak256(abi.encodePacked(message)); - bytes32 toSign = ALICE_ACCOUNT.replaySafeHash(hash); - (uint8 v, bytes32 r, bytes32 s) = vm.sign(ALICE.privateKey, toSign); - bytes memory signature = abi.encodePacked(r, s, v); - bytes4 ret = ALICE_ACCOUNT.isValidSignature(hash, abi.encodePacked(address(VALIDATOR_MODULE), signature)); + _TestTemps memory t; + t.contents = keccak256("123"); + (t.v, t.r, t.s) = vm.sign(ALICE.privateKey, _toERC1271Hash(t.contents)); + bytes memory contentsType = "Contents(bytes32 stuff)"; + bytes memory signature = abi.encodePacked( + t.r, t.s, t.v, _DOMAIN_SEP_B, t.contents, contentsType, uint16(contentsType.length) + ); + bytes4 ret = ALICE_ACCOUNT.isValidSignature(_toContentsHash(t.contents), abi.encodePacked(address(VALIDATOR_MODULE), signature)); assertEq(ret, bytes4(0x1626ba7e)); + + // unchecked { + // uint256 vs = uint256(t.s) | uint256(t.v - 27) << 255; + // signature = abi.encodePacked( + // t.r, vs, _DOMAIN_SEP_B, t.contents, contentsType, uint16(contentsType.length) + // ); + // assertEq( + // ALICE_ACCOUNT.isValidSignature(_toContentsHash(t.contents), abi.encodePacked(address(VALIDATOR_MODULE), signature)), bytes4(0x1626ba7e) + // ); + // } + } + + // function test_isValidSignature_MockValidator_Wrong1271Signer_Fail() public { + // string memory message = "Some Message"; + // bytes32 hash = keccak256(abi.encodePacked(message)); + // bytes32 toSign = ALICE_ACCOUNT.replaySafeHash(hash); + // (uint8 v, bytes32 r, bytes32 s) = vm.sign(BOB.privateKey, toSign); + // bytes memory signature = abi.encodePacked(r, s, v); + // bytes4 ret = ALICE_ACCOUNT.isValidSignature(hash, abi.encodePacked(address(VALIDATOR_MODULE), signature)); + // assertEq(ret, bytes4(0xFFFFFFFF)); + // } + + function _toERC1271Hash(bytes32 contents) internal view returns (bytes32) { + bytes32 parentStructHash = keccak256( + abi.encodePacked( + abi.encode( + keccak256( + "TypedDataSign(bytes32 hash,Contents contents,bytes1 fields,string name,string version,uint256 chainId,address verifyingContract,bytes32 salt,uint256[] extensions)Contents(bytes32 stuff)" + ), + _toContentsHash(contents), + contents + ), + _accountDomainStructFields() + ) + ); + return keccak256(abi.encodePacked("\x19\x01", _DOMAIN_SEP_B, parentStructHash)); + } + + function _toContentsHash(bytes32 contents) internal pure returns (bytes32) { + return keccak256(abi.encodePacked(hex"1901", _DOMAIN_SEP_B, contents)); } - function test_isValidSignature_MockValidator_Wrong1271Signer_Fail() public { - string memory message = "Some Message"; - bytes32 hash = keccak256(abi.encodePacked(message)); - bytes32 toSign = ALICE_ACCOUNT.replaySafeHash(hash); - (uint8 v, bytes32 r, bytes32 s) = vm.sign(BOB.privateKey, toSign); - bytes memory signature = abi.encodePacked(r, s, v); - bytes4 ret = ALICE_ACCOUNT.isValidSignature(hash, abi.encodePacked(address(VALIDATOR_MODULE), signature)); - assertEq(ret, bytes4(0xFFFFFFFF)); + struct _AccountDomainStruct { + bytes1 fields; + string name; + string version; + uint256 chainId; + address verifyingContract; + bytes32 salt; + uint256[] extensions; + } + + function _accountDomainStructFields() internal view returns (bytes memory) { + _AccountDomainStruct memory t; + (t.fields, t.name, t.version, t.chainId, t.verifyingContract, t.salt, t.extensions) = + ALICE_ACCOUNT.eip712Domain(); + + return abi.encode( + t.fields, + keccak256(bytes(t.name)), + keccak256(bytes(t.version)), + t.chainId, + t.verifyingContract, + t.salt, + keccak256(abi.encodePacked(t.extensions)) + ); } // @ TODO From 2ae4bbaa57c57e9249bea7b1a6515f2190bf8e09 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Tue, 30 Apr 2024 12:57:43 +0400 Subject: [PATCH 07/21] test for personal sign --- .../TestERC1271Account_IsValidSignature.sol | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol index 70a80993..e2fd7eab 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol @@ -24,11 +24,22 @@ contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { bytes32 internal constant _DOMAIN_SEP_B = 0xa1a044077d7677adbbfa892ded5390979b33993e0e2a457e3f974bbcda53821b; + bytes32 internal constant _DOMAIN_TYPEHASH = + 0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f; + function setUp() public { init(); } - function test_isValidSignature_MockValidator_Success() public { + function test_isValidSignature_PersonalSign_MockValidator_Success() public { + _TestTemps memory t; + t.contents = keccak256("123"); + (t.v, t.r, t.s) = vm.sign(ALICE.privateKey, _toERC1271HashPersonalSign(t.contents)); + bytes memory signature = abi.encodePacked(t.r, t.s, t.v); + assertEq(ALICE_ACCOUNT.isValidSignature(t.contents, abi.encodePacked(address(VALIDATOR_MODULE), signature)), bytes4(0x1626ba7e)); + } + + function test_isValidSignature_EIP712Sign_MockValidator_Success() public { _TestTemps memory t; t.contents = keccak256("123"); (t.v, t.r, t.s) = vm.sign(ALICE.privateKey, _toERC1271Hash(t.contents)); @@ -80,6 +91,23 @@ contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { return keccak256(abi.encodePacked(hex"1901", _DOMAIN_SEP_B, contents)); } + function _toERC1271HashPersonalSign(bytes32 childHash) internal view returns (bytes32) { + bytes32 domainSeparator = keccak256( + abi.encode( + keccak256( + "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" + ), + keccak256("Nexus"), + keccak256("0.0.1"), + block.chainid, + address(ALICE_ACCOUNT) + ) + ); + bytes32 parentStructHash = + keccak256(abi.encode(keccak256("PersonalSign(bytes prefixed)"), childHash)); + return keccak256(abi.encodePacked("\x19\x01", domainSeparator, parentStructHash)); + } + struct _AccountDomainStruct { bytes1 fields; string name; From 79f4c55a5359788df6cdcc15e1aab8303e8eea69 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Tue, 30 Apr 2024 16:33:36 +0400 Subject: [PATCH 08/21] update test --- .../erc1271/TestERC1271Account_IsValidSignature.sol | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol index e2fd7eab..d78cdd27 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol @@ -37,6 +37,12 @@ contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { (t.v, t.r, t.s) = vm.sign(ALICE.privateKey, _toERC1271HashPersonalSign(t.contents)); bytes memory signature = abi.encodePacked(t.r, t.s, t.v); assertEq(ALICE_ACCOUNT.isValidSignature(t.contents, abi.encodePacked(address(VALIDATOR_MODULE), signature)), bytes4(0x1626ba7e)); + + unchecked { + uint256 vs = uint256(t.s) | uint256(t.v - 27) << 255; + signature = abi.encodePacked(t.r, vs); + assertEq(ALICE_ACCOUNT.isValidSignature(t.contents, abi.encodePacked(address(VALIDATOR_MODULE), signature)), bytes4(0x1626ba7e)); + } } function test_isValidSignature_EIP712Sign_MockValidator_Success() public { From baba8f791896c4d2d54ad069f41dfa8215600817 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 1 May 2024 17:12:25 +0400 Subject: [PATCH 09/21] validator: fix signature verification issue --- contracts/Nexus.sol | 57 +++++++++---------- .../TestERC1271Account_IsValidSignature.sol | 3 +- 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/contracts/Nexus.sol b/contracts/Nexus.sol index 50bc5263..fe19afc2 100644 --- a/contracts/Nexus.sol +++ b/contracts/Nexus.sol @@ -233,7 +233,8 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U function isValidSignature(bytes32 hash, bytes calldata data) external view virtual override returns (bytes4) { address validator = address(bytes20(data[0:20])); if (!_isValidatorInstalled(validator)) revert InvalidModule(validator); - return IValidator(validator).isValidSignatureWithSender(msg.sender, _erc1271HashForIsValidSignatureViaNestedEIP712(hash, data[20:]), data[20:]); + (bytes32 computeHash, bytes calldata truncatedSignature) = _erc1271HashForIsValidSignatureViaNestedEIP712(hash, data[20:]); + return IValidator(validator).isValidSignatureWithSender(msg.sender, computeHash, truncatedSignature); } /// @notice Retrieves the address of the current implementation from the EIP-1967 slot. @@ -332,10 +333,10 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U internal view virtual - returns (bytes32) + returns (bytes32, bytes calldata) { - bool result; - bytes32 t = _typedDataSignFields(); + bool result; + bytes32 t = _typedDataSignFields(); /// @solidity memory-safe-assembly assembly { let m := mload(0x40) // Cache the free memory pointer. @@ -355,34 +356,30 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U break } // Else, use the `TypedDataSign` workflow. - // Construct `TYPED_DATA_SIGN_TYPEHASH` on-the-fly. - mstore(m, "TypedDataSign(bytes32 hash,") - let p := add(m, 0x1b) // Advance 27 bytes. + mstore(m, "TypedDataSign(") // To construct `TYPED_DATA_SIGN_TYPEHASH` on-the-fly. + let p := add(m, 0x0e) // Advance 14 bytes. calldatacopy(p, add(o, 0x40), c) // Copy the contents type. - // Whether the contents name is invalid. Starts with lowercase or '('. - let d := byte(0, mload(p)) - d := or(gt(26, sub(d, 97)), eq(d, 40)) + let d := byte(0, mload(p)) // For denoting if the contents name is invalid. + d := or(gt(26, sub(d, 97)), eq(40, d)) // Starts with lowercase or '('. // Store the end sentinel '(', and advance `p` until we encounter a '(' byte. for { mstore(add(p, c), 40) } 1 { p := add(p, 1) } { let b := byte(0, mload(p)) - if eq(b, 40) { break } - d := or(d, and(1, shr(b, 0x120100000001))) // Has a byte in ", )\x00". + if eq(40, b) { break } + d := or(d, shr(b, 0x120100000001)) // Has a byte in ", )\x00". } mstore(p, " contents,bytes1 fields,string n") mstore(add(p, 0x20), "ame,string version,uint256 chain") mstore(add(p, 0x40), "Id,address verifyingContract,byt") mstore(add(p, 0x60), "es32 salt,uint256[] extensions)") - p := add(p, 0x7f) // Advance 127 bytes. - calldatacopy(p, add(o, 0x40), c) // Copy the contents type. + calldatacopy(add(p, 0x7f), add(o, 0x40), c) // Copy the contents type. // Fill in the missing fields of the `TypedDataSign`. - mstore(t, keccak256(m, sub(add(p, c), m))) // `TYPED_DATA_SIGN_TYPEHASH`. - mstore(add(t, 0x20), hash) // `hash`. - mstore(add(t, 0x40), calldataload(add(o, 0x20))) // `contents`. + calldatacopy(t, o, 0x40) // Copy `contents` to `add(t, 0x20)`. + mstore(t, keccak256(m, sub(add(add(p, 0x7f), c), m))) // `TYPED_DATA_SIGN_TYPEHASH`. // The "\x19\x01" prefix is already at 0x00. // `DOMAIN_SEP_B` is already at 0x20. - mstore(0x40, keccak256(t, 0x140)) // `hashStruct(typedDataSign)`. + mstore(0x40, keccak256(t, 0x120)) // `hashStruct(typedDataSign)`. // Compute the final hash, corrupted if the contents name is invalid. - hash := keccak256(0x1e, add(0x42, d)) + hash := keccak256(0x1e, add(0x42, and(1, d))) result := 1 // Use `result` to temporarily denote if we will use `DOMAIN_SEP_B`. signature.length := sub(signature.length, l) // Truncate the signature. break @@ -390,7 +387,7 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U mstore(0x40, m) // Restore the free memory pointer. } if (!result) hash = _hashTypedData(hash); - return hash; + return (hash, signature); } function _domainNameAndVersion() internal pure override returns (string memory name, string memory version) { @@ -440,7 +437,7 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U else return false; } - /// @dev For use in `_erc1271HashForIsValidSignatureViaNestedEIP712`, + /// @dev For use in `_erc1271IsValidSignatureViaNestedEIP712`, function _typedDataSignFields() private view returns (bytes32 m) { ( bytes1 fields, @@ -454,15 +451,15 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U /// @solidity memory-safe-assembly assembly { m := mload(0x40) // Grab the free memory pointer. - mstore(0x40, add(m, 0x140)) // Allocate the memory. - // Skip 3 words: `TYPED_DATA_SIGN_TYPEHASH, hash, contents`. - mstore(add(m, 0x60), shl(248, byte(0, fields))) - mstore(add(m, 0x80), keccak256(add(name, 0x20), mload(name))) - mstore(add(m, 0xa0), keccak256(add(version, 0x20), mload(version))) - mstore(add(m, 0xc0), chainId) - mstore(add(m, 0xe0), shr(96, shl(96, verifyingContract))) - mstore(add(m, 0x100), salt) - mstore(add(m, 0x120), keccak256(add(extensions, 0x20), shl(5, mload(extensions)))) + mstore(0x40, add(m, 0x120)) // Allocate the memory. + // Skip 2 words: `TYPED_DATA_SIGN_TYPEHASH, contents`. + mstore(add(m, 0x40), shl(248, byte(0, fields))) + mstore(add(m, 0x60), keccak256(add(name, 0x20), mload(name))) + mstore(add(m, 0x80), keccak256(add(version, 0x20), mload(version))) + mstore(add(m, 0xa0), chainId) + mstore(add(m, 0xc0), shr(96, shl(96, verifyingContract))) + mstore(add(m, 0xe0), salt) + mstore(add(m, 0x100), keccak256(add(extensions, 0x20), shl(5, mload(extensions)))) } } } diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol index d78cdd27..f031fa2f 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol @@ -82,9 +82,8 @@ contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { abi.encodePacked( abi.encode( keccak256( - "TypedDataSign(bytes32 hash,Contents contents,bytes1 fields,string name,string version,uint256 chainId,address verifyingContract,bytes32 salt,uint256[] extensions)Contents(bytes32 stuff)" + "TypedDataSign(Contents contents,bytes1 fields,string name,string version,uint256 chainId,address verifyingContract,bytes32 salt,uint256[] extensions)Contents(bytes32 stuff)" ), - _toContentsHash(contents), contents ), _accountDomainStructFields() From b6c38427b7084894fb04f47746e66279c8d2996a Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 1 May 2024 17:29:07 +0400 Subject: [PATCH 10/21] clean tests --- .../TestERC1271Account_IsValidSignature.sol | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol index f031fa2f..e41bd45f 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol @@ -48,7 +48,7 @@ contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { function test_isValidSignature_EIP712Sign_MockValidator_Success() public { _TestTemps memory t; t.contents = keccak256("123"); - (t.v, t.r, t.s) = vm.sign(ALICE.privateKey, _toERC1271Hash(t.contents)); + (t.v, t.r, t.s) = vm.sign(ALICE.privateKey, _toERC1271Hash(t.contents, payable(address(ALICE_ACCOUNT)))); bytes memory contentsType = "Contents(bytes32 stuff)"; bytes memory signature = abi.encodePacked( t.r, t.s, t.v, _DOMAIN_SEP_B, t.contents, contentsType, uint16(contentsType.length) @@ -56,28 +56,30 @@ contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { bytes4 ret = ALICE_ACCOUNT.isValidSignature(_toContentsHash(t.contents), abi.encodePacked(address(VALIDATOR_MODULE), signature)); assertEq(ret, bytes4(0x1626ba7e)); - // unchecked { - // uint256 vs = uint256(t.s) | uint256(t.v - 27) << 255; - // signature = abi.encodePacked( - // t.r, vs, _DOMAIN_SEP_B, t.contents, contentsType, uint16(contentsType.length) - // ); - // assertEq( - // ALICE_ACCOUNT.isValidSignature(_toContentsHash(t.contents), abi.encodePacked(address(VALIDATOR_MODULE), signature)), bytes4(0x1626ba7e) - // ); - // } + unchecked { + uint256 vs = uint256(t.s) | uint256(t.v - 27) << 255; + signature = abi.encodePacked( + t.r, vs, _DOMAIN_SEP_B, t.contents, contentsType, uint16(contentsType.length) + ); + assertEq( + ALICE_ACCOUNT.isValidSignature(_toContentsHash(t.contents), abi.encodePacked(address(VALIDATOR_MODULE), signature)), bytes4(0x1626ba7e) + ); + } + } + + function test_isValidSignature_EIP712Sign_MockValidator_Wrong1271Signer_Fail() public { + _TestTemps memory t; + t.contents = keccak256("123"); + (t.v, t.r, t.s) = vm.sign(BOB.privateKey, _toERC1271Hash(t.contents, payable(address(ALICE_ACCOUNT)))); + bytes memory contentsType = "Contents(bytes32 stuff)"; + bytes memory signature = abi.encodePacked( + t.r, t.s, t.v, _DOMAIN_SEP_B, t.contents, contentsType, uint16(contentsType.length) + ); + bytes4 ret = ALICE_ACCOUNT.isValidSignature(_toContentsHash(t.contents), abi.encodePacked(address(VALIDATOR_MODULE), signature)); + assertEq(ret, bytes4(0xFFFFFFFF)); } - // function test_isValidSignature_MockValidator_Wrong1271Signer_Fail() public { - // string memory message = "Some Message"; - // bytes32 hash = keccak256(abi.encodePacked(message)); - // bytes32 toSign = ALICE_ACCOUNT.replaySafeHash(hash); - // (uint8 v, bytes32 r, bytes32 s) = vm.sign(BOB.privateKey, toSign); - // bytes memory signature = abi.encodePacked(r, s, v); - // bytes4 ret = ALICE_ACCOUNT.isValidSignature(hash, abi.encodePacked(address(VALIDATOR_MODULE), signature)); - // assertEq(ret, bytes4(0xFFFFFFFF)); - // } - - function _toERC1271Hash(bytes32 contents) internal view returns (bytes32) { + function _toERC1271Hash(bytes32 contents, address payable account) internal view returns (bytes32) { bytes32 parentStructHash = keccak256( abi.encodePacked( abi.encode( @@ -86,7 +88,7 @@ contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { ), contents ), - _accountDomainStructFields() + _accountDomainStructFields(account) ) ); return keccak256(abi.encodePacked("\x19\x01", _DOMAIN_SEP_B, parentStructHash)); @@ -123,10 +125,10 @@ contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { uint256[] extensions; } - function _accountDomainStructFields() internal view returns (bytes memory) { + function _accountDomainStructFields(address payable account) internal view returns (bytes memory) { _AccountDomainStruct memory t; (t.fields, t.name, t.version, t.chainId, t.verifyingContract, t.salt, t.extensions) = - ALICE_ACCOUNT.eip712Domain(); + Nexus(account).eip712Domain(); return abi.encode( t.fields, From b135ca2b43e0e0a028820ee92c7e80f2024b181d Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 1 May 2024 18:31:30 +0400 Subject: [PATCH 11/21] cleanup --- .../concrete/erc1271/TestERC1271Account_IsValidSignature.sol | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol index e41bd45f..13a57ef5 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol @@ -24,9 +24,6 @@ contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { bytes32 internal constant _DOMAIN_SEP_B = 0xa1a044077d7677adbbfa892ded5390979b33993e0e2a457e3f974bbcda53821b; - bytes32 internal constant _DOMAIN_TYPEHASH = - 0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f; - function setUp() public { init(); } From fe0ff3f324f181f58fa2a1b5e419a6dbe33b14de Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Fri, 3 May 2024 13:14:30 +0400 Subject: [PATCH 12/21] refactor: natspec --- contracts/Nexus.sol | 74 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/contracts/Nexus.sol b/contracts/Nexus.sol index bc74c46b..0358f705 100644 --- a/contracts/Nexus.sol +++ b/contracts/Nexus.sol @@ -230,6 +230,7 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U /// bytes4(keccak256("isValidSignature(bytes32,bytes)") = 0x1626ba7e /// @dev Delegates the validation to a validator module specified within the signature data. function isValidSignature(bytes32 hash, bytes calldata data) external view virtual override returns (bytes4) { + // First 20 bytes of data will be validator address and rest of the bytes is complete signature. address validator = address(bytes20(data[0:20])); if (!_isValidatorInstalled(validator)) revert InvalidModule(validator); (bytes32 computeHash, bytes calldata truncatedSignature) = _erc1271HashForIsValidSignatureViaNestedEIP712(hash, data[20:]); @@ -328,6 +329,74 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U return keccak256(abi.encodePacked("\x19\x01", _domainSeparator(), keccak256(abi.encode(_MESSAGE_TYPEHASH, hash)))); } + /// @dev ERC1271 signature validation (Nested EIP-712 workflow). + /// + /// This implementation uses a nested EIP-712 approach to + /// prevent signature replays when a single signer owns multiple smart contract accounts, + /// while still enabling wallet UIs (e.g. Metamask) to show the EIP-712 values. + /// + /// Crafted for phishing resistance, efficiency, flexibility. + /// __________________________________________________________________________________________ + /// + /// Glossary: + /// + /// - `DOMAIN_SEP_B`: The domain separator of the `hash`. + /// Provided by the front end. Intended to be the domain separator of the contract + /// that will call `isValidSignature` on this account. + /// + /// - `DOMAIN_SEP_A`: The domain separator of this account. + /// See: `EIP712._domainSeparator()`. + /// __________________________________________________________________________________________ + /// + /// For the `TypedDataSign` workflow, the final hash will be: + /// ``` + /// keccak256(\x19\x01 โ€– DOMAIN_SEP_B โ€– + /// hashStruct(TypedDataSign({ + /// contents: hashStruct(originalStruct), + /// name: keccak256(bytes(eip712Domain().name)), + /// version: keccak256(bytes(eip712Domain().version)), + /// chainId: eip712Domain().chainId, + /// verifyingContract: eip712Domain().verifyingContract, + /// salt: eip712Domain().salt + /// extensions: keccak256(abi.encodePacked(eip712Domain().extensions)) + /// })) + /// ) + /// ``` + /// where `โ€–` denotes the concatenation operator for bytes. + /// The order of the fields is important: `contents` comes before `name`. + /// + /// The signature will be `r โ€– s โ€– v โ€– + /// DOMAIN_SEP_B โ€– contents โ€– contentsType โ€– uint16(contentsType.length)`, + /// where `contents` is the bytes32 struct hash of the original struct. + /// + /// The `DOMAIN_SEP_B` and `contents` will be used to verify if `hash` is indeed correct. + /// __________________________________________________________________________________________ + /// + /// For the `PersonalSign` workflow, the final hash will be: + /// ``` + /// keccak256(\x19\x01 โ€– DOMAIN_SEP_A โ€– + /// hashStruct(PersonalSign({ + /// prefixed: keccak256(bytes(\x19Ethereum Signed Message:\n โ€– + /// base10(bytes(someString).length) โ€– someString)) + /// })) + /// ) + /// ``` + /// where `โ€–` denotes the concatenation operator for bytes. + /// + /// The `PersonalSign` type hash will be `keccak256("PersonalSign(bytes prefixed)")`. + /// The signature will be `r โ€– s โ€– v`. + /// __________________________________________________________________________________________ + /// + /// For demo and typescript code, see: + /// - https://github.com/junomonster/nested-eip-712 + /// - https://github.com/frangio/eip712-wrapper-for-eip1271 + /// + /// Their nomenclature may differ from ours, although the high-level idea is similar. + /// + /// Of course, if you are a wallet app maker and can update your app's UI at will, + /// you can choose a more minimalistic signature scheme like + /// `keccak256(abi.encode(address(this), hash))` instead of all these acrobatics. + /// All these are just for widespread out-of-the-box compatibility with other wallet clients. function _erc1271HashForIsValidSignatureViaNestedEIP712(bytes32 hash, bytes calldata signature) internal view @@ -389,15 +458,18 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U return (hash, signature); } + /// @dev EIP712 domain name and version. function _domainNameAndVersion() internal pure override returns (string memory name, string memory version) { name = "Nexus"; version = "0.0.1"; } + /// @dev EIP712 hashTypedData method. function hashTypedData(bytes32 structHash) external view returns (bytes32) { return _hashTypedData(structHash); } + /// @dev EIP712 domain separator. function DOMAIN_SEPARATOR() external view returns (bytes32) { return _domainSeparator(); } @@ -436,7 +508,7 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U else return false; } - /// @dev For use in `_erc1271IsValidSignatureViaNestedEIP712`, + /// @dev For use in `_erc1271HashForIsValidSignatureViaNestedEIP712`, function _typedDataSignFields() private view returns (bytes32 m) { ( bytes1 fields, From 7a1b130c66cbe5cd72f73705c791dd18b10615f9 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Sat, 4 May 2024 20:58:11 +0400 Subject: [PATCH 13/21] add token with permit for further testing --- contracts/mocks/MockToken.sol | 11 ----- contracts/mocks/TokenWithPermit.sol | 77 +++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 contracts/mocks/TokenWithPermit.sol diff --git a/contracts/mocks/MockToken.sol b/contracts/mocks/MockToken.sol index 9d186ea1..49f7b695 100644 --- a/contracts/mocks/MockToken.sol +++ b/contracts/mocks/MockToken.sol @@ -1,17 +1,6 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.24; -// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ -// _ __ _ __ -// / | / /__ | |/ /_ _______ -// / |/ / _ \| / / / / ___/ -// / /| / __/ / /_/ (__ ) -// /_/ |_/\___/_/|_\__,_/____/ -// -// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ -// Nexus: A suite of contracts for Modular Smart Account compliant with ERC-7579 and ERC-4337, developed by Biconomy. -// Learn more at https://biconomy.io/ - import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; contract MockToken is ERC20 { diff --git a/contracts/mocks/TokenWithPermit.sol b/contracts/mocks/TokenWithPermit.sol new file mode 100644 index 00000000..d5ab2614 --- /dev/null +++ b/contracts/mocks/TokenWithPermit.sol @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity ^0.8.24; + +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// _ __ _ __ +// / | / /__ | |/ /_ _______ +// / |/ / _ \| / / / / ___/ +// / /| / __/ / /_/ (__ ) +// /_/ |_/\___/_/|_\__,_/____/ +// +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// Nexus: A suite of contracts for Modular Smart Account compliant with ERC-7579 and ERC-4337, developed by Biconomy. +// Learn more at https://biconomy.io/ + +import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol"; + +/** + * @dev Interface of the ERC1271 standard signature validation method for + * contracts as defined in https://eips.ethereum.org/EIPS/eip-1271[ERC-1271]. + */ +interface IERC1271 { + /** + * @dev Should return whether the signature provided is valid for the provided data + * @param hash Hash of the data to be signed + * @param signature Signature byte array associated with _data + */ + function isValidSignature(bytes32 hash, bytes memory signature) external view returns (bytes4 magicValue); +} + +contract TokenWithPermit is ERC20Permit { + + error ERC1271InvalidSigner(address signer); + + bytes32 internal constant PERMIT_TYPEHASH_LOCAL = + keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); + + constructor(string memory name, string memory symbol) ERC20Permit(name) ERC20(name, symbol) { + _mint(msg.sender, 10_000_000 * 10 ** decimals()); + } + + function mint(address sender, uint256 amount) external { + _mint(sender, amount); + } + + function decimals() public view virtual override returns (uint8) { + return 18; + } + + function permitWith1271( + address owner, + address spender, + uint256 value, + uint256 deadline, + bytes calldata signature + ) public virtual { + if (block.timestamp > deadline) { + revert ERC2612ExpiredSignature(deadline); + } + + bytes32 structHash = keccak256(abi.encode(PERMIT_TYPEHASH_LOCAL, owner, spender, value, _useNonce(owner), deadline)); + + bytes32 hash = _hashTypedDataV4(structHash); + + if(owner.code.length > 0) { + bytes4 result = IERC1271(owner).isValidSignature(hash, signature); + if(result != bytes4(0x1626ba7e)) { + revert ERC1271InvalidSigner(owner); + } + } else { + address signer = ECDSA.recover(hash, signature); + if (signer != owner) { + revert ERC2612InvalidSigner(signer, owner); + } + } + _approve(owner, spender, value); + } +} From 2f5b9edc7a9ca528d0099eea17d261a7fb6e9584 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Mon, 6 May 2024 02:06:18 +0400 Subject: [PATCH 14/21] draft fror more tests --- .../TestERC1271Account_MockProtocol.sol | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol new file mode 100644 index 00000000..2e04cd0e --- /dev/null +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import "../../../utils/Imports.sol"; +import "../../../utils/SmartAccountTestLab.t.sol"; +import { TokenWithPermit } from "../../../../../contracts/mocks/TokenWithPermit.sol"; + + +// Todo +// Note: remove below temp refs afterwards +// refs +// https://etherscan.io/address/0x00000000006c3852cbef3e08e8df289169ede581#code +// https://github.com/thirdweb-dev/seaport-eip1271 + +contract TestERC1271Account_MockProtocol is Test, SmartAccountTestLab { + + struct _TestTemps { + bytes32 userOpHash; + bytes32 contents; + address signer; + uint256 privateKey; + uint8 v; + bytes32 r; + bytes32 s; + uint256 missingAccountFunds; + } + + // todo + bytes32 internal constant _PARENT_TYPEHASH = + 0xd61db970ec8a2edc5f9fd31d876abe01b785909acb16dcd4baaf3b434b4c439b; + + // todo // permit domain separator + bytes32 internal _DOMAIN_SEP_B; + + + TokenWithPermit public permitToken; + + function setUp() public { + init(); + permitToken = new TokenWithPermit("TestToken", "TST"); + _DOMAIN_SEP_B = permitToken.DOMAIN_SEPARATOR(); + } + + function test_isValidSignature_PersonalSign_MockValidator_Success() public { + } + + function test_isValidSignature_EIP712Sign_MockValidator_Success() public { + } + + function _toContentsHash(bytes32 contents) internal view returns (bytes32) { + return keccak256(abi.encodePacked(hex"1901", _DOMAIN_SEP_B, contents)); + } + + function _toERC1271HashPersonalSign(bytes32 childHash) internal view returns (bytes32) { + bytes32 domainSeparator = keccak256( + abi.encode( + keccak256( + "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" + ), + keccak256("Nexus"), + keccak256("0.0.1"), + block.chainid, + address(ALICE_ACCOUNT) + ) + ); + bytes32 parentStructHash = + keccak256(abi.encode(keccak256("PersonalSign(bytes prefixed)"), childHash)); + return keccak256(abi.encodePacked("\x19\x01", domainSeparator, parentStructHash)); + } + + struct _AccountDomainStruct { + bytes1 fields; + string name; + string version; + uint256 chainId; + address verifyingContract; + bytes32 salt; + uint256[] extensions; + } + + function _accountDomainStructFields(address payable account) internal view returns (bytes memory) { + _AccountDomainStruct memory t; + (t.fields, t.name, t.version, t.chainId, t.verifyingContract, t.salt, t.extensions) = + Nexus(account).eip712Domain(); + + return abi.encode( + t.fields, + keccak256(bytes(t.name)), + keccak256(bytes(t.version)), + t.chainId, + t.verifyingContract, + t.salt, + keccak256(abi.encodePacked(t.extensions)) + ); + } + + // @ TODO + // other test scenarios +} \ No newline at end of file From 54f79be3131cbf14788c61359bd5faaded97ef23 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 8 May 2024 03:45:38 +0400 Subject: [PATCH 15/21] add more test for mock permit protocol --- contracts/mocks/TokenWithPermit.sol | 8 ++-- .../TestERC1271Account_MockProtocol.sol | 39 ++++++++++++++++--- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/contracts/mocks/TokenWithPermit.sol b/contracts/mocks/TokenWithPermit.sol index d5ab2614..f4d05bfd 100644 --- a/contracts/mocks/TokenWithPermit.sol +++ b/contracts/mocks/TokenWithPermit.sol @@ -31,7 +31,7 @@ contract TokenWithPermit is ERC20Permit { error ERC1271InvalidSigner(address signer); - bytes32 internal constant PERMIT_TYPEHASH_LOCAL = + bytes32 public constant PERMIT_TYPEHASH_LOCAL = keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); constructor(string memory name, string memory symbol) ERC20Permit(name) ERC20(name, symbol) { @@ -59,15 +59,15 @@ contract TokenWithPermit is ERC20Permit { bytes32 structHash = keccak256(abi.encode(PERMIT_TYPEHASH_LOCAL, owner, spender, value, _useNonce(owner), deadline)); - bytes32 hash = _hashTypedDataV4(structHash); + bytes32 childHash = _hashTypedDataV4(structHash); if(owner.code.length > 0) { - bytes4 result = IERC1271(owner).isValidSignature(hash, signature); + bytes4 result = IERC1271(owner).isValidSignature(childHash, signature); if(result != bytes4(0x1626ba7e)) { revert ERC1271InvalidSigner(owner); } } else { - address signer = ECDSA.recover(hash, signature); + address signer = ECDSA.recover(childHash, signature); if (signer != owner) { revert ERC2612InvalidSigner(signer, owner); } diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol index 2e04cd0e..9e1fd996 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol @@ -41,14 +41,22 @@ contract TestERC1271Account_MockProtocol is Test, SmartAccountTestLab { _DOMAIN_SEP_B = permitToken.DOMAIN_SEPARATOR(); } - function test_isValidSignature_PersonalSign_MockValidator_Success() public { + function test_isValidSignature_PersonalSign_MockProtocol_MockValidator_Success() public { } - function test_isValidSignature_EIP712Sign_MockValidator_Success() public { - } - - function _toContentsHash(bytes32 contents) internal view returns (bytes32) { - return keccak256(abi.encodePacked(hex"1901", _DOMAIN_SEP_B, contents)); + function test_isValidSignature_EIP712Sign_MockProtocol_MockValidator_Success() public { + _TestTemps memory t; + t.contents = keccak256(abi.encode(permitToken.PERMIT_TYPEHASH_LOCAL, address(ALICE_ACCOUNT), address(0x69), 1e18, permitToken.nonces(address(ALICE_ACCOUNT)), block.timestamp)); + (t.v, t.r, t.s) = vm.sign(ALICE.privateKey, _toERC1271Hash(t.contents, payable(address(ALICE_ACCOUNT)))); + bytes memory contentsType = "Contents(bytes32 stuff)"; + bytes memory signature = abi.encodePacked( + t.r, t.s, t.v, _DOMAIN_SEP_B, t.contents, contentsType, uint16(contentsType.length) + ); + bytes memory completeSignature = abi.encodePacked(address(VALIDATOR_MODULE), signature); + bytes4 ret = ALICE_ACCOUNT.isValidSignature(_toContentsHash(t.contents), completeSignature); + assertEq(ret, bytes4(0x1626ba7e)); + permitToken.permitWith1271(address(ALICE_ACCOUNT), address(0x69), 1e18, block.timestamp, completeSignature); + assertEq(permitToken.allowance(address(ALICE_ACCOUNT), address(0x69)), 1e18); } function _toERC1271HashPersonalSign(bytes32 childHash) internal view returns (bytes32) { @@ -78,6 +86,25 @@ contract TestERC1271Account_MockProtocol is Test, SmartAccountTestLab { uint256[] extensions; } + function _toContentsHash(bytes32 contents) internal view returns (bytes32) { + return keccak256(abi.encodePacked(hex"1901", _DOMAIN_SEP_B, contents)); + } + + function _toERC1271Hash(bytes32 contents, address payable account) internal view returns (bytes32) { + bytes32 parentStructHash = keccak256( + abi.encodePacked( + abi.encode( + keccak256( + "TypedDataSign(Contents contents,bytes1 fields,string name,string version,uint256 chainId,address verifyingContract,bytes32 salt,uint256[] extensions)Contents(bytes32 stuff)" + ), + contents + ), + _accountDomainStructFields(account) + ) + ); + return keccak256(abi.encodePacked("\x19\x01", _DOMAIN_SEP_B, parentStructHash)); + } + function _accountDomainStructFields(address payable account) internal view returns (bytes memory) { _AccountDomainStruct memory t; (t.fields, t.name, t.version, t.chainId, t.verifyingContract, t.salt, t.extensions) = From c66a5b313149b46f4f53e835e1f9d0ace4e1282b Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 8 May 2024 04:08:40 +0400 Subject: [PATCH 16/21] refactor --- .../erc1271/TestERC1271Account_MockProtocol.sol | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol index 9e1fd996..802a169b 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol @@ -86,8 +86,18 @@ contract TestERC1271Account_MockProtocol is Test, SmartAccountTestLab { uint256[] extensions; } - function _toContentsHash(bytes32 contents) internal view returns (bytes32) { + function _toContentsHash(bytes32 contents) internal view returns (bytes32 digest) { return keccak256(abi.encodePacked(hex"1901", _DOMAIN_SEP_B, contents)); + // @todo Review + // temp + // bytes32 ds = _DOMAIN_SEP_B; + // assembly { + // let ptr := mload(0x40) + // mstore(ptr, hex"19_01") + // mstore(add(ptr, 0x02), ds) + // mstore(add(ptr, 0x22), contents) + // digest := keccak256(ptr, 0x42) + // } } function _toERC1271Hash(bytes32 contents, address payable account) internal view returns (bytes32) { From fde4d6e13b2f8b16d6f6695c70f7596fea7c10c2 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 8 May 2024 19:04:30 +0400 Subject: [PATCH 17/21] fix test --- .../TestERC1271Account_MockProtocol.sol | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol index 802a169b..38627cdb 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol @@ -5,13 +5,6 @@ import "../../../utils/Imports.sol"; import "../../../utils/SmartAccountTestLab.t.sol"; import { TokenWithPermit } from "../../../../../contracts/mocks/TokenWithPermit.sol"; - -// Todo -// Note: remove below temp refs afterwards -// refs -// https://etherscan.io/address/0x00000000006c3852cbef3e08e8df289169ede581#code -// https://github.com/thirdweb-dev/seaport-eip1271 - contract TestERC1271Account_MockProtocol is Test, SmartAccountTestLab { struct _TestTemps { @@ -41,12 +34,9 @@ contract TestERC1271Account_MockProtocol is Test, SmartAccountTestLab { _DOMAIN_SEP_B = permitToken.DOMAIN_SEPARATOR(); } - function test_isValidSignature_PersonalSign_MockProtocol_MockValidator_Success() public { - } - function test_isValidSignature_EIP712Sign_MockProtocol_MockValidator_Success() public { _TestTemps memory t; - t.contents = keccak256(abi.encode(permitToken.PERMIT_TYPEHASH_LOCAL, address(ALICE_ACCOUNT), address(0x69), 1e18, permitToken.nonces(address(ALICE_ACCOUNT)), block.timestamp)); + t.contents = keccak256(abi.encode(permitToken.PERMIT_TYPEHASH_LOCAL(), address(ALICE_ACCOUNT), address(0x69), 1e18, permitToken.nonces(address(ALICE_ACCOUNT)), block.timestamp)); (t.v, t.r, t.s) = vm.sign(ALICE.privateKey, _toERC1271Hash(t.contents, payable(address(ALICE_ACCOUNT)))); bytes memory contentsType = "Contents(bytes32 stuff)"; bytes memory signature = abi.encodePacked( @@ -88,16 +78,6 @@ contract TestERC1271Account_MockProtocol is Test, SmartAccountTestLab { function _toContentsHash(bytes32 contents) internal view returns (bytes32 digest) { return keccak256(abi.encodePacked(hex"1901", _DOMAIN_SEP_B, contents)); - // @todo Review - // temp - // bytes32 ds = _DOMAIN_SEP_B; - // assembly { - // let ptr := mload(0x40) - // mstore(ptr, hex"19_01") - // mstore(add(ptr, 0x02), ds) - // mstore(add(ptr, 0x22), contents) - // digest := keccak256(ptr, 0x42) - // } } function _toERC1271Hash(bytes32 contents, address payable account) internal view returns (bytes32) { From fa9b0d4f28d9b9978410f5cdfa9e8ca327e22349 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Wed, 8 May 2024 19:06:08 +0400 Subject: [PATCH 18/21] add refs --- .../concrete/erc1271/TestERC1271Account_MockProtocol.sol | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol index 38627cdb..0eed990e 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol @@ -5,6 +5,12 @@ import "../../../utils/Imports.sol"; import "../../../utils/SmartAccountTestLab.t.sol"; import { TokenWithPermit } from "../../../../../contracts/mocks/TokenWithPermit.sol"; +// some refs +// https://pastebin.com/m6tqJBKh +// https://pastebin.com/EVQxRH3n (just use typedData) +// https://pastebin.com/XjgWxSuG +// https://pastebin.com/y0hncv31 + contract TestERC1271Account_MockProtocol is Test, SmartAccountTestLab { struct _TestTemps { From 40da63da9232477ed82b723e08bc1b222eb97f1d Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Thu, 9 May 2024 12:35:07 +0400 Subject: [PATCH 19/21] refactor and add more tests --- .../TestERC1271Account_MockProtocol.sol | 43 +++++++++++++------ test/foundry/utils/EventsAndErrors.sol | 4 +- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol index 0eed990e..437c810f 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol @@ -55,21 +55,36 @@ contract TestERC1271Account_MockProtocol is Test, SmartAccountTestLab { assertEq(permitToken.allowance(address(ALICE_ACCOUNT), address(0x69)), 1e18); } - function _toERC1271HashPersonalSign(bytes32 childHash) internal view returns (bytes32) { - bytes32 domainSeparator = keccak256( - abi.encode( - keccak256( - "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" - ), - keccak256("Nexus"), - keccak256("0.0.1"), - block.chainid, - address(ALICE_ACCOUNT) - ) + function test_isValidSignature_EIP712Sign_MockProtocol_MockValidator_Failure_WrongSigner() public { + _TestTemps memory t; + t.contents = keccak256(abi.encode(permitToken.PERMIT_TYPEHASH_LOCAL(), address(ALICE_ACCOUNT), address(0x69), 1e18, permitToken.nonces(address(ALICE_ACCOUNT)), block.timestamp)); + (t.v, t.r, t.s) = vm.sign(BOB.privateKey, _toERC1271Hash(t.contents, payable(address(ALICE_ACCOUNT)))); + bytes memory contentsType = "Contents(bytes32 stuff)"; + bytes memory signature = abi.encodePacked( + t.r, t.s, t.v, _DOMAIN_SEP_B, t.contents, contentsType, uint16(contentsType.length) + ); + bytes memory completeSignature = abi.encodePacked(address(VALIDATOR_MODULE), signature); + + vm.expectRevert(abi.encodeWithSelector(ERC1271InvalidSigner.selector, address(ALICE_ACCOUNT))); + permitToken.permitWith1271(address(ALICE_ACCOUNT), address(0x69), 1e18, block.timestamp, completeSignature); + + assertEq(permitToken.allowance(address(ALICE_ACCOUNT), address(0x69)), 0); + } + + function test_isValidSignature_EIP712Sign_MockProtocol_MockValidator_Failure_SignWrongAllowance() public { + _TestTemps memory t; + t.contents = keccak256(abi.encode(permitToken.PERMIT_TYPEHASH_LOCAL(), address(ALICE_ACCOUNT), address(0x69), 1e6, permitToken.nonces(address(ALICE_ACCOUNT)), block.timestamp)); + (t.v, t.r, t.s) = vm.sign(BOB.privateKey, _toERC1271Hash(t.contents, payable(address(ALICE_ACCOUNT)))); + bytes memory contentsType = "Contents(bytes32 stuff)"; + bytes memory signature = abi.encodePacked( + t.r, t.s, t.v, _DOMAIN_SEP_B, t.contents, contentsType, uint16(contentsType.length) ); - bytes32 parentStructHash = - keccak256(abi.encode(keccak256("PersonalSign(bytes prefixed)"), childHash)); - return keccak256(abi.encodePacked("\x19\x01", domainSeparator, parentStructHash)); + bytes memory completeSignature = abi.encodePacked(address(VALIDATOR_MODULE), signature); + + vm.expectRevert(abi.encodeWithSelector(ERC1271InvalidSigner.selector, address(ALICE_ACCOUNT))); + permitToken.permitWith1271(address(ALICE_ACCOUNT), address(0x69), 1e18, block.timestamp, completeSignature); + + assertEq(permitToken.allowance(address(ALICE_ACCOUNT), address(0x69)), 0); } struct _AccountDomainStruct { diff --git a/test/foundry/utils/EventsAndErrors.sol b/test/foundry/utils/EventsAndErrors.sol index 492c993f..ad0f2ad9 100644 --- a/test/foundry/utils/EventsAndErrors.sol +++ b/test/foundry/utils/EventsAndErrors.sol @@ -37,6 +37,8 @@ contract EventsAndErrors { error HookPostCheckFailed(); error HookAlreadyInstalled(address currentHook); error FallbackAlreadyInstalledForSelector(bytes4 selector); - + event TryExecuteUnsuccessful(uint256 batchExecutionindex, bytes result); + + error ERC1271InvalidSigner(address signer); } From 397d479f093bee8750c1d90790d935a7860f7c6d Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Thu, 9 May 2024 14:07:22 +0400 Subject: [PATCH 20/21] feat: add ERC6492 unwrapping --- contracts/Nexus.sol | 13 ++++ .../TestERC1271Account_IsValidSignature.sol | 34 ++++++++++ test/foundry/utils/Helpers.sol | 64 +++++++++++++++++++ 3 files changed, 111 insertions(+) diff --git a/contracts/Nexus.sol b/contracts/Nexus.sol index 0358f705..78579aa5 100644 --- a/contracts/Nexus.sol +++ b/contracts/Nexus.sol @@ -403,6 +403,19 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U virtual returns (bytes32, bytes calldata) { + assembly { + // Unwraps the ERC6492 wrapper if it exists. + // See: https://eips.ethereum.org/EIPS/eip-6492 + if eq( + calldataload(add(signature.offset, sub(signature.length, 0x20))), + mul(0x6492, div(not(mload(0x60)), 0xffff)) // `0x6492...6492`. + ) { + let o := add(signature.offset, calldataload(add(signature.offset, 0x40))) + signature.length := calldataload(o) + signature.offset := add(o, 0x20) + } + } + bool result; bytes32 t = _typedDataSignFields(); /// @solidity memory-safe-assembly diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol index 13a57ef5..1f3c5475 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_IsValidSignature.sol @@ -50,6 +50,7 @@ contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { bytes memory signature = abi.encodePacked( t.r, t.s, t.v, _DOMAIN_SEP_B, t.contents, contentsType, uint16(contentsType.length) ); + if (_random() % 4 == 0) signature = _erc6492Wrap(signature); bytes4 ret = ALICE_ACCOUNT.isValidSignature(_toContentsHash(t.contents), abi.encodePacked(address(VALIDATOR_MODULE), signature)); assertEq(ret, bytes4(0x1626ba7e)); @@ -138,6 +139,39 @@ contract TestERC1271Account_IsValidSignature is Test, SmartAccountTestLab { ); } + function _randomString(string memory byteChoices, bool nonEmpty) + internal + returns (string memory result) + { + uint256 randomness = _random(); + uint256 resultLength = _bound(_random(), nonEmpty ? 1 : 0, _random() % 32 != 0 ? 4 : 128); + /// @solidity memory-safe-assembly + assembly { + if mload(byteChoices) { + result := mload(0x40) + mstore(0x00, randomness) + mstore(0x40, and(add(add(result, 0x40), resultLength), not(31))) + mstore(result, resultLength) + + // forgefmt: disable-next-item + for { let i := 0 } lt(i, resultLength) { i := add(i, 1) } { + mstore(0x20, gas()) + mstore8( + add(add(result, 0x20), i), + mload(add(add(byteChoices, 1), mod(keccak256(0x00, 0x40), mload(byteChoices)))) + ) + } + } + } + } + + function _erc6492Wrap(bytes memory signature) internal returns (bytes memory) { + return abi.encodePacked( + abi.encode(_randomNonZeroAddress(), bytes(_randomString("12345", false)), signature), + bytes32(0x6492649264926492649264926492649264926492649264926492649264926492) + ); + } + // @ TODO // other test scenarios } \ No newline at end of file diff --git a/test/foundry/utils/Helpers.sol b/test/foundry/utils/Helpers.sol index c969238b..dfdaceed 100644 --- a/test/foundry/utils/Helpers.sol +++ b/test/foundry/utils/Helpers.sol @@ -264,6 +264,70 @@ contract Helpers is CheatCodes, EventsAndErrors { return keccak256(a) == keccak256(b); } + /// @dev Returns a random non-zero address. + function _randomNonZeroAddress() internal returns (address result) { + do { + result = address(uint160(_random())); + } while (result == address(0)); + } + + /// @dev credits: vectorized || solady + /// @dev Returns a pseudorandom random number from [0 .. 2**256 - 1] (inclusive). + /// For usage in fuzz tests, please ensure that the function has an unnamed uint256 argument. + /// e.g. `testSomething(uint256) public`. + function _random() internal returns (uint256 r) { + /// @solidity memory-safe-assembly + assembly { + // This is the keccak256 of a very long string I randomly mashed on my keyboard. + let sSlot := 0xd715531fe383f818c5f158c342925dcf01b954d24678ada4d07c36af0f20e1ee + let sValue := sload(sSlot) + + mstore(0x20, sValue) + r := keccak256(0x20, 0x40) + + // If the storage is uninitialized, initialize it to the keccak256 of the calldata. + if iszero(sValue) { + sValue := sSlot + let m := mload(0x40) + calldatacopy(m, 0, calldatasize()) + r := keccak256(m, calldatasize()) + } + sstore(sSlot, add(r, 1)) + + // Do some biased sampling for more robust tests. + // prettier-ignore + for {} 1 {} { + let d := byte(0, r) + // With a 1/256 chance, randomly set `r` to any of 0,1,2. + if iszero(d) { + r := and(r, 3) + break + } + // With a 1/2 chance, set `r` to near a random power of 2. + if iszero(and(2, d)) { + // Set `t` either `not(0)` or `xor(sValue, r)`. + let t := xor(not(0), mul(iszero(and(4, d)), not(xor(sValue, r)))) + // Set `r` to `t` shifted left or right by a random multiple of 8. + switch and(8, d) + case 0 { + if iszero(and(16, d)) { t := 1 } + r := add(shl(shl(3, and(byte(3, r), 0x1f)), t), sub(and(r, 7), 3)) + } + default { + if iszero(and(16, d)) { t := shl(255, 1) } + r := add(shr(shl(3, and(byte(3, r), 0x1f)), t), sub(and(r, 7), 3)) + } + // With a 1/2 chance, negate `r`. + if iszero(and(0x20, d)) { r := not(r) } + break + } + // Otherwise, just set `r` to `xor(sValue, r)`. + r := xor(sValue, r) + break + } + } + } + function test() public pure { // This function is used to ignore file in coverage report } From e7bb38c3c72fe4ed50e0920e155b0b4c1d09d042 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Thu, 9 May 2024 14:19:13 +0400 Subject: [PATCH 21/21] dev notes --- .../unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol index 437c810f..3503322a 100644 --- a/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol +++ b/test/foundry/unit/concrete/erc1271/TestERC1271Account_MockProtocol.sol @@ -101,6 +101,8 @@ contract TestERC1271Account_MockProtocol is Test, SmartAccountTestLab { return keccak256(abi.encodePacked(hex"1901", _DOMAIN_SEP_B, contents)); } + // Review: https://github.com/Vectorized/solady/blob/08b0c1debff97f2b4984a2bbbdec7a5d9f95d5db/test/ERC1271.t.sol#L286 + function _toERC1271Hash(bytes32 contents, address payable account) internal view returns (bytes32) { bytes32 parentStructHash = keccak256( abi.encodePacked(