Skip to content

Commit

Permalink
Closes #83: Add integration tests (#90)
Browse files Browse the repository at this point in the history 
* Add integration tests

* Formatting

* Make compatible with stackup

* Minor refactortings

* Remove nonce check from module, as done by Entrypoint

* Add more comments
  • Loading branch information
@rmeissner
rmeissner authored Oct 13, 2023
1 parent 72d5e21 commit 035f905
Show file tree
Hide file tree
Showing 10 changed files with 469 additions and 165 deletions.
2 changes: 2 additions & 0 deletions 4337/README.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Safe Module/Fallback handler for EIP4337 Support

:warning: **This module MUST only be used with Safe 1.4.1 or newer** :warning:

## Execution Flow

The diagram below outlines the flow that is triggered when a user operation is submitted to the entrypoint. Additionally the gas overhead compared to a native implementation is mentioned.
Expand Down
93 changes: 30 additions & 63 deletions 4337/contracts/EIP4337Module.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,16 @@ import {ISafe} from "./interfaces/Safe.sol";
/// @title EIP4337Module
abstract contract EIP4337Module is HandlerContext, CompatibilityFallbackHandler {
using UserOperationLib for UserOperation;

// value in case of signature failure, with no time-range.
// equivalent to _packValidationData(true,0,0);
uint256 internal constant SIG_VALIDATION_FAILED = 1;

bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH = keccak256("EIP712Domain(uint256 chainId,address verifyingContract)");

bytes32 private constant SAFE_OP_TYPEHASH =
keccak256(
"SafeOp(address safe,bytes callData,uint256 nonce,uint256 verificationGas,uint256 preVerificationGas,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,uint256 callGas,address entryPoint)"
"SafeOp(address safe,bytes callData,uint256 nonce,uint256 preVerificationGas,uint256 verificationGasLimit,uint256 callGasLimit,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,address entryPoint)"
);

address public immutable supportedEntryPoint;
Expand All @@ -32,32 +37,28 @@ abstract contract EIP4337Module is HandlerContext, CompatibilityFallbackHandler
UserOperation calldata userOp,
bytes32,
uint256 requiredPrefund
) external returns (uint256) {
) external returns (uint256 validationResult) {
address payable safeAddress = payable(userOp.sender);
// The entryPoint address is appended to the calldata in `HandlerContext` contract
// Because of this, the relayer may be manipulate the entryPoint address, therefore we have to verify that
// Because of this, the relayer may manipulate the entryPoint address, therefore we have to verify that
// the sender is the Safe specified in the userOperation
require(safeAddress == msg.sender, "Invalid Caller");

validateReplayProtection(userOp);

// We verify that the userOp calls the expected execution function
require(expectedExecutionFunctionId == bytes4(userOp.callData), "Unsupported execution function id");

// We need to make sure that the entryPoint's requested prefund is in bounds
require(requiredPrefund <= userOp.requiredPreFund(), "Prefund too high");

address entryPoint = _msgSender();
require(entryPoint == supportedEntryPoint, "Unsupported entry point");
_validateSignatures(entryPoint, userOp);
// The userOp nonce is validated in the Entrypoint (for 0.6.0+), therefore we will not check it again
validationResult = validateSignatures(entryPoint, userOp);

// We trust the entrypoint to set the correct prefund value, based on the operation params
// We need to perform this even if the signature is not valid, else the simulation function of the Entrypoint will not work
if (requiredPrefund != 0) {
ISafe(safeAddress).execTransactionFromModule(entryPoint, requiredPrefund, "", 0);
}
return 0;
}

function validateReplayProtection(UserOperation calldata userOp) internal virtual;

function domainSeparator() public view returns (bytes32) {
return keccak256(abi.encode(DOMAIN_SEPARATOR_TYPEHASH, block.chainid, this));
}
Expand All @@ -66,101 +67,67 @@ abstract contract EIP4337Module is HandlerContext, CompatibilityFallbackHandler
/// @param safe Safe address
/// @param callData Call data
/// @param nonce Nonce of the operation
/// @param verificationGas Gas required for verification
/// @param preVerificationGas Gas required for pre-verification (e.g. for EOA signature verification)
/// @param verificationGasLimit Gas required for verification
/// @param callGasLimit Gas available during the execution of the call
/// @param maxFeePerGas Max fee per gas
/// @param maxPriorityFeePerGas Max priority fee per gas
/// @param callGas Gas available during the execution of the call
/// @param entryPoint Address of the entry point
/// @return Operation hash bytes
function encodeOperationData(
function getOperationHash(
address safe,
bytes calldata callData,
uint256 nonce,
uint256 verificationGas,
uint256 preVerificationGas,
uint256 verificationGasLimit,
uint256 callGasLimit,
uint256 maxFeePerGas,
uint256 maxPriorityFeePerGas,
uint256 callGas,
address entryPoint
) public view returns (bytes memory) {
) public view returns (bytes32) {
bytes32 safeOperationHash = keccak256(
abi.encode(
SAFE_OP_TYPEHASH,
safe,
keccak256(callData),
nonce,
verificationGas,
preVerificationGas,
verificationGasLimit,
callGasLimit,
maxFeePerGas,
maxPriorityFeePerGas,
callGas,
entryPoint
)
);

return abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator(), safeOperationHash);
}

function getOperationHash(
address safe,
bytes calldata callData,
uint256 nonce,
uint256 verificationGas,
uint256 preVerificationGas,
uint256 maxFeePerGas,
uint256 maxPriorityFeePerGas,
uint256 callGas,
address entryPoint
) public view returns (bytes32) {
return
keccak256(
encodeOperationData(
safe,
callData,
nonce,
verificationGas,
preVerificationGas,
maxFeePerGas,
maxPriorityFeePerGas,
callGas,
entryPoint
)
);
return keccak256(abi.encodePacked(bytes1(0x19), bytes1(0x01), domainSeparator(), safeOperationHash));
}

/// @dev Validates that the user operation is correctly signed. Users methods from Safe contract, reverts if signatures are invalid
/// @param entryPoint Address of the entry point
/// @param userOp User operation struct
function _validateSignatures(address entryPoint, UserOperation calldata userOp) internal view {
function validateSignatures(address entryPoint, UserOperation calldata userOp) internal view returns (uint256) {
bytes32 operationHash = getOperationHash(
payable(userOp.sender),
userOp.callData,
userOp.nonce,
userOp.verificationGasLimit,
userOp.preVerificationGas,
userOp.verificationGasLimit,
userOp.callGasLimit,
userOp.maxFeePerGas,
userOp.maxPriorityFeePerGas,
userOp.callGasLimit,
entryPoint
);

ISafe(payable(userOp.sender)).checkSignatures(operationHash, "", userOp.signature);
try ISafe(payable(userOp.sender)).checkSignatures(operationHash, "", userOp.signature) {
return 0;
} catch {
return SIG_VALIDATION_FAILED;
}
}
}

contract Simple4337Module is EIP4337Module {
constructor(address entryPoint)
EIP4337Module(entryPoint, bytes4(keccak256("execTransactionFromModule(address,uint256,bytes,uint8)")))
{}

function validateReplayProtection(UserOperation calldata userOp) internal override {
// The entrypoints handles the increase of the nonce
// Right shifting fills up with 0s from the left
uint192 key = uint192(userOp.nonce >> 64);
uint256 safeNonce = INonceManager(supportedEntryPoint).getNonce(userOp.sender, key);

// Check returned nonce against the user operation nonce
require(safeNonce == userOp.nonce, "Invalid Nonce");
}
}
26 changes: 13 additions & 13 deletions 4337/contracts/test/SafeMock.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ contract Safe4337Mock is SafeMock {

bytes32 private constant SAFE_OP_TYPEHASH =
keccak256(
"SafeOp(address safe,bytes callData,uint256 nonce,uint256 verificationGas,uint256 preVerificationGas,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,uint256 callGas,address entryPoint)"
"SafeOp(address safe,bytes callData,uint256 nonce,uint256 preVerificationGas,uint256 verificationGasLimit,uint256 callGasLimit,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,address entryPoint)"
);

bytes4 public immutable expectedExecutionFunctionId;
Expand Down Expand Up @@ -164,22 +164,22 @@ contract Safe4337Mock is SafeMock {
/// @param safe Safe address
/// @param callData Call data
/// @param nonce Nonce of the operation
/// @param verificationGas Gas required for verification
/// @param preVerificationGas Gas required for pre-verification (e.g. for EOA signature verification)
/// @param verificationGasLimit Gas required for verification
/// @param callGasLimit Gas available during the execution of the call
/// @param maxFeePerGas Max fee per gas
/// @param maxPriorityFeePerGas Max priority fee per gas
/// @param callGas Gas available during the execution of the call
/// @param entryPoint Address of the entry point
/// @return Operation hash bytes
function encodeOperationData(
address safe,
bytes calldata callData,
uint256 nonce,
uint256 verificationGas,
uint256 preVerificationGas,
uint256 verificationGasLimit,
uint256 callGasLimit,
uint256 maxFeePerGas,
uint256 maxPriorityFeePerGas,
uint256 callGas,
address entryPoint
) public view returns (bytes memory) {
bytes32 safeOperationHash = keccak256(
Expand All @@ -188,11 +188,11 @@ contract Safe4337Mock is SafeMock {
safe,
keccak256(callData),
nonce,
verificationGas,
preVerificationGas,
verificationGasLimit,
callGasLimit,
maxFeePerGas,
maxPriorityFeePerGas,
callGas,
entryPoint
)
);
Expand All @@ -204,11 +204,11 @@ contract Safe4337Mock is SafeMock {
address safe,
bytes calldata callData,
uint256 nonce,
uint256 verificationGas,
uint256 preVerificationGas,
uint256 verificationGasLimit,
uint256 callGasLimit,
uint256 maxFeePerGas,
uint256 maxPriorityFeePerGas,
uint256 callGas,
address entryPoint
) public view returns (bytes32) {
return
Expand All @@ -217,11 +217,11 @@ contract Safe4337Mock is SafeMock {
safe,
callData,
nonce,
verificationGas,
preVerificationGas,
verificationGasLimit,
callGasLimit,
maxFeePerGas,
maxPriorityFeePerGas,
callGas,
entryPoint
)
);
Expand All @@ -239,11 +239,11 @@ contract Safe4337Mock is SafeMock {
payable(userOp.sender),
userOp.callData,
userOp.nonce,
userOp.verificationGasLimit,
userOp.preVerificationGas,
userOp.verificationGasLimit,
userOp.callGasLimit,
userOp.maxFeePerGas,
userOp.maxPriorityFeePerGas,
userOp.callGasLimit,
entryPoint
);
bytes32 operationHash = keccak256(operationData);
Expand Down
7 changes: 6 additions & 1 deletion 4337/contracts/test/TestEntryPoint.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,12 @@ contract TestEntryPoint is INonceManager {

require(gasleft() > userOp.verificationGasLimit, "Not enough gas for verification");

Account(userOp.sender).validateUserOp{gas: userOp.verificationGasLimit}(userOp, bytes32(0), requiredPrefund);
uint256 validationResult = Account(userOp.sender).validateUserOp{gas: userOp.verificationGasLimit}(
userOp,
bytes32(0),
requiredPrefund
);
require(validationResult == 0, "Signature validation failed");
uint256 userBalance = balances[userOp.sender];
if (userBalance < requiredPrefund) {
revert NotEnoughFunds(requiredPrefund, userBalance);
Expand Down
Loading

0 comments on commit 035f905

Please sign in to comment.