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

Implement Support for ERC-4337 EntryPoint v0.7 #243

Merged
merged 3 commits into from
Feb 12, 2024
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
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
modules/**
Copy link
Member Author

@mmv08 mmv08 Jan 29, 2024

Choose a reason for hiding this comment

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

This and other formatting/linting-related changes were needed to fix the failing CI job. The problem was conflicting eslint-prettier-plugin rules and prettier itself due to misconfiguration.

TLDR: For linting and formatting javascript/typescript code, we use eslint-plugin-prettier, which runs prettier as ESLint rules. For checking formatting, we use a standalone prettier package instead. After updating dependencies, they introduced conflicting rules. So, ESLint would say that prettier's formatting needs to be corrected and vice versa. To fix this, I updated the prettier script to only format solidity files in the 4337 module and added the examples directory to .prettierignore for the global check (since the only app there uses eslint for formatting as well)

examples/**
4 changes: 2 additions & 2 deletions modules/4337-gas-metering/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
/* Completeness */
"skipLibCheck": true /* Skip type checking all .d.ts files. */,
"outDir": "./dist" /* Redirect output structure to the directory. */,
"noImplicitReturns": true /* Report error when not all code paths in function return a value. */
"noImplicitReturns": true /* Report error when not all code paths in function return a value. */,
},
"exclude": ["node_modules"]
"exclude": ["node_modules"],
}
4 changes: 4 additions & 0 deletions modules/4337/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module.exports = {
ignorePatterns: ['build', '.eslintrc.js', 'certora', '.certora_internal'],
extends: ['../../.eslintrc.js'],
}
2 changes: 1 addition & 1 deletion modules/4337/certora/conf/Safe4337Module.conf
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"solc": "solc8.23",
"verify": "Safe4337Module:certora/specs/Safe4337Module.spec",
"packages": [
"@account-abstraction=../../node_modules/@account-abstraction",
"@account-abstraction=./node_modules/@account-abstraction",
"@safe-global=../../node_modules/@safe-global"
]
}
2 changes: 1 addition & 1 deletion modules/4337/certora/conf/TransactionExecutionMethods.conf
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"solc": "solc8.23",
"verify": "Safe4337Module:certora/specs/TransactionExecutionMethods.spec",
"packages": [
"@account-abstraction=../../node_modules/@account-abstraction",
"@account-abstraction=./node_modules/@account-abstraction",
"@safe-global=../../node_modules/@safe-global"
]
}
2 changes: 1 addition & 1 deletion modules/4337/certora/conf/ValidationDataLastBitOne.conf
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"solc": "solc8.23",
"verify": "Safe4337Module:certora/specs/ValidationDataLastBitOne.spec",
"packages": [
"@account-abstraction=../../node_modules/@account-abstraction",
"@account-abstraction=./node_modules/@account-abstraction",
"@safe-global=../../node_modules/@safe-global"
]
}
14 changes: 7 additions & 7 deletions modules/4337/certora/specs/Safe4337Module.spec
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ methods {
) external => DISPATCHER(true);

// Optional
function validateUserOp(Safe4337Module.UserOperation,bytes32,uint256) external returns(uint256);
function validateUserOp(Safe4337Module.PackedUserOperation,bytes32,uint256) external returns(uint256);
function executeUserOp(address, uint256, bytes, uint8) external;
function executeUserOpWithErrorString(address, uint256, bytes, uint8) external;
function getOperationHash(
Safe4337Module.UserOperation userOp
Safe4337Module.PackedUserOperation userOp
) external returns(bytes32) envfree => PER_CALLEE_CONSTANT;
}
persistent ghost ERC2771MessageSender() returns address;
Expand All @@ -36,7 +36,7 @@ function checkSignaturesFunctionCalled() returns bool {
}

rule onlyEntryPointCallable(method f) filtered {
f -> f.selector == sig:validateUserOp(Safe4337Module.UserOperation,bytes32,uint256).selector ||
f -> f.selector == sig:validateUserOp(Safe4337Module.PackedUserOperation,bytes32,uint256).selector ||
f.selector == sig:executeUserOp(address,uint256,bytes,uint8).selector ||
f.selector == sig:executeUserOpWithErrorString(address,uint256,bytes,uint8).selector
} {
Expand All @@ -48,7 +48,7 @@ rule onlyEntryPointCallable(method f) filtered {

// checkSignatures should be always called if validateUserOp succeeds
rule checkSignaturesIsCalledIfValidateUserOpSucceeds(address sender,
Safe4337Module.UserOperation userOp,
Safe4337Module.PackedUserOperation userOp,
bytes32 userOpHash,
uint256 missingAccountFunds) {
env e;
Expand All @@ -59,7 +59,7 @@ rule checkSignaturesIsCalledIfValidateUserOpSucceeds(address sender,
}

rule signatureTimestampsPresentInValidationData(address sender,
Safe4337Module.UserOperation userOp,
Safe4337Module.PackedUserOperation userOp,
bytes32 userOpHash,
uint256 missingAccountFunds) {
env e;
Expand All @@ -76,7 +76,7 @@ rule signatureTimestampsPresentInValidationData(address sender,
}

rule validationDataLastBitZeroIfCheckSignaturesSucceeds(address sender,
Safe4337Module.UserOperation userOp,
Safe4337Module.PackedUserOperation userOp,
bytes32 userOpHash,
uint256 missingAccountFunds) {
env e;
Expand All @@ -93,7 +93,7 @@ rule validationDataLastBitZeroIfCheckSignaturesSucceeds(address sender,
}

rule balanceChangeAfterValidateUserOp(
Safe4337Module.UserOperation userOp,
Safe4337Module.PackedUserOperation userOp,
bytes32 dummyData,
uint256 missingAccountFunds) {

Expand Down
4 changes: 2 additions & 2 deletions modules/4337/certora/specs/TransactionExecutionMethods.spec
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ methods {


// Optional
function validateUserOp(Safe4337Module.UserOperation,bytes32,uint256) external returns(uint256);
function validateUserOp(Safe4337Module.PackedUserOperation,bytes32,uint256) external returns(uint256);
function executeUserOp(address, uint256, bytes, uint8) external;
function executeUserOpWithErrorString(address, uint256, bytes, uint8) external;
}
Expand All @@ -23,7 +23,7 @@ function ExecTxCalled() returns bool {
}

rule transactionExecutionMethods(method f) filtered { f->
f.selector != sig:validateUserOp(Safe4337Module.UserOperation,bytes32,uint256).selector &&
f.selector != sig:validateUserOp(Safe4337Module.PackedUserOperation,bytes32,uint256).selector &&
f.selector != sig:executeUserOp(address,uint256,bytes,uint8).selector &&
f.selector != sig:executeUserOpWithErrorString(address,uint256,bytes,uint8).selector
} {
Expand Down
6 changes: 3 additions & 3 deletions modules/4337/certora/specs/ValidationDataLastBitOne.spec
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ methods {
function safeContract.getSignatures(bytes signature) external returns (bytes) envfree;

// Optional
function validateUserOp(Safe4337Module.UserOperation,bytes32,uint256) external returns(uint256);
function validateUserOp(Safe4337Module.PackedUserOperation,bytes32,uint256) external returns(uint256);
function getOperationHash(
Safe4337Module.UserOperation userOp
Safe4337Module.PackedUserOperation userOp
) external returns(bytes32) envfree => PER_CALLEE_CONSTANT;
}

rule validationDataLastBitOneIfCheckSignaturesFails(address sender,
Safe4337Module.UserOperation userOp,
Safe4337Module.PackedUserOperation userOp,
bytes32 dummyData,
uint256 missingAccountFunds) {
env e;
Expand Down
35 changes: 16 additions & 19 deletions modules/4337/contracts/Safe4337Module.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ pragma solidity 0.8.23;

import {HandlerContext} from "@safe-global/safe-contracts/contracts/handler/HandlerContext.sol";
import {CompatibilityFallbackHandler} from "@safe-global/safe-contracts/contracts/handler/CompatibilityFallbackHandler.sol";
import {IAccount} from "@account-abstraction/contracts/interfaces/IAccount.sol";
import {UserOperation} from "@account-abstraction/contracts/interfaces/UserOperation.sol";
import {_packValidationData} from "@account-abstraction/contracts/core/Helpers.sol";
import {IAccount} from "@account-abstraction/contracts/contracts/interfaces/IAccount.sol";
import {PackedUserOperation} from "@account-abstraction/contracts/contracts/interfaces/PackedUserOperation.sol";
import {_packValidationData} from "@account-abstraction/contracts/contracts/core/Helpers.sol";
import {ISafe} from "./interfaces/Safe.sol";

/**
Expand Down Expand Up @@ -33,8 +33,7 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
* {uint256} nonce - A unique number associated with the user operation, preventing replay attacks by ensuring each operation is unique.
* {bytes} initCode - The packed encoding of a factory address and its factory-specific data for creating a new Safe account.
* {bytes} callData - The bytes representing the data of the function call to be executed.
* {uint256} callGasLimit - The maximum amount of gas allowed for executing the function call.
* {uint256} verificationGasLimit - The maximum amount of gas allowed for the verification process.
* {bytes32} accountGasLimits - Packed encoding of the gas limits for the account: {uint128 validationGasLimit, uint128 callGasLimit}.
* {uint256} preVerificationGas - The amount of gas allocated for pre-verification steps before executing the main operation.
* {uint256} maxFeePerGas - The maximum fee per gas that the user is willing to pay for the transaction.
* {uint256} maxPriorityFeePerGas - The maximum priority fee per gas that the user is willing to pay for the transaction.
Expand All @@ -44,10 +43,10 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
* {address} entryPoint - The address of the entry point that will execute the user operation.
* @dev When validating the user operation, the signature timestamps are pre-pended to the signature bytes.
* keccak256(
"SafeOp(address safe,uint256 nonce,bytes initCode,bytes callData,uint256 callGasLimit,uint256 verificationGasLimit,uint256 preVerificationGas,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,bytes paymasterAndData,uint48 validAfter,uint48 validUntil,address entryPoint)"
) = 0x84aa190356f56b8c87825f54884392a9907c23ee0f8e1ea86336b763faf021bd
"SafeOp(address safe,uint256 nonce,bytes initCode,bytes callData,bytes32 accountGasLimits,uint256 preVerificationGas,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,bytes paymasterAndData,uint48 validAfter,uint48 validUntil,address entryPoint)"
) = 0x9efbfd16c059a992a21cba49ddec650b37de25cf6baa04788c16c00b47bb62de
*/
bytes32 private constant SAFE_OP_TYPEHASH = 0x84aa190356f56b8c87825f54884392a9907c23ee0f8e1ea86336b763faf021bd;
bytes32 private constant SAFE_OP_TYPEHASH = 0x9efbfd16c059a992a21cba49ddec650b37de25cf6baa04788c16c00b47bb62de;

/**
* @dev A structure used internally for manually encoding a Safe operation for when computing the EIP-712 struct hash.
Expand All @@ -58,8 +57,7 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
uint256 nonce;
bytes32 initCodeHash;
bytes32 callDataHash;
uint256 callGasLimit;
uint256 verificationGasLimit;
bytes32 accountGasLimits;
uint256 preVerificationGas;
uint256 maxFeePerGas;
uint256 maxPriorityFeePerGas;
Expand Down Expand Up @@ -128,7 +126,7 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
* @inheritdoc IAccount
*/
function validateUserOp(
UserOperation calldata userOp,
PackedUserOperation calldata userOp,
bytes32,
uint256 missingAccountFunds
) external onlySupportedEntryPoint returns (uint256 validationData) {
Expand Down Expand Up @@ -203,7 +201,7 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
* @param userOp The ERC-4337 user operation.
* @return operationHash Operation hash.
*/
function getOperationHash(UserOperation calldata userOp) external view returns (bytes32 operationHash) {
function getOperationHash(PackedUserOperation calldata userOp) external view returns (bytes32 operationHash) {
(bytes memory operationData, , , ) = _getSafeOp(userOp);
operationHash = keccak256(operationData);
}
Expand All @@ -217,7 +215,7 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
* @param userOp User operation struct.
* @return validationData An integer indicating the result of the validation.
*/
function _validateSignatures(UserOperation calldata userOp) internal view returns (uint256 validationData) {
function _validateSignatures(PackedUserOperation calldata userOp) internal view returns (uint256 validationData) {
(bytes memory operationData, uint48 validAfter, uint48 validUntil, bytes calldata signatures) = _getSafeOp(userOp);
try ISafe(payable(userOp.sender)).checkSignatures(keccak256(operationData), operationData, signatures) {
// The timestamps are validated by the entry point, therefore we will not check them again
Expand All @@ -236,7 +234,7 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
* @return signatures The Safe owner signatures extracted from the user operation.
*/
function _getSafeOp(
UserOperation calldata userOp
PackedUserOperation calldata userOp
) internal view returns (bytes memory operationData, uint48 validAfter, uint48 validUntil, bytes calldata signatures) {
// Extract additional Safe operation fields from the user operation signature which is encoded as:
// `abi.encodePacked(validAfter, validUntil, signatures)`
Expand All @@ -262,8 +260,7 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
nonce: userOp.nonce,
initCodeHash: keccak256(userOp.initCode),
callDataHash: keccak256(userOp.callData),
callGasLimit: userOp.callGasLimit,
verificationGasLimit: userOp.verificationGasLimit,
accountGasLimits: userOp.accountGasLimits,
preVerificationGas: userOp.preVerificationGas,
maxFeePerGas: userOp.maxFeePerGas,
maxPriorityFeePerGas: userOp.maxPriorityFeePerGas,
Expand All @@ -277,9 +274,9 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
// solhint-disable-next-line no-inline-assembly
assembly ("memory-safe") {
// Since the `encodedSafeOp` value's memory layout is identical to the result of `abi.encode`-ing the
// individual `SafeOp` fields, we can pass it directly to `keccak256`. Additionally, there are 14
// 32-byte fields to hash, for a length of `14 * 32 = 448` bytes.
safeOpStructHash := keccak256(encodedSafeOp, 448)
// individual `SafeOp` fields, we can pass it directly to `keccak256`. Additionally, there are 13
// 32-byte fields to hash, for a length of `13 * 32 = 448` bytes.
safeOpStructHash := keccak256(encodedSafeOp, 416)
}

operationData = abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator(), safeOpStructHash);
Expand Down
10 changes: 5 additions & 5 deletions modules/4337/contracts/experimental/SafeSignerLaunchpad.sol
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity >=0.8.0 <0.9.0;

import {IAccount} from "@account-abstraction/contracts/interfaces/IAccount.sol";
import {UserOperation} from "@account-abstraction/contracts/interfaces/UserOperation.sol";
import {_packValidationData} from "@account-abstraction/contracts/core/Helpers.sol";
import {IAccount} from "@account-abstraction/contracts/contracts/interfaces/IAccount.sol";
import {PackedUserOperation} from "@account-abstraction/contracts/contracts/interfaces/PackedUserOperation.sol";
import {_packValidationData} from "@account-abstraction/contracts/contracts/core/Helpers.sol";
import {SafeStorage} from "@safe-global/safe-contracts/contracts/libraries/SafeStorage.sol";
import {SignatureValidatorConstants} from "./SignatureValidatorConstants.sol";

Expand Down Expand Up @@ -137,7 +137,7 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage, SignatureValidatorConstan
}

function validateUserOp(
UserOperation calldata userOp,
PackedUserOperation calldata userOp,
bytes32 userOpHash,
uint256 missingAccountFunds
) external override onlyProxy onlySupportedEntryPoint returns (uint256 validationData) {
Expand Down Expand Up @@ -183,7 +183,7 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage, SignatureValidatorConstan
* @return validationData An integer indicating the result of the validation.
*/
function _validateSignatures(
UserOperation calldata userOp,
PackedUserOperation calldata userOp,
bytes32 userOpHash,
address signerFactory,
bytes memory signerData
Expand Down
Loading
Loading