From 1a83dd3dd5863f92b28ca76e94d61ebf2046de84 Mon Sep 17 00:00:00 2001 From: adam Date: Fri, 11 Oct 2024 19:39:38 -0400 Subject: [PATCH] feat: sig call buffers --- ...ularAccount_UserOp_deferredValidation.snap | 2 +- ...ModularAccount_Runtime_BatchTransfers.snap | 2 +- ...iModularAccount_Runtime_Erc20Transfer.snap | 2 +- ...count_Runtime_InstallSessionKey_Case1.snap | 2 +- ...ModularAccount_Runtime_NativeTransfer.snap | 2 +- ...t_Runtime_UseSessionKey_Case1_Counter.snap | 2 +- ...unt_Runtime_UseSessionKey_Case1_Token.snap | 2 +- ...iModularAccount_UserOp_BatchTransfers.snap | 2 +- ...miModularAccount_UserOp_Erc20Transfer.snap | 2 +- ...ccount_UserOp_InstallSessionKey_Case1.snap | 2 +- ...iModularAccount_UserOp_NativeTransfer.snap | 2 +- ...nt_UserOp_UseSessionKey_Case1_Counter.snap | 2 +- ...ount_UserOp_UseSessionKey_Case1_Token.snap | 2 +- ...ularAccount_UserOp_deferredValidation.snap | 2 +- src/account/ModularAccountBase.sol | 44 ++-- src/account/SemiModularAccountBase.sol | 16 +- src/libraries/ExecutionLib.sol | 179 ++++++++++++++- src/libraries/MemManagementLib.sol | 18 ++ test/account/PHCallBuffers.t.sol | 6 +- test/account/SigCallBuffer.t.sol | 215 ++++++++++++++++++ 20 files changed, 456 insertions(+), 50 deletions(-) create mode 100644 test/account/SigCallBuffer.t.sol diff --git a/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap b/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap index 718792f45..0ceea7995 100644 --- a/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap +++ b/.forge-snapshots/ModularAccount_UserOp_deferredValidation.snap @@ -1 +1 @@ -257468 \ No newline at end of file +257405 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_BatchTransfers.snap b/.forge-snapshots/SemiModularAccount_Runtime_BatchTransfers.snap index 5ed4b1238..8c9c3be3c 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_BatchTransfers.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_BatchTransfers.snap @@ -1 +1 @@ -89286 \ No newline at end of file +89189 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap b/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap index 1325eed1c..8231166ca 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_Erc20Transfer.snap @@ -1 +1 @@ -74791 \ No newline at end of file +74706 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap b/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap index 663b4046c..4ac435a65 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_InstallSessionKey_Case1.snap @@ -1 +1 @@ -422778 \ No newline at end of file +422693 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap b/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap index 57af4b6e4..16132bb93 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_NativeTransfer.snap @@ -1 +1 @@ -50950 \ No newline at end of file +50865 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap index faf693d81..da9056908 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -80029 \ No newline at end of file +79969 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap index 68ed10e94..eb30ea85a 100644 --- a/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -113587 \ No newline at end of file +113527 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap b/.forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap index 7e6925065..8b587a17c 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap @@ -1 +1 @@ -193755 \ No newline at end of file +193634 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap b/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap index e9fdcd8ff..e216c182b 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap @@ -1 +1 @@ -180839 \ No newline at end of file +180730 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap b/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap index d78c62286..21cd441dd 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap @@ -1 +1 @@ -529934 \ No newline at end of file +529825 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap b/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap index a509ee65c..f764a4201 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap @@ -1 +1 @@ -157114 \ No newline at end of file +157005 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap index 8426f1a20..0cc94b198 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap @@ -1 +1 @@ -196336 \ No newline at end of file +196156 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap index 14840318b..dfc004ff3 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap @@ -1 +1 @@ -227472 \ No newline at end of file +227292 \ No newline at end of file diff --git a/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap b/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap index 580debc10..eb5972c28 100644 --- a/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap +++ b/.forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap @@ -1 +1 @@ -254088 \ No newline at end of file +254023 \ No newline at end of file diff --git a/src/account/ModularAccountBase.sol b/src/account/ModularAccountBase.sol index e61b2f5c5..bf0fcd476 100644 --- a/src/account/ModularAccountBase.sol +++ b/src/account/ModularAccountBase.sol @@ -9,8 +9,6 @@ import { ModuleEntity, ValidationConfig } from "@erc6900/reference-implementation/interfaces/IModularAccount.sol"; -import {IValidationHookModule} from "@erc6900/reference-implementation/interfaces/IValidationHookModule.sol"; -import {IValidationModule} from "@erc6900/reference-implementation/interfaces/IValidationModule.sol"; import {HookConfig, HookConfigLib} from "@erc6900/reference-implementation/libraries/HookConfigLib.sol"; import {ModuleEntityLib} from "@erc6900/reference-implementation/libraries/ModuleEntityLib.sol"; import {ValidationConfigLib} from "@erc6900/reference-implementation/libraries/ValidationConfigLib.sol"; @@ -32,10 +30,11 @@ import { ExecutionLib, PHCallBuffer, RTCallBuffer, + SigCallBuffer, UOCallBuffer } from "../libraries/ExecutionLib.sol"; import {LinkedListSet, LinkedListSetLib} from "../libraries/LinkedListSetLib.sol"; -import {MemManagementLib} from "../libraries/MemManagementLib.sol"; +import {MemManagementLib, MemSnapshot} from "../libraries/MemManagementLib.sol"; import {SparseCalldataSegmentLib} from "../libraries/SparseCalldataSegmentLib.sol"; import {AccountStorage, getAccountStorage, toSetValue} from "./AccountStorage.sol"; import {AccountStorageInitializable} from "./AccountStorageInitializable.sol"; @@ -539,9 +538,12 @@ abstract contract ModularAccountBase is uoValidation ); + // Clear the memory after performing signature validation + MemSnapshot memSnapshot = MemManagementLib.freezeFMP(); if (_isValidSignature(sigValidation, typedDataHash, sig) != _1271_MAGIC_VALUE) { revert DeferredActionSignatureInvalid(); } + MemManagementLib.restoreFMP(memSnapshot); return (deadline, uoValidation); } @@ -751,45 +753,45 @@ abstract contract ModularAccountBase is HookConfig[] memory preSignatureValidationHooks = MemManagementLib.loadValidationHooks(getAccountStorage().validationData[sigValidation]); + SigCallBuffer sigCallBuffer; + if (!_validationIsNative(sigValidation) || preSignatureValidationHooks.length > 0) { + sigCallBuffer = ExecutionLib.allocateSigCallBuffer(hash, signature); + } for (uint256 i = preSignatureValidationHooks.length; i > 0;) { // Decrement here, instead of in the loop body, to convert from length to an index. unchecked { --i; } - (address hookModule, uint32 hookEntityId) = preSignatureValidationHooks[i].moduleEntity().unpack(); - - bytes memory currentSignatureSegment; + bytes calldata currentSignatureSegment; (currentSignatureSegment, signature) = signature.advanceSegmentIfAtIndex(uint8(preSignatureValidationHooks.length - i - 1)); - // If this reverts, bubble up revert reason. - IValidationHookModule(hookModule).preSignatureValidationHook( - hookEntityId, msg.sender, hash, currentSignatureSegment + ExecutionLib.invokePreSignatureValidationHook( + sigCallBuffer, preSignatureValidationHooks[i], currentSignatureSegment ); } signature = signature.getFinalSegment(); - return _exec1271Validation(sigValidation, hash, signature); + + return _exec1271Validation(sigCallBuffer, hash, sigValidation, signature); } - function _exec1271Validation(ModuleEntity sigValidation, bytes32 hash, bytes calldata signature) - internal - view - virtual - returns (bytes4) - { + function _exec1271Validation( + SigCallBuffer buffer, + bytes32 hash, + ModuleEntity sigValidation, + bytes calldata signatureSegment + ) internal view virtual returns (bytes4) { + (hash); // unused AccountStorage storage _storage = getAccountStorage(); - (address module, uint32 entityId) = sigValidation.unpack(); if (!_storage.validationData[sigValidation].isSignatureValidation) { + (address module, uint32 entityId) = sigValidation.unpack(); revert SignatureValidationInvalid(module, entityId); } - if ( - IValidationModule(module).validateSignature(address(this), entityId, msg.sender, hash, signature) - == _1271_MAGIC_VALUE - ) { + if (ExecutionLib.invokeSignatureValidation(buffer, sigValidation, signatureSegment) == _1271_MAGIC_VALUE) { return _1271_MAGIC_VALUE; } return _1271_INVALID; diff --git a/src/account/SemiModularAccountBase.sol b/src/account/SemiModularAccountBase.sol index b9d8f4521..85897470c 100644 --- a/src/account/SemiModularAccountBase.sol +++ b/src/account/SemiModularAccountBase.sol @@ -11,7 +11,7 @@ import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/Signa import {DIRECT_CALL_VALIDATION_ENTITYID, FALLBACK_VALIDATION} from "../helpers/Constants.sol"; import {SignatureType} from "../helpers/SignatureType.sol"; -import {RTCallBuffer, UOCallBuffer} from "../libraries/ExecutionLib.sol"; +import {RTCallBuffer, SigCallBuffer, UOCallBuffer} from "../libraries/ExecutionLib.sol"; import {SemiModularKnownSelectorsLib} from "../libraries/SemiModularKnownSelectorsLib.sol"; import {ModularAccountBase} from "./ModularAccountBase.sol"; @@ -134,12 +134,12 @@ abstract contract SemiModularAccountBase is ModularAccountBase { } } - function _exec1271Validation(ModuleEntity sigValidation, bytes32 hash, bytes calldata signature) - internal - view - override - returns (bytes4) - { + function _exec1271Validation( + SigCallBuffer buffer, + bytes32 hash, + ModuleEntity sigValidation, + bytes calldata signature + ) internal view override returns (bytes4) { if (sigValidation.eq(FALLBACK_VALIDATION)) { address fallbackSigner = _getFallbackSigner(); @@ -148,7 +148,7 @@ abstract contract SemiModularAccountBase is ModularAccountBase { } return _1271_INVALID; } - return super._exec1271Validation(sigValidation, hash, signature); + return super._exec1271Validation(buffer, hash, sigValidation, signature); } function _checkSignature(address owner, bytes32 digest, bytes calldata sig) internal view returns (bool) { diff --git a/src/libraries/ExecutionLib.sol b/src/libraries/ExecutionLib.sol index 77d057ef9..c4177330a 100644 --- a/src/libraries/ExecutionLib.sol +++ b/src/libraries/ExecutionLib.sol @@ -18,6 +18,8 @@ type RTCallBuffer is bytes32; type PHCallBuffer is bytes32; +type SigCallBuffer is bytes32; + type DensePostHookData is bytes32; using HookConfigLib for HookConfig; @@ -241,10 +243,7 @@ library ExecutionLib { result := buffer } - // Prepare the buffer for pre-runtime validation hooks. - _prepareRuntimeCallBufferPreValidationHooks(result); - - // Buffer contents: + // Buffer contents, before update: // 0xAAAAAAAA // selector // 0x000: 0x________________________BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB // account // 0x020: 0x________________________________________________________CCCCCCCC // entityId @@ -253,6 +252,9 @@ library ExecutionLib { // 0x080: 0x______________________________________________________________c0 // callData offset // 0x0a0: 0x_____________________________________________________________FFF // authorization offset // 0x0c0... // dynamic fields + + // Prepare the buffer for pre-runtime validation hooks. + _prepareRuntimeCallBufferPreValidationHooks(result); } function invokeRuntimeCallBufferPreValidationHook( @@ -811,6 +813,161 @@ library ExecutionLib { } } + function allocateSigCallBuffer(bytes32 hash, bytes calldata signature) + internal + view + returns (SigCallBuffer result) + { + bytes memory buffer = abi.encodeCall( + IValidationModule.validateSignature, (address(0), uint32(0), msg.sender, hash, signature) + ); + + assembly ("memory-safe") { + result := buffer + } + + // Buffer contents, before update: + // 0xAAAAAAAA // selector + // 0x000: 0x________________________BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB // account + // 0x020: 0x________________________________________________________CCCCCCCC // entityId + // 0x040: 0x________________________DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD // msg.sender + // 0x060: 0xEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE // hash + // 0x080: 0x______________________________________________________________a0 // signature offset + // 0x0a0... // dynamic fields + + // Prepare the buffer for pre-signature validation hooks. + _prepareSigValidationCallBufferPreSigValidationHooks(result); + } + + function invokePreSignatureValidationHook( + SigCallBuffer buffer, + HookConfig hookEntity, + bytes calldata signatureSegment + ) internal view { + assembly ("memory-safe") { + // Load the module address and entity id + let entityId := and(shr(64, hookEntity), 0xffffffff) + let moduleAddress := shr(96, hookEntity) + + // Update the buffer with the entity Id + mstore(add(buffer, 0x44), entityId) + + // Copy in the signature segment + // Since the buffer's copy of the signature exceeds the length of any sub-segments, we can safely write + // over it. + mstore(add(buffer, 0xc4), signatureSegment.length) + + // If there is a nonzero signature segment length, copy in the data. + if signatureSegment.length { + // Because we will be sending the data with word-aligned padding ("strict ABI encoding"), we need + // to zero out the last word of the buffer to prevent sending garbage data. + let roundedDownSignatureLength := and(signatureSegment.length, not(0x1f)) + mstore(add(add(buffer, 0xe4), roundedDownSignatureLength), 0) + // Copy in the data + calldatacopy(add(buffer, 0xe4), signatureSegment.offset, signatureSegment.length) + } + + // The data amount we actually want to call with is: + // 0xa4 (4 byte selector + 5 words of data: entity id, sender, hash, signature offset, signature + // length) + word-align(signature length) + let actualCallLength := add(0xa4, and(add(signatureSegment.length, 0x1f), not(0x1f))) + + // Perform the call + let success := + staticcall( + gas(), + moduleAddress, + /*argOffset*/ + add(buffer, 0x40), // jump over 32 bytes for length, and another 32 bytes for the account + /*argSize*/ + actualCallLength, + /*retOffset*/ + 0, + /*retSize*/ + 0x20 + ) + + if iszero(success) { + // Bubble up the revert if the call reverts. + let m := mload(0x40) + returndatacopy(m, 0, returndatasize()) + revert(m, returndatasize()) + } + } + } + + function invokeSignatureValidation( + SigCallBuffer buffer, + ModuleEntity validationFunction, + bytes calldata signatureSegment + ) internal view returns (bytes4 result) { + assembly ("memory-safe") { + // Load the module address and entity id + let entityId := and(shr(64, validationFunction), 0xffffffff) + let moduleAddress := shr(96, validationFunction) + + // Store the account in the `account` field. + mstore(add(buffer, 0x24), address()) + + // Update the buffer with the entity Id + mstore(add(buffer, 0x44), entityId) + + // Fix the calldata offsets of `signature`, due to including the `account` field for signature + // validation. + mstore(add(buffer, 0xa4), 0xa0) + + // Copy in the signature segment + // Since the buffer's copy of the signature exceeds the length of any sub-segments, we can safely write + // over it. + mstore(add(buffer, 0xc4), signatureSegment.length) + + // If there is a nonzero signature segment length, copy in the data. + if signatureSegment.length { + // Because we will be sending the data with word-aligned padding ("strict ABI encoding"), we need + // to zero out the last word of the buffer to prevent sending garbage data. + let roundedDownSignatureLength := and(signatureSegment.length, not(0x1f)) + mstore(add(add(buffer, 0xe4), roundedDownSignatureLength), 0) + // Copy in the data + calldatacopy(add(buffer, 0xe4), signatureSegment.offset, signatureSegment.length) + } + + // The data amount we actually want to call with is: + // 0xc4 (4 byte selector + 6 words of data: account, entity id, sender, hash, signature offset, + // signature length) + word-align(signature length) + let actualCallLength := add(0xc4, and(add(signatureSegment.length, 0x1f), not(0x1f))) + + // Perform the call + switch and( + gt(returndatasize(), 0x1f), + staticcall( + gas(), + moduleAddress, + /*argOffset*/ + add(buffer, 0x20), // jump over 32 bytes for length, and another 32 bytes for the account + /*argSize*/ + actualCallLength, + /*retOffset*/ + 0, + /*retSize*/ + 0x20 + ) + ) + case 0 { + // Bubble up the revert if the call reverts. + let m := mload(0x40) + returndatacopy(m, 0, returndatasize()) + revert(m, returndatasize()) + } + default { + // Otherwise, we return the first word of the return data as the signature validation result + result := mload(0) + + // If any of the lower 28 bytes are nonzero, it would be an abi decoding failure. + if shl(32, result) { revert(0, 0) } + } + } + } + /// @return The new working memory pointer function _appendPostHookToRun(bytes32 workingMemPtr, HookConfig hookConfig, uint256 returnedBytesSize) private @@ -893,4 +1050,18 @@ library ExecutionLib { mstore(authorizationOffsetPtr, sub(authorizationOffset, 0x20)) } } + + function _prepareSigValidationCallBufferPreSigValidationHooks(SigCallBuffer buffer) private pure { + uint32 selector = uint32(IValidationHookModule.preSignatureValidationHook.selector); + assembly ("memory-safe") { + // Update the buffer with the selector. This will squash a portion of the `account` param for runtime + // validation, but that will be restored before calling. + mstore(add(buffer, 0x24), selector) + + // Fix the calldata offset of `signature`, due to excluding the `account` field. + + // The offset of the signature starts out as 0xa0, but for pre-validation hooks, it should be 0x80. + mstore(add(buffer, 0xa4), 0x80) + } + } } diff --git a/src/libraries/MemManagementLib.sol b/src/libraries/MemManagementLib.sol index ccb2056a0..8f9453ba6 100644 --- a/src/libraries/MemManagementLib.sol +++ b/src/libraries/MemManagementLib.sol @@ -6,6 +6,8 @@ import {HookConfig} from "@erc6900/reference-implementation/interfaces/IModularA import {ExecutionData, ValidationData} from "../account/AccountStorage.sol"; import {LinkedListSet, LinkedListSetLib, SENTINEL_VALUE, SetValue} from "./LinkedListSetLib.sol"; +type MemSnapshot is uint256; + library MemManagementLib { function loadExecHooks(ExecutionData storage execData, ValidationData storage valData) internal @@ -148,6 +150,22 @@ library MemManagementLib { return target; } + function freezeFMP() internal pure returns (MemSnapshot) { + MemSnapshot snapshot; + + assembly ("memory-safe") { + snapshot := mload(0x40) + } + + return snapshot; + } + + function restoreFMP(MemSnapshot snapshot) internal pure { + assembly ("memory-safe") { + mstore(0x40, snapshot) + } + } + // Used to load both pre-validation hooks and pre-execution hooks, associated with a validation function. // The caller must first get the length of the hooks from the ValidationData struct. function _loadValidationAssociatedHooks(uint256 hookCount, LinkedListSet storage hooks) diff --git a/test/account/PHCallBuffers.t.sol b/test/account/PHCallBuffers.t.sol index d317b9e4c..1f52f4079 100644 --- a/test/account/PHCallBuffers.t.sol +++ b/test/account/PHCallBuffers.t.sol @@ -94,7 +94,7 @@ contract PHCallBufferTest is AccountTestBase { // pre hooks in `executeWithRuntimeValidation` (freshly allocated buffer from SMA) // SMA allows for skipping allocation of an RT call buffer, even in the RT validation case. - function test_preExecHooksWithRtValidation_freshBuffer_regularCallData() public withSMATest { + function test_preExecHooksWithRtValidation_freshBuffer_regularCallData() public { _switchToSMA(); _allowTestDirectCalls(); _install3ValAssocExecHooks(); @@ -121,7 +121,7 @@ contract PHCallBufferTest is AccountTestBase { // SMA allows for skipping allocation of an RT call buffer, even in the RT validation case. // This alternate version of the tests checks behavior when the provided calldata is incorrectly abi-encoded // and not word-aligned. - function test_preExecHooksWithRtValidation_freshBuffer_unalignedCallData() public withSMATest { + function test_preExecHooksWithRtValidation_freshBuffer_unalignedCallData() public { _switchToSMA(); _allowTestDirectCalls(); _install3ValAssocExecHooks(); @@ -145,7 +145,7 @@ contract PHCallBufferTest is AccountTestBase { account1.executeWithRuntimeValidation(callData, authorization); } - function test_preExecHooksWithRtValidation_reusePRTOnlyBuffer_regularCalldata() public withSMATest { + function test_preExecHooksWithRtValidation_reusePRTOnlyBuffer_regularCalldata() public { _switchToSMA(); _allowTestDirectCalls(); _install3ValAssocExecHooks(); diff --git a/test/account/SigCallBuffer.t.sol b/test/account/SigCallBuffer.t.sol new file mode 100644 index 000000000..a0696db1e --- /dev/null +++ b/test/account/SigCallBuffer.t.sol @@ -0,0 +1,215 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.26; + +import {ExecutionManifest} from "@erc6900/reference-implementation/interfaces/IExecutionModule.sol"; +import {IValidationHookModule} from "@erc6900/reference-implementation/interfaces/IValidationHookModule.sol"; +import {IValidationModule} from "@erc6900/reference-implementation/interfaces/IValidationModule.sol"; +import {HookConfig, HookConfigLib} from "@erc6900/reference-implementation/libraries/HookConfigLib.sol"; +import {ModuleEntity, ModuleEntityLib} from "@erc6900/reference-implementation/libraries/ModuleEntityLib.sol"; +import { + ValidationConfig, + ValidationConfigLib +} from "@erc6900/reference-implementation/libraries/ValidationConfigLib.sol"; +import {IERC1271} from "@openzeppelin/contracts/interfaces/IERC1271.sol"; + +import {FALLBACK_VALIDATION} from "../../src/helpers/Constants.sol"; + +import {MockModule} from "../mocks/modules/MockModule.sol"; +import {AccountTestBase} from "../utils/AccountTestBase.sol"; + +contract SigCallBufferTest is AccountTestBase { + using ValidationConfigLib for ValidationConfig; + // installed entity id is their index + + MockModule[] public validationHooks; + + // installed with entity id 0 + MockModule public validationModule; + + ModuleEntity internal _validationFunction; + + struct FuzzConfig { + uint8 validationHookCount; + bytes[256] preValidationHookData; + bytes signature; + } + + function setUp() public override { + _allowTestDirectCalls(); + } + + function test_sigCallBuffer_noData() public withSMATest { + bytes32 hash = keccak256("test"); + + _setUp4ValidationHooks(); + + for (uint256 i = 0; i < 3; i++) { + vm.expectCall( + address(validationHooks[i]), + abi.encodeCall( + IValidationHookModule.preSignatureValidationHook, (uint32(i), beneficiary, hash, "") + ) + ); + } + + if (!_isSMATest) { + vm.expectCall( + address(validationModule), + abi.encodeCall( + IValidationModule.validateSignature, + (address(account1), uint32(0), beneficiary, hash, abi.encodePacked(EOA_TYPE_SIGNATURE)) + ) + ); + } + + vm.prank(beneficiary); + account1.isValidSignature( + hash, _encode1271Signature(_validationFunction, abi.encodePacked(EOA_TYPE_SIGNATURE)) + ); + } + + function test_sigCallBuffer_withData() public withSMATest { + bytes32 hash = keccak256("test"); + + FuzzConfig memory fuzzConfig; + fuzzConfig.validationHookCount = 4; + fuzzConfig.signature = "abcdefghijklmnopqrstuvwxyz"; + + fuzzConfig.preValidationHookData[0] = hex"abcd"; + fuzzConfig.preValidationHookData[1] = hex""; + fuzzConfig.preValidationHookData[2] = hex"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; + fuzzConfig.preValidationHookData[3] = + hex"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffabcd"; + + _setUp4ValidationHooks(); + + _expectCalls(fuzzConfig, hash); + + PreValidationHookData[] memory preValidationHookDatasToSend = _generatePreHooksDatasArray(fuzzConfig); + + bytes memory signature; + if (_isSMATest) { + // Signature will fail, but not revert. + signature = abi.encodePacked(EOA_TYPE_SIGNATURE); + } else { + signature = fuzzConfig.signature; + } + vm.prank(beneficiary); + account1.isValidSignature( + hash, _encode1271Signature(_validationFunction, preValidationHookDatasToSend, signature) + ); + } + + function testFuzz_sigCallBuffer(bytes32 hash, FuzzConfig memory fuzzConfig) public withSMATest { + _installValidationAndAssocHook(fuzzConfig); + + _expectCalls(fuzzConfig, hash); + + PreValidationHookData[] memory preValidationHookDatasToSend = _generatePreHooksDatasArray(fuzzConfig); + + bytes memory signature; + if (_isSMATest) { + // Signature will fail, but not revert. + signature = abi.encodePacked(EOA_TYPE_SIGNATURE); + } else { + signature = fuzzConfig.signature; + } + + vm.prank(beneficiary); + account1.isValidSignature( + hash, _encode1271Signature(_validationFunction, preValidationHookDatasToSend, signature) + ); + } + + function _setUp4ValidationHooks() internal { + FuzzConfig memory fuzzConfig; + fuzzConfig.validationHookCount = 4; + + _installValidationAndAssocHook(fuzzConfig); + } + + function _installValidationAndAssocHook(FuzzConfig memory fuzzConfig) internal { + HookConfig[] memory hooks = new HookConfig[](fuzzConfig.validationHookCount); + bytes[] memory hookInstalls = new bytes[](fuzzConfig.validationHookCount); + + ExecutionManifest memory m; // empty manifest + validationHooks = new MockModule[](fuzzConfig.validationHookCount); + + for (uint256 i = 0; i < fuzzConfig.validationHookCount; i++) { + // To get different addresses for vm.expectCall between SMA and non-SMA tests, we need to deploy new + // the validation hooks at different addresses + validationHooks[i] = new MockModule{salt: keccak256(abi.encode(i, _isSMATest))}(m); + // These modules emit events, but signature validation happens as a staticcall, so we can't expect the + // events. Instead, we mock all of the calls coming into these contracts, and expect calls to them. They + // are also re-deployed for the SMA test, so the tests should be distinct. + vm.mockCall(address(validationHooks[i]), "", ""); + + hooks[i] = + HookConfigLib.packValidationHook({_module: address(validationHooks[i]), _entityId: uint32(i)}); + + hookInstalls[i] = abi.encodePacked(hooks[i]); + } + + validationModule = new MockModule(m); + + ValidationConfig validationConfig; + + if (_isSMATest) { + validationConfig = ValidationConfigLib.pack({ + _validationFunction: FALLBACK_VALIDATION, + _isGlobal: true, + _isSignatureValidation: true, + _isUserOpValidation: false + }); + } else { + validationConfig = ValidationConfigLib.pack({ + _module: address(validationModule), + _entityId: 0, + _isGlobal: true, + _isSignatureValidation: true, + _isUserOpValidation: false + }); + + vm.mockCall(address(validationModule), "", abi.encode(IERC1271.isValidSignature.selector)); + } + + _validationFunction = validationConfig.moduleEntity(); + + account1.installValidation(validationConfig, new bytes4[](0), "", hookInstalls); + } + + function _expectCalls(FuzzConfig memory fuzzConfig, bytes32 hash) internal { + for (uint256 i = 0; i < fuzzConfig.validationHookCount; i++) { + vm.expectCall( + address(validationHooks[i]), + abi.encodeCall( + IValidationHookModule.preSignatureValidationHook, + (uint32(i), beneficiary, hash, fuzzConfig.preValidationHookData[i]) + ) + ); + } + + if (!_isSMATest) { + vm.expectCall( + address(validationModule), + abi.encodeCall( + IValidationModule.validateSignature, + (address(account1), uint32(0), beneficiary, hash, fuzzConfig.signature) + ) + ); + } + } + + function _generatePreHooksDatasArray(FuzzConfig memory fuzzConfig) + internal + pure + returns (PreValidationHookData[] memory) + { + bytes[] memory hookDataDynamicArray = new bytes[](fuzzConfig.validationHookCount); + for (uint256 i = 0; i < fuzzConfig.validationHookCount; i++) { + hookDataDynamicArray[i] = fuzzConfig.preValidationHookData[i]; + } + + return _generatePreHooksDatasArray(hookDataDynamicArray); + } +}