Skip to content

Commit

Permalink
fix: use 2771 for 4337 prevalidation msg.sender
Browse files Browse the repository at this point in the history
  • Loading branch information
highskore committed Jan 10, 2025
1 parent 731daf9 commit ab75991
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 28 deletions.
5 changes: 2 additions & 3 deletions contracts/base/ModuleManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ abstract contract ModuleManager is Storage, EIP712, IModuleManagerEventsAndError
// Mark nonce as used
_getAccountStorage().nonces[data.nonce] = true;
// Check if the signature is valid
require((IValidator(validator).isValidSignatureWithSender(msg.sender, hash, signature[20:]) == bytes4(0x1626ba7e)), EmergencyUninstallSigError());
require((IValidator(validator).isValidSignatureWithSender(address(this), hash, signature[20:]) == ERC1271_MAGICVALUE), EmergencyUninstallSigError());
}

/// @dev Retrieves the pre-validation hook from the storage based on the hook type.
Expand Down Expand Up @@ -434,7 +434,6 @@ abstract contract ModuleManager is Storage, EIP712, IModuleManagerEventsAndError
uint256 missingAccountFunds
)
internal
view
virtual
returns (bytes32 postHash, bytes memory postSig)
{
Expand All @@ -443,7 +442,7 @@ abstract contract ModuleManager is Storage, EIP712, IModuleManagerEventsAndError
// If no pre-validation hook is installed, return the original hash and signature
if (preValidationHook == address(0)) return (hash, userOp.signature);
// Otherwise, call the pre-validation hook and return the updated hash and signature
else return IPreValidationHookERC4337(preValidationHook).preValidationHookERC4337(address(this), userOp, missingAccountFunds, hash);
else return IPreValidationHookERC4337(preValidationHook).preValidationHookERC4337(userOp, missingAccountFunds, hash);
}

/// @notice Checks if an enable mode signature is valid.
Expand Down
5 changes: 1 addition & 4 deletions contracts/interfaces/modules/IPreValidationHook.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { IModule } from "./IModule.sol";
interface IPreValidationHookERC1271 is IModule {
/// @notice Performs pre-validation checks for isValidSignature
/// @dev This method is called before the validation of a signature on a validator within isValidSignature
/// @param account The account calling the hook
/// @param account The account to validate the signature for
/// @param sender The original sender of the request
/// @param hash The hash of signed data
/// @param data The signature data to validate
Expand All @@ -31,19 +31,16 @@ interface IPreValidationHookERC1271 is IModule {
interface IPreValidationHookERC4337 is IModule {
/// @notice Performs pre-validation checks for user operations
/// @dev This method is called before the validation of a user operation
/// @param account The account calling the hook
/// @param userOp The user operation to be validated
/// @param missingAccountFunds The amount of funds missing in the account
/// @param userOpHash The hash of the user operation data
/// @return hookHash The hash after applying the pre-validation hook
/// @return hookSignature The signature after applying the pre-validation hook
function preValidationHookERC4337(
address account,
PackedUserOperation calldata userOp,
uint256 missingAccountFunds,
bytes32 userOpHash
)
external
view
returns (bytes32 hookHash, bytes memory hookSignature);
}
20 changes: 20 additions & 0 deletions contracts/mocks/Mock7739PreValidationHook.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,26 @@ contract Mock7739PreValidationHook is IPreValidationHookERC1271 {
bytes32 internal constant _PERSONAL_SIGN_TYPEHASH = 0x983e65e5148e570cd828ead231ee759a8d7958721a768f93bc4483ba005c32de;
bytes32 internal constant _DOMAIN_TYPEHASH = 0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f;

address public immutable prevalidationHookMultiplexer;

constructor(address _prevalidationHookMultiplexer) {
prevalidationHookMultiplexer = _prevalidationHookMultiplexer;
}

function _msgSender() internal view returns (address sender) {
if (isTrustedForwarder(msg.sender) && msg.data.length >= 20) {
assembly {
sender := shr(96, calldataload(sub(calldatasize(), 20)))
}
} else {
return msg.sender;
}
}

function isTrustedForwarder(address forwarder) public view returns (bool) {
return forwarder == prevalidationHookMultiplexer;
}

function preValidationHookERC1271(
address account,
address,
Expand Down
1 change: 0 additions & 1 deletion contracts/mocks/MockPreValidationHook.sol
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ contract MockPreValidationHook is IPreValidationHookERC1271, IPreValidationHookE
}

function preValidationHookERC4337(
address,
PackedUserOperation calldata userOp,
uint256,
bytes32 userOpHash
Expand Down
30 changes: 16 additions & 14 deletions contracts/mocks/MockPreValidationHookMultiplexer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ contract MockPreValidationHookMultiplexer is IPreValidationHookERC1271, IPreVali
}

// Separate configurations for each hook type
mapping(address account => mapping(uint256 hookType => HookConfig)) internal accountConfig;
mapping(uint256 hookType => mapping(address account => HookConfig)) internal accountConfig;

error AlreadyInitialized(uint256 hookType);
error NotInitialized(uint256 hookType);
error InvalidHookType(uint256 hookType);
error OnInstallFailed(address hook);
error OnUninstallFailed(address hook);
error SubHookFailed(address hook);

function onInstall(bytes calldata data) external {
(uint256 moduleType, address[] memory hooks, bytes[] memory hookData) = abi.decode(data, (uint256, address[], bytes[]));
Expand All @@ -26,12 +27,12 @@ contract MockPreValidationHookMultiplexer is IPreValidationHookERC1271, IPreVali
revert InvalidHookType(moduleType);
}

if (accountConfig[msg.sender][moduleType].initialized) {
if (accountConfig[moduleType][msg.sender].initialized) {
revert AlreadyInitialized(moduleType);
}

accountConfig[msg.sender][moduleType].hooks = hooks;
accountConfig[msg.sender][moduleType].initialized = true;
accountConfig[moduleType][msg.sender].hooks = hooks;
accountConfig[moduleType][msg.sender].initialized = true;

for (uint256 i = 0; i < hooks.length; i++) {
bytes memory subHookOnInstallCalldata = abi.encodeCall(IModule.onInstall, hookData[i]);
Expand All @@ -47,9 +48,9 @@ contract MockPreValidationHookMultiplexer is IPreValidationHookERC1271, IPreVali
revert InvalidHookType(moduleType);
}

address[] memory hooks = accountConfig[msg.sender][moduleType].hooks;
address[] memory hooks = accountConfig[moduleType][msg.sender].hooks;

delete accountConfig[msg.sender][moduleType];
delete accountConfig[moduleType][msg.sender];

for (uint256 i = 0; i < hooks.length; i++) {
bytes memory subHookOnUninstallCalldata = abi.encodeCall(IModule.onUninstall, hookData[i]);
Expand All @@ -59,16 +60,14 @@ contract MockPreValidationHookMultiplexer is IPreValidationHookERC1271, IPreVali
}

function preValidationHookERC4337(
address account,
PackedUserOperation calldata userOp,
uint256 missingAccountFunds,
bytes32 userOpHash
)
external
view
returns (bytes32 hookHash, bytes memory hookSignature)
{
HookConfig storage config = accountConfig[msg.sender][MODULE_TYPE_PREVALIDATION_HOOK_ERC4337];
HookConfig storage config = accountConfig[MODULE_TYPE_PREVALIDATION_HOOK_ERC4337][msg.sender];

if (!config.initialized) {
revert NotInitialized(MODULE_TYPE_PREVALIDATION_HOOK_ERC4337);
Expand All @@ -79,7 +78,10 @@ contract MockPreValidationHookMultiplexer is IPreValidationHookERC1271, IPreVali
PackedUserOperation memory op = userOp;

for (uint256 i = 0; i < config.hooks.length; i++) {
(hookHash, hookSignature) = IPreValidationHookERC4337(config.hooks[i]).preValidationHookERC4337(account, op, missingAccountFunds, hookHash);
bytes memory subHookData = abi.encodeWithSelector(IPreValidationHookERC4337.preValidationHookERC4337.selector, op, missingAccountFunds, hookHash);
(bool success, bytes memory result) = config.hooks[i].call(abi.encodePacked(subHookData, msg.sender));
require(success, SubHookFailed(config.hooks[i]));
(hookHash, hookSignature) = abi.decode(result, (bytes32, bytes));
op.signature = hookSignature;
}

Expand All @@ -96,7 +98,7 @@ contract MockPreValidationHookMultiplexer is IPreValidationHookERC1271, IPreVali
view
returns (bytes32 hookHash, bytes memory hookSignature)
{
HookConfig storage config = accountConfig[msg.sender][MODULE_TYPE_PREVALIDATION_HOOK_ERC1271];
HookConfig storage config = accountConfig[MODULE_TYPE_PREVALIDATION_HOOK_ERC1271][msg.sender];

if (!config.initialized) {
revert NotInitialized(MODULE_TYPE_PREVALIDATION_HOOK_ERC1271);
Expand All @@ -118,12 +120,12 @@ contract MockPreValidationHookMultiplexer is IPreValidationHookERC1271, IPreVali

function isInitialized(address smartAccount) external view returns (bool) {
// Account is initialized if either hook type is initialized
return accountConfig[smartAccount][MODULE_TYPE_PREVALIDATION_HOOK_ERC4337].initialized
|| accountConfig[smartAccount][MODULE_TYPE_PREVALIDATION_HOOK_ERC1271].initialized;
return accountConfig[MODULE_TYPE_PREVALIDATION_HOOK_ERC4337][smartAccount].initialized
|| accountConfig[MODULE_TYPE_PREVALIDATION_HOOK_ERC1271][smartAccount].initialized;
}

function isHookTypeInitialized(address smartAccount, uint256 hookType) external view returns (bool) {
return accountConfig[smartAccount][hookType].initialized;
return accountConfig[hookType][smartAccount].initialized;
}

function isValidModuleType(uint256 moduleTypeId) internal pure returns (bool) {
Expand Down
2 changes: 1 addition & 1 deletion contracts/mocks/MockResourceLockPreValidationHook.sol
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ contract MockResourceLockPreValidationHook is IPreValidationHookERC4337, IPreVal
}

function preValidationHookERC4337(
address account,
PackedUserOperation calldata userOp,
uint256 missingAccountFunds,
bytes32 userOpHash
Expand All @@ -75,6 +74,7 @@ contract MockResourceLockPreValidationHook is IPreValidationHookERC4337, IPreVal
view
returns (bytes32 hookHash, bytes memory hookSignature)
{
address account = _msgSender();
require(enoughETHAvailable(account, missingAccountFunds), InsufficientUnlockedETH(missingAccountFunds));
return (userOpHash, userOp.signature);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ contract TestNexusPreValidation_Integration_HookMultiplexer is TestModuleManagem

// Deploy supporting contracts
accountLocker = new MockAccountLocker();
erc7739Hook = new Mock7739PreValidationHook();
hookMultiplexer = new MockPreValidationHookMultiplexer();
erc7739Hook = new Mock7739PreValidationHook(address(hookMultiplexer));
resourceLockHook = new MockResourceLockPreValidationHook(address(accountLocker), address(hookMultiplexer));
// Deploy the simple validator
SIMPLE_VALIDATOR = new MockSimpleValidator();
Expand Down Expand Up @@ -132,7 +132,7 @@ contract TestNexusPreValidation_Integration_HookMultiplexer is TestModuleManagem
test_installMultiplePreValidationHooks();

// Lock resources
vm.prank(address(accountLocker));

MockAccountLocker(accountLocker).setLockedAmount(address(BOB_ACCOUNT), address(this), 1);

// Prepare test data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ contract TestNexusPreValidation_Integration_ResourceLockHooks is TestModuleManag
userOps[0] = buildUserOpWithCalldata(BOB, "", address(VALIDATOR_MODULE));

// Set locked amount to block ETH transactions
vm.prank(address(accountLocker));

MockAccountLocker(accountLocker).setLockedAmount(address(BOB_ACCOUNT), NATIVE_TOKEN, lockedAmount);

// Ensure account has enough total balance
Expand Down Expand Up @@ -96,7 +96,7 @@ contract TestNexusPreValidation_Integration_ResourceLockHooks is TestModuleManag
userOps[0] = buildUserOpWithCalldata(BOB, "", address(VALIDATOR_MODULE));

// Set locked amount
vm.prank(address(accountLocker));

MockAccountLocker(accountLocker).setLockedAmount(address(BOB_ACCOUNT), NATIVE_TOKEN, lockedAmount);

// Ensure account has enough total balance
Expand Down Expand Up @@ -153,7 +153,7 @@ contract TestNexusPreValidation_Integration_ResourceLockHooks is TestModuleManag
bytes memory validatorSignature = abi.encodePacked(address(VALIDATOR_MODULE), signature);

// Set locked amount to block signature validation
vm.prank(address(accountLocker));

MockAccountLocker(accountLocker).setLockedAmount(address(BOB_ACCOUNT), address(this), 1);

// Expect revert due to resource lock
Expand Down

0 comments on commit ab75991

Please sign in to comment.