diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index ce9a4a9c..f96642c1 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -65,7 +65,6 @@ contract UpgradeableModularAccount is } IEntryPoint private immutable _ENTRY_POINT; - uint256 internal constant _SIG_VALIDATION_FAILED = 1; // As per the EIP-165 spec, no interface should ever match 0xffffffff bytes4 internal constant _INTERFACE_ID_INVALID = 0xffffffff; diff --git a/src/helpers/ValidationDataHelpers.sol b/src/helpers/ValidationDataHelpers.sol index e3c9bd2d..ba9a3630 100644 --- a/src/helpers/ValidationDataHelpers.sol +++ b/src/helpers/ValidationDataHelpers.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.22; +import {SIG_VALIDATION_FAILED} from "../libraries/Constants.sol"; + /// @dev This helper function assumes that uint160(validationData1) and uint160(validationData2) can only be 0 or 1 // solhint-disable-next-line private-vars-leading-underscore function _coalescePreValidation(uint256 validationData1, uint256 validationData2) @@ -47,5 +49,6 @@ function _coalesceValidation(uint256 preValidationData, uint256 validationData) 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) == SIG_VALIDATION_FAILED ? SIG_VALIDATION_FAILED : uint160(validationData); } diff --git a/src/libraries/AssociatedLinkedListSetLib.sol b/src/libraries/AssociatedLinkedListSetLib.sol index b0532b2b..a2e7ee6a 100644 --- a/src/libraries/AssociatedLinkedListSetLib.sol +++ b/src/libraries/AssociatedLinkedListSetLib.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.22; -import {SetValue, SENTINEL_VALUE, HAS_NEXT_FLAG} from "./LinkedListSetUtils.sol"; +import {SetValue, SENTINEL_VALUE, HAS_NEXT_FLAG} from "./Constants.sol"; /// @dev Type representing the set, which is just a storage slot placeholder like the solidity mapping type. struct AssociatedLinkedListSet { diff --git a/src/libraries/CastLib.sol b/src/libraries/CastLib.sol index d942cbaf..31f01c73 100644 --- a/src/libraries/CastLib.sol +++ b/src/libraries/CastLib.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.22; import {FunctionReference} from "./FunctionReferenceLib.sol"; -import {SetValue} from "./LinkedListSetUtils.sol"; +import {SetValue} from "./Constants.sol"; /// @title Cast Library /// @author Alchemy diff --git a/src/libraries/LinkedListSetUtils.sol b/src/libraries/Constants.sol similarity index 78% rename from src/libraries/LinkedListSetUtils.sol rename to src/libraries/Constants.sol index 7560ec0d..59159d4a 100644 --- a/src/libraries/LinkedListSetUtils.sol +++ b/src/libraries/Constants.sol @@ -9,3 +9,7 @@ bytes32 constant SENTINEL_VALUE = bytes32(uint256(1)); /// @dev Removing the last element will result in this flag not being set correctly, but all operations will /// function normally, albeit with one extra sload for getAll. bytes32 constant HAS_NEXT_FLAG = bytes32(uint256(2)); + +/// @dev As defined by ERC-4337. +uint256 constant SIG_VALIDATION_PASSED = 0; +uint256 constant SIG_VALIDATION_FAILED = 1; diff --git a/src/libraries/CountableLinkedListSetLib.sol b/src/libraries/CountableLinkedListSetLib.sol index 43d70fc5..b04c7ff0 100644 --- a/src/libraries/CountableLinkedListSetLib.sol +++ b/src/libraries/CountableLinkedListSetLib.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.22; import {LinkedListSet, LinkedListSetLib} from "./LinkedListSetLib.sol"; -import {SetValue} from "./LinkedListSetUtils.sol"; +import {SetValue} from "./Constants.sol"; /// @title Countable Linked List Set Library /// @author Alchemy diff --git a/src/libraries/LinkedListSetLib.sol b/src/libraries/LinkedListSetLib.sol index 478c6b4d..5a43190b 100644 --- a/src/libraries/LinkedListSetLib.sol +++ b/src/libraries/LinkedListSetLib.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.22; -import {SetValue, SENTINEL_VALUE, HAS_NEXT_FLAG} from "./LinkedListSetUtils.sol"; +import {SetValue, SENTINEL_VALUE, HAS_NEXT_FLAG} from "./Constants.sol"; struct LinkedListSet { // Byte Layout diff --git a/src/plugins/owner/MultiOwnerPlugin.sol b/src/plugins/owner/MultiOwnerPlugin.sol index b332a924..aa88da2f 100644 --- a/src/plugins/owner/MultiOwnerPlugin.sol +++ b/src/plugins/owner/MultiOwnerPlugin.sol @@ -24,7 +24,7 @@ import { AssociatedLinkedListSet, AssociatedLinkedListSetLib } from "../../libraries/AssociatedLinkedListSetLib.sol"; import {CastLib} from "../../libraries/CastLib.sol"; -import {SetValue} from "../../libraries/LinkedListSetUtils.sol"; +import {SetValue, SIG_VALIDATION_PASSED, SIG_VALIDATION_FAILED} from "../../libraries/Constants.sol"; /// @title Multi Owner Plugin /// @author Alchemy @@ -59,11 +59,6 @@ contract MultiOwnerPlugin is BasePlugin, IMultiOwnerPlugin, IERC1271 { bytes32 private constant _HASHED_VERSION = keccak256(bytes(_VERSION)); bytes32 private immutable _SALT = bytes32(bytes20(address(this))); - // ERC-4337 specific value: signature validation passed - uint256 internal constant _SIG_VALIDATION_PASSED = 0; - // ERC-4337 specific value: signature validation failed - uint256 internal constant _SIG_VALIDATION_FAILED = 1; - // bytes4(keccak256("isValidSignature(bytes32,bytes)")) bytes4 internal constant _1271_MAGIC_VALUE = 0x1626ba7e; bytes4 internal constant _1271_MAGIC_VALUE_FAILURE = 0xffffffff; @@ -183,14 +178,14 @@ contract MultiOwnerPlugin is BasePlugin, IMultiOwnerPlugin, IERC1271 { (address signer, ECDSA.RecoverError error) = userOpHash.toEthSignedMessageHash().tryRecover(userOp.signature); if (error == ECDSA.RecoverError.NoError && isOwnerOf(msg.sender, signer)) { - return _SIG_VALIDATION_PASSED; + return SIG_VALIDATION_PASSED; } if (_isValidERC1271OwnerTypeSignature(msg.sender, userOpHash, userOp.signature)) { - return _SIG_VALIDATION_PASSED; + return SIG_VALIDATION_PASSED; } - return _SIG_VALIDATION_FAILED; + return SIG_VALIDATION_FAILED; } revert NotImplemented(msg.sig, functionId); diff --git a/src/plugins/session/SessionKeyPlugin.sol b/src/plugins/session/SessionKeyPlugin.sol index 569cadfd..db703e3b 100644 --- a/src/plugins/session/SessionKeyPlugin.sol +++ b/src/plugins/session/SessionKeyPlugin.sol @@ -24,7 +24,9 @@ import { AssociatedLinkedListSet, AssociatedLinkedListSetLib } from "../../libraries/AssociatedLinkedListSetLib.sol"; import {CastLib} from "../../libraries/CastLib.sol"; -import {SetValue, SENTINEL_VALUE} from "../../libraries/LinkedListSetUtils.sol"; +import { + SetValue, SENTINEL_VALUE, SIG_VALIDATION_PASSED, SIG_VALIDATION_FAILED +} from "../../libraries/Constants.sol"; /// @title Session Key Plugin /// @author Alchemy @@ -42,9 +44,6 @@ contract SessionKeyPlugin is ISessionKeyPlugin, SessionKeyPermissions, BasePlugi string internal constant _VERSION = "1.0.0"; string internal constant _AUTHOR = "Alchemy"; - uint256 internal constant _SIG_VALIDATION_PASSED = 0; - uint256 internal constant _SIG_VALIDATION_FAILED = 1; - // Constants used in the manifest uint256 internal constant _MANIFEST_DEPENDENCY_INDEX_OWNER_USER_OP_VALIDATION = 0; uint256 internal constant _MANIFEST_DEPENDENCY_INDEX_OWNER_RUNTIME_VALIDATION = 1; @@ -177,7 +176,7 @@ contract SessionKeyPlugin is ISessionKeyPlugin, SessionKeyPermissions, BasePlugi ) { return _checkUserOpPermissions(userOp, calls, sessionKey); } - return _SIG_VALIDATION_FAILED; + return SIG_VALIDATION_FAILED; } revert InvalidSignature(sessionKey); } diff --git a/src/plugins/session/permissions/SessionKeyPermissions.sol b/src/plugins/session/permissions/SessionKeyPermissions.sol index 689400c0..bbce8120 100644 --- a/src/plugins/session/permissions/SessionKeyPermissions.sol +++ b/src/plugins/session/permissions/SessionKeyPermissions.sol @@ -10,6 +10,7 @@ import {SessionKeyPermissionsLoupe} from "./SessionKeyPermissionsLoupe.sol"; import {IStandardExecutor} from "../../../interfaces/IStandardExecutor.sol"; import {UserOperation} from "../../../interfaces/erc4337/UserOperation.sol"; +import {SIG_VALIDATION_PASSED, SIG_VALIDATION_FAILED} from "../../../libraries/Constants.sol"; /// @title Session Key Permissions /// @author Alchemy @@ -128,8 +129,8 @@ abstract contract SessionKeyPermissions is ISessionKeyPlugin, SessionKeyPermissi // otherwise a packed struct of the aggregator address (0 here), and two // 6-byte timestamps indicating the start and end times at which the op // is valid. - return uint160(validationSuccess ? 0 : 1) | (uint256(validUntil) << 160) - | (uint256(currentValidAfter) << (208)); + return uint160(validationSuccess ? SIG_VALIDATION_PASSED : SIG_VALIDATION_FAILED) + | (uint256(validUntil) << 160) | (uint256(currentValidAfter) << (208)); } /// @dev Checks permissions on a per-call basis. Should be run during user op validation once per `Call` struct diff --git a/test/account/ValidationIntersection.t.sol b/test/account/ValidationIntersection.t.sol index 3e1f192b..ced8e029 100644 --- a/test/account/ValidationIntersection.t.sol +++ b/test/account/ValidationIntersection.t.sol @@ -9,6 +9,7 @@ import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAcc import {IEntryPoint} from "../../src/interfaces/erc4337/IEntryPoint.sol"; import {UserOperation} from "../../src/interfaces/erc4337/UserOperation.sol"; import {FunctionReference} from "../../src/interfaces/IPluginManager.sol"; +import {SIG_VALIDATION_FAILED, SIG_VALIDATION_PASSED} from "../../src/libraries/Constants.sol"; import {MultiOwnerPlugin} from "../../src/plugins/owner/MultiOwnerPlugin.sol"; import {MultiOwnerMSCAFactory} from "../../src/factory/MultiOwnerMSCAFactory.sol"; @@ -20,8 +21,6 @@ import { } from "../mocks/plugins/ValidationPluginMocks.sol"; contract ValidationIntersectionTest is Test { - uint256 internal constant _SIG_VALIDATION_FAILED = 1; - IEntryPoint public entryPoint; address public owner1; @@ -92,8 +91,8 @@ contract ValidationIntersectionTest is Test { function test_validationIntersect_authorizer_sigfail_validationFunction() public { oneHookPlugin.setValidationData( - _SIG_VALIDATION_FAILED, - 0 // returns OK + SIG_VALIDATION_FAILED, + SIG_VALIDATION_PASSED // returns OK ); UserOperation memory userOp; @@ -104,13 +103,13 @@ contract ValidationIntersectionTest is Test { uint256 returnedValidationData = account1.validateUserOp(userOp, uoHash, 1 wei); // Down-cast to only check the authorizer - assertEq(uint160(returnedValidationData), _SIG_VALIDATION_FAILED); + assertEq(uint160(returnedValidationData), SIG_VALIDATION_FAILED); } function test_validationIntersect_authorizer_sigfail_hook() public { oneHookPlugin.setValidationData( - 0, // returns OK - _SIG_VALIDATION_FAILED + SIG_VALIDATION_PASSED, // returns OK + SIG_VALIDATION_FAILED ); UserOperation memory userOp; @@ -121,7 +120,7 @@ contract ValidationIntersectionTest is Test { uint256 returnedValidationData = account1.validateUserOp(userOp, uoHash, 1 wei); // Down-cast to only check the authorizer - assertEq(uint160(returnedValidationData), _SIG_VALIDATION_FAILED); + assertEq(uint160(returnedValidationData), SIG_VALIDATION_FAILED); } function test_validationIntersect_timeBounds_intersect_1() public { @@ -170,7 +169,7 @@ contract ValidationIntersectionTest is Test { address badAuthorizer = makeAddr("badAuthorizer"); oneHookPlugin.setValidationData( - 0, // returns OK + SIG_VALIDATION_PASSED, // returns OK uint256(uint160(badAuthorizer)) // returns an aggregator, which preValidation hooks are not allowed to // do. ); @@ -196,7 +195,7 @@ contract ValidationIntersectionTest is Test { oneHookPlugin.setValidationData( uint256(uint160(goodAuthorizer)), // returns a valid aggregator - 0 // returns OK + SIG_VALIDATION_PASSED // returns OK ); UserOperation memory userOp; @@ -240,7 +239,7 @@ contract ValidationIntersectionTest is Test { uint48 end2 = uint48(25); twoHookPlugin.setValidationData( - 0, // returns OK + SIG_VALIDATION_PASSED, // returns OK _packValidationData(address(0), start1, end1), _packValidationData(address(0), start2, end2) ); @@ -257,9 +256,9 @@ contract ValidationIntersectionTest is Test { function test_validationIntersect_multiplePreValidationHooksSigFail() public { twoHookPlugin.setValidationData( - 0, // returns OK - 0, // returns OK - _SIG_VALIDATION_FAILED + SIG_VALIDATION_PASSED, // returns OK + SIG_VALIDATION_PASSED, // returns OK + SIG_VALIDATION_FAILED ); UserOperation memory userOp; @@ -271,7 +270,7 @@ contract ValidationIntersectionTest is Test { uint256 returnedValidationData = account1.validateUserOp(userOp, uoHash, 1 wei); // Down-cast to only check the authorizer - assertEq(uint160(returnedValidationData), _SIG_VALIDATION_FAILED); + assertEq(uint160(returnedValidationData), SIG_VALIDATION_FAILED); } function _unpackValidationData(uint256 validationData) diff --git a/test/invariant/handlers/AssociatedLinkedListSetHandler.sol b/test/invariant/handlers/AssociatedLinkedListSetHandler.sol index 525c6f0c..d0c387d3 100644 --- a/test/invariant/handlers/AssociatedLinkedListSetHandler.sol +++ b/test/invariant/handlers/AssociatedLinkedListSetHandler.sol @@ -11,7 +11,7 @@ import { AssociatedLinkedListSet, AssociatedLinkedListSetLib } from "../../../src/libraries/AssociatedLinkedListSetLib.sol"; -import {SetValue} from "../../../src/libraries/LinkedListSetUtils.sol"; +import {SetValue} from "../../../src/libraries/Constants.sol"; /// @notice A handler contract for differential invariant testing AssociatedLinkedListSetLib /// This contract maps logic for adding, removeing, clearing, and inspecting a list diff --git a/test/invariant/handlers/LinkedListSetHandler.sol b/test/invariant/handlers/LinkedListSetHandler.sol index 77ccba2f..1468ce5f 100644 --- a/test/invariant/handlers/LinkedListSetHandler.sol +++ b/test/invariant/handlers/LinkedListSetHandler.sol @@ -8,7 +8,7 @@ import {StdUtils} from "forge-std/StdUtils.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; import {LinkedListSetLib, LinkedListSet as EnumerableSetType} from "../../../src/libraries/LinkedListSetLib.sol"; -import {SetValue} from "../../../src/libraries/LinkedListSetUtils.sol"; +import {SetValue} from "../../../src/libraries/Constants.sol"; /// @notice A handler contract for differential invariant testing LinkedListSetLib /// This contract maps logic for adding, removeing, clearing, and inspecting a list diff --git a/test/libraries/AssociatedLinkedListSetLib.t.sol b/test/libraries/AssociatedLinkedListSetLib.t.sol index 74b62f34..748081f4 100644 --- a/test/libraries/AssociatedLinkedListSetLib.t.sol +++ b/test/libraries/AssociatedLinkedListSetLib.t.sol @@ -7,7 +7,7 @@ import { AssociatedLinkedListSet, AssociatedLinkedListSetLib } from "../../src/libraries/AssociatedLinkedListSetLib.sol"; -import {SetValue, SENTINEL_VALUE} from "../../src/libraries/LinkedListSetUtils.sol"; +import {SetValue, SENTINEL_VALUE} from "../../src/libraries/Constants.sol"; contract AssociatedLinkedListSetLibTest is Test { using AssociatedLinkedListSetLib for AssociatedLinkedListSet; diff --git a/test/libraries/CountableLinkedListSetLib.t.sol b/test/libraries/CountableLinkedListSetLib.t.sol index c09deadb..b971de1a 100644 --- a/test/libraries/CountableLinkedListSetLib.t.sol +++ b/test/libraries/CountableLinkedListSetLib.t.sol @@ -5,7 +5,7 @@ import {Test} from "forge-std/Test.sol"; import {CountableLinkedListSetLib} from "../../src/libraries/CountableLinkedListSetLib.sol"; import {LinkedListSet, LinkedListSetLib} from "../../src/libraries/LinkedListSetLib.sol"; -import {SetValue} from "../../src/libraries/LinkedListSetUtils.sol"; +import {SetValue} from "../../src/libraries/Constants.sol"; contract CountableLinkedListSetLibTest is Test { using LinkedListSetLib for LinkedListSet; diff --git a/test/libraries/LinkedListSetLib.t.sol b/test/libraries/LinkedListSetLib.t.sol index 806e28ef..4373cc52 100644 --- a/test/libraries/LinkedListSetLib.t.sol +++ b/test/libraries/LinkedListSetLib.t.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.22; import {Test} from "forge-std/Test.sol"; import {LinkedListSet, LinkedListSetLib} from "../../src/libraries/LinkedListSetLib.sol"; -import {SetValue, SENTINEL_VALUE} from "../../src/libraries/LinkedListSetUtils.sol"; +import {SetValue, SENTINEL_VALUE} from "../../src/libraries/Constants.sol"; // Ported over from test/AssociatedLinkedListSetLib.t.sol, dropping test_no_address_collision contract LinkedListSetLibTest is Test {