From 79766e36f6e4239c2942080cb035c5852e32da82 Mon Sep 17 00:00:00 2001 From: Fangting Liu Date: Thu, 11 Jul 2024 15:49:59 -0700 Subject: [PATCH 1/2] rename validation res helper file and var names --- src/account/UpgradeableModularAccount.sol | 20 +++++++++---------- ...taHelpers.sol => ValidationResHelpers.sol} | 20 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) rename src/helpers/{ValidationDataHelpers.sol => ValidationResHelpers.sol} (72%) diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index da3ababb..ec075e8b 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -13,7 +13,7 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet import {PluginEntityLib} from "../helpers/PluginEntityLib.sol"; import {ValidationConfigLib} from "../helpers/ValidationConfigLib.sol"; import {SparseCalldataSegmentLib} from "../helpers/SparseCalldataSegmentLib.sol"; -import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationDataHelpers.sol"; +import {_coalescePreValidation, _coalesceValidation} from "../helpers/ValidationResHelpers.sol"; import {IPlugin, PluginManifest} from "../interfaces/IPlugin.sol"; import {IValidation} from "../interfaces/IValidation.sol"; import {IValidationHook} from "../interfaces/IValidationHook.sol"; @@ -405,7 +405,7 @@ contract UpgradeableModularAccount is bytes calldata signatureSegment; (signatureSegment, signature) = signature.getNextSegment(); - uint256 validationData; + uint256 validationRes; // Do preUserOpValidation hooks PluginEntity[] memory preUserOpValidationHooks = @@ -433,14 +433,14 @@ contract UpgradeableModularAccount is } (address plugin, uint32 entityId) = preUserOpValidationHooks[i].unpack(); - uint256 currentValidationData = + uint256 currentValidationRes = IValidationHook(plugin).preUserOpValidationHook(entityId, userOp, userOpHash); - if (uint160(currentValidationData) > 1) { + if (uint160(currentValidationRes) > 1) { // If the aggregator is not 0 or 1, it is an unexpected value - revert UnexpectedAggregator(plugin, entityId, address(uint160(currentValidationData))); + revert UnexpectedAggregator(plugin, entityId, address(uint160(currentValidationRes))); } - validationData = _coalescePreValidation(validationData, currentValidationData); + validationRes = _coalescePreValidation(validationRes, currentValidationRes); } // Run the user op validationFunction @@ -452,17 +452,17 @@ contract UpgradeableModularAccount is userOp.signature = signatureSegment.getBody(); (address plugin, uint32 entityId) = userOpValidationFunction.unpack(); - uint256 currentValidationData = IValidation(plugin).validateUserOp(entityId, userOp, userOpHash); + uint256 currentValidationRes = IValidation(plugin).validateUserOp(entityId, userOp, userOpHash); if (preUserOpValidationHooks.length != 0) { // If we have other validation data we need to coalesce with - validationData = _coalesceValidation(validationData, currentValidationData); + validationRes = _coalesceValidation(validationRes, currentValidationRes); } else { - validationData = currentValidationData; + validationRes = currentValidationRes; } } - return validationData; + return validationRes; } function _doRuntimeValidation( diff --git a/src/helpers/ValidationDataHelpers.sol b/src/helpers/ValidationResHelpers.sol similarity index 72% rename from src/helpers/ValidationDataHelpers.sol rename to src/helpers/ValidationResHelpers.sol index 3f61b19c..854d442c 100644 --- a/src/helpers/ValidationDataHelpers.sol +++ b/src/helpers/ValidationResHelpers.sol @@ -2,31 +2,31 @@ pragma solidity ^0.8.25; // solhint-disable-next-line private-vars-leading-underscore -function _coalescePreValidation(uint256 validationData1, uint256 validationData2) +function _coalescePreValidation(uint256 validationRes1, uint256 validationRes2) pure returns (uint256 resValidationData) { - uint48 validUntil1 = uint48(validationData1 >> 160); + uint48 validUntil1 = uint48(validationRes1 >> 160); if (validUntil1 == 0) { validUntil1 = type(uint48).max; } - uint48 validUntil2 = uint48(validationData2 >> 160); + uint48 validUntil2 = uint48(validationRes2 >> 160); if (validUntil2 == 0) { validUntil2 = type(uint48).max; } resValidationData = ((validUntil1 > validUntil2) ? uint256(validUntil2) << 160 : uint256(validUntil1) << 160); - uint48 validAfter1 = uint48(validationData1 >> 208); - uint48 validAfter2 = uint48(validationData2 >> 208); + uint48 validAfter1 = uint48(validationRes1 >> 208); + uint48 validAfter2 = uint48(validationRes2 >> 208); resValidationData |= ((validAfter1 < validAfter2) ? uint256(validAfter2) << 208 : uint256(validAfter1) << 208); // Once we know that the authorizer field is 0 or 1, we can safely bubble up SIG_FAIL with bitwise OR - resValidationData |= uint160(validationData1) | uint160(validationData2); + resValidationData |= uint160(validationRes1) | uint160(validationRes2); } // solhint-disable-next-line private-vars-leading-underscore -function _coalesceValidation(uint256 preValidationData, uint256 validationData) +function _coalesceValidation(uint256 preValidationData, uint256 validationRes) pure returns (uint256 resValidationData) { @@ -34,17 +34,17 @@ function _coalesceValidation(uint256 preValidationData, uint256 validationData) if (validUntil1 == 0) { validUntil1 = type(uint48).max; } - uint48 validUntil2 = uint48(validationData >> 160); + uint48 validUntil2 = uint48(validationRes >> 160); if (validUntil2 == 0) { validUntil2 = type(uint48).max; } resValidationData = ((validUntil1 > validUntil2) ? uint256(validUntil2) << 160 : uint256(validUntil1) << 160); uint48 validAfter1 = uint48(preValidationData >> 208); - uint48 validAfter2 = uint48(validationData >> 208); + uint48 validAfter2 = uint48(validationRes >> 208); resValidationData |= ((validAfter1 < validAfter2) ? uint256(validAfter2) << 208 : uint256(validAfter1) << 208); // If prevalidation failed, bubble up failure - resValidationData |= uint160(preValidationData) == 1 ? 1 : uint160(validationData); + resValidationData |= uint160(preValidationData) == 1 ? 1 : uint160(validationRes); } From 92cd259cb05a649b2fdbda9a87bca4b7c6c5060a Mon Sep 17 00:00:00 2001 From: Fangting Liu Date: Fri, 12 Jul 2024 09:32:22 -0700 Subject: [PATCH 2/2] rename test function name for validation result data handling --- test/account/ValidationIntersection.t.sol | 20 ++++++++++---------- test/utils/AccountTestBase.sol | 6 +++--- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index 3ab0df9b..3bdaa1b6 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -156,7 +156,7 @@ contract ValidationIntersectionTest is AccountTestBase { uint48 end2 = uint48(25); oneHookPlugin.setValidationData( - _packValidationData(address(0), start1, end1), _packValidationData(address(0), start2, end2) + _packValidationRes(address(0), start1, end1), _packValidationRes(address(0), start2, end2) ); PackedUserOperation memory userOp; @@ -167,7 +167,7 @@ contract ValidationIntersectionTest is AccountTestBase { vm.prank(address(entryPoint)); uint256 returnedValidationData = account1.validateUserOp(userOp, uoHash, 1 wei); - assertEq(returnedValidationData, _packValidationData(address(0), start2, end1)); + assertEq(returnedValidationData, _packValidationRes(address(0), start2, end1)); } function test_validationIntersect_timeBounds_intersect_2() public { @@ -178,7 +178,7 @@ contract ValidationIntersectionTest is AccountTestBase { uint48 end2 = uint48(25); oneHookPlugin.setValidationData( - _packValidationData(address(0), start2, end2), _packValidationData(address(0), start1, end1) + _packValidationRes(address(0), start2, end2), _packValidationRes(address(0), start1, end1) ); PackedUserOperation memory userOp; @@ -189,7 +189,7 @@ contract ValidationIntersectionTest is AccountTestBase { vm.prank(address(entryPoint)); uint256 returnedValidationData = account1.validateUserOp(userOp, uoHash, 1 wei); - assertEq(returnedValidationData, _packValidationData(address(0), start2, end1)); + assertEq(returnedValidationData, _packValidationRes(address(0), start2, end1)); } function test_validationIntersect_revert_unexpectedAuthorizer() public { @@ -247,7 +247,7 @@ contract ValidationIntersectionTest is AccountTestBase { address goodAuthorizer = makeAddr("goodAuthorizer"); oneHookPlugin.setValidationData( - _packValidationData(goodAuthorizer, start1, end1), _packValidationData(address(0), start2, end2) + _packValidationRes(goodAuthorizer, start1, end1), _packValidationRes(address(0), start2, end2) ); PackedUserOperation memory userOp; @@ -258,7 +258,7 @@ contract ValidationIntersectionTest is AccountTestBase { vm.prank(address(entryPoint)); uint256 returnedValidationData = account1.validateUserOp(userOp, uoHash, 1 wei); - assertEq(returnedValidationData, _packValidationData(goodAuthorizer, start2, end1)); + assertEq(returnedValidationData, _packValidationRes(goodAuthorizer, start2, end1)); } function test_validationIntersect_multiplePreValidationHooksIntersect() public { @@ -270,8 +270,8 @@ contract ValidationIntersectionTest is AccountTestBase { twoHookPlugin.setValidationData( 0, // returns OK - _packValidationData(address(0), start1, end1), - _packValidationData(address(0), start2, end2) + _packValidationRes(address(0), start1, end1), + _packValidationRes(address(0), start2, end2) ); PackedUserOperation memory userOp; @@ -282,7 +282,7 @@ contract ValidationIntersectionTest is AccountTestBase { vm.prank(address(entryPoint)); uint256 returnedValidationData = account1.validateUserOp(userOp, uoHash, 1 wei); - assertEq(returnedValidationData, _packValidationData(address(0), start2, end1)); + assertEq(returnedValidationData, _packValidationRes(address(0), start2, end1)); } function test_validationIntersect_multiplePreValidationHooksSigFail() public { @@ -318,7 +318,7 @@ contract ValidationIntersectionTest is AccountTestBase { validAfter = uint48(validationData >> (48 + 160)); } - function _packValidationData(address authorizer, uint48 validAfter, uint48 validUntil) + function _packValidationRes(address authorizer, uint48 validAfter, uint48 validUntil) internal pure returns (uint256) diff --git a/test/utils/AccountTestBase.sol b/test/utils/AccountTestBase.sol index 3a752f07..a8f84a21 100644 --- a/test/utils/AccountTestBase.sol +++ b/test/utils/AccountTestBase.sol @@ -214,7 +214,7 @@ abstract contract AccountTestBase is OptimizedTest { for (uint256 i = 0; i < preValidationHookData.length; ++i) { sig = abi.encodePacked( sig, - _packValidationDataWithIndex( + _packValidationResWithIndex( preValidationHookData[i].index, preValidationHookData[i].validationData ) ); @@ -222,7 +222,7 @@ abstract contract AccountTestBase is OptimizedTest { // Index of the actual validation data is the length of the preValidationHooksRetrieved - aka // one-past-the-end - sig = abi.encodePacked(sig, _packValidationDataWithIndex(255, validationData)); + sig = abi.encodePacked(sig, _packValidationResWithIndex(255, validationData)); return sig; } @@ -238,7 +238,7 @@ abstract contract AccountTestBase is OptimizedTest { } // helper function to pack validation data with an index, according to the sparse calldata segment spec. - function _packValidationDataWithIndex(uint8 index, bytes memory validationData) + function _packValidationResWithIndex(uint8 index, bytes memory validationData) internal pure returns (bytes memory)