Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add flag for user op validation and pack efficiently #160

Merged
merged 1 commit into from
Aug 29, 2024
Merged

feat: add flag for user op validation and pack efficiently #160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/account/AccountFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ contract AccountFactory is Ownable {
new ERC1967Proxy{salt: combinedSalt}(address(ACCOUNT_IMPL), "");
// point proxy to actual implementation and init plugins
ReferenceModularAccount(payable(addr)).initializeWithValidation(
ValidationConfigLib.pack(SINGLE_SIGNER_VALIDATION_MODULE, entityId, true, true),
ValidationConfigLib.pack(SINGLE_SIGNER_VALIDATION_MODULE, entityId, true, true, true),
new bytes4[](0),
pluginInstallData,
new bytes[](0)
Expand Down
4 changes: 3 additions & 1 deletion src/account/AccountStorage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ struct ExecutionData {
struct ValidationData {
// Whether or not this validation can be used as a global validation function.
bool isGlobal;
// Whether or not this validation is a signature validator.
// Whether or not this validation is allowed to validate ERC-1271 signatures.
bool isSignatureValidation;
// Whether or not this validation is allowed to validate ERC-4337 user operations.
bool isUserOpValidation;
// The pre validation hooks for this validation function.
ModuleEntity[] preValidationHooks;
// Permission hooks for this validation function.
Expand Down
1 change: 1 addition & 0 deletions src/account/ModularAccountView.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ abstract contract ModularAccountView is IModularAccountView {
ValidationData storage validationData = getAccountStorage().validationData[validationFunction];
data.isGlobal = validationData.isGlobal;
data.isSignatureValidation = validationData.isSignatureValidation;
data.isUserOpValidation = validationData.isUserOpValidation;
data.preValidationHooks = validationData.preValidationHooks;

uint256 permissionHooksLen = validationData.permissionHooks.length();
Expand Down
1 change: 1 addition & 0 deletions src/account/ModuleManagerInternals.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ abstract contract ModuleManagerInternals is IModularAccount {

_validationData.isGlobal = validationConfig.isGlobal();
_validationData.isSignatureValidation = validationConfig.isSignatureValidation();
_validationData.isUserOpValidation = validationConfig.isUserOpValidation();

_onInstall(validationConfig.module(), installData, type(IValidationModule).interfaceId);
emit ValidationInstalled(validationConfig.module(), validationConfig.entityId());
Expand Down
7 changes: 7 additions & 0 deletions src/account/ReferenceModularAccount.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ contract ReferenceModularAccount is
error RuntimeValidationFunctionReverted(address module, uint32 entityId, bytes revertReason);
error SelfCallRecursionDepthExceeded();
error SignatureValidationInvalid(address module, uint32 entityId);
error UserOpValidationInvalid(address module, uint32 entityId);
error UnexpectedAggregator(address module, uint32 entityId, address aggregator);
error UnrecognizedFunction(bytes4 selector);
error ValidationFunctionMissing(bytes4 selector);
Expand Down Expand Up @@ -593,8 +594,14 @@ contract ReferenceModularAccount is
PackedUserOperation memory userOp,
bytes32 userOpHash
) internal virtual returns (uint256) {
AccountStorage storage _storage = getAccountStorage();

(address module, uint32 entityId) = userOpValidationFunction.unpack();

if (!_storage.validationData[userOpValidationFunction].isUserOpValidation) {
revert UserOpValidationInvalid(module, entityId);
}

return IValidationModule(module).validateUserOp(entityId, userOp, userOpHash);
}

Expand Down
99 changes: 63 additions & 36 deletions src/helpers/ValidationConfigLib.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,67 +7,78 @@ import {ModuleEntity, ValidationConfig} from "../interfaces/IModularAccount.sol"
// Layout:
// 0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA________________________ // Address
// 0x________________________________________BBBBBBBB________________ // Entity ID
// 0x________________________________________________CC______________ // isGlobal
// 0x__________________________________________________DD____________ // isSignatureValidation
// 0x____________________________________________________000000000000 // unused
// 0x________________________________________________CC______________ // validation flags
// 0x__________________________________________________00000000000000 // unused

// Validation flags layout:
// 0b00000___ // unused
// 0b_____A__ // isGlobal
// 0b______B_ // isSignatureValidation
// 0b_______C // isUserOpValidation

library ValidationConfigLib {
function pack(ModuleEntity _validationFunction, bool _isGlobal, bool _isSignatureValidation)
internal
pure
returns (ValidationConfig)
{
// is user op validation flag stored in last bit of the 25th byte
bytes32 internal constant _VALIDATION_FLAG_IS_USER_OP = bytes32(uint256(1) << 56);
// is signature validation flag stored in second to last bit of the 25th byte
bytes32 internal constant _VALIDATION_FLAG_IS_SIGNATURE = bytes32(uint256(1) << 57);
// is global flag stored in the third to last bit of the 25th byte
bytes32 internal constant _VALIDATION_FLAG_IS_GLOBAL = bytes32(uint256(1) << 58);

function pack(
ModuleEntity _validationFunction,
bool _isGlobal,
bool _isSignatureValidation,
bool _isUserOpValidation
) internal pure returns (ValidationConfig) {
return ValidationConfig.wrap(
bytes26(
bytes26(ModuleEntity.unwrap(_validationFunction))
// isGlobal flag stored in the 25th byte
| bytes26(bytes32(_isGlobal ? uint256(1) << 56 : 0))
// isSignatureValidation flag stored in the 26th byte
| bytes26(bytes32(_isSignatureValidation ? uint256(1) << 48 : 0))
bytes25(
bytes25(ModuleEntity.unwrap(_validationFunction))
| bytes25(bytes32(_isGlobal ? _VALIDATION_FLAG_IS_GLOBAL : bytes32(0)))
| bytes25(bytes32(_isSignatureValidation ? _VALIDATION_FLAG_IS_SIGNATURE : bytes32(0)))
| bytes25(bytes32(_isUserOpValidation ? _VALIDATION_FLAG_IS_USER_OP : bytes32(0)))
)
);
}

function pack(address _module, uint32 _entityId, bool _isGlobal, bool _isSignatureValidation)
internal
pure
returns (ValidationConfig)
{
function pack(
address _module,
uint32 _entityId,
bool _isGlobal,
bool _isSignatureValidation,
bool _isUserOpValidation
) internal pure returns (ValidationConfig) {
return ValidationConfig.wrap(
bytes26(
bytes25(
// module address stored in the first 20 bytes
bytes26(bytes20(_module))
bytes25(bytes20(_module))
// entityId stored in the 21st - 24th byte
| bytes26(bytes24(uint192(_entityId)))
// isGlobal flag stored in the 25th byte
| bytes26(bytes32(_isGlobal ? uint256(1) << 56 : 0))
// isSignatureValidation flag stored in the 26th byte
| bytes26(bytes32(_isSignatureValidation ? uint256(1) << 48 : 0))
| bytes25(bytes24(uint192(_entityId)))
| bytes25(bytes32(_isGlobal ? _VALIDATION_FLAG_IS_GLOBAL : bytes32(0)))
| bytes25(bytes32(_isSignatureValidation ? _VALIDATION_FLAG_IS_SIGNATURE : bytes32(0)))
| bytes25(bytes32(_isUserOpValidation ? _VALIDATION_FLAG_IS_USER_OP : bytes32(0)))
)
);
}

function unpackUnderlying(ValidationConfig config)
internal
pure
returns (address _module, uint32 _entityId, bool _isGlobal, bool _isSignatureValidation)
returns (address _module, uint32 _entityId, uint8 flags)
{
bytes26 configBytes = ValidationConfig.unwrap(config);
bytes25 configBytes = ValidationConfig.unwrap(config);
_module = address(bytes20(configBytes));
_entityId = uint32(bytes4(configBytes << 160));
_isGlobal = uint8(configBytes[24]) == 1;
_isSignatureValidation = uint8(configBytes[25]) == 1;
flags = uint8(configBytes[24]);
}

function unpack(ValidationConfig config)
internal
pure
returns (ModuleEntity _validationFunction, bool _isGlobal, bool _isSignatureValidation)
returns (ModuleEntity _validationFunction, uint8 flags)
{
bytes26 configBytes = ValidationConfig.unwrap(config);
bytes25 configBytes = ValidationConfig.unwrap(config);
_validationFunction = ModuleEntity.wrap(bytes24(configBytes));
_isGlobal = uint8(configBytes[24]) == 1;
_isSignatureValidation = uint8(configBytes[25]) == 1;
flags = uint8(configBytes[24]);
}

function module(ValidationConfig config) internal pure returns (address) {
Expand All @@ -83,10 +94,26 @@ library ValidationConfigLib {
}

function isGlobal(ValidationConfig config) internal pure returns (bool) {
return uint8(ValidationConfig.unwrap(config)[24]) == 1;
return ValidationConfig.unwrap(config) & _VALIDATION_FLAG_IS_GLOBAL != 0;
}

function isGlobal(uint8 flags) internal pure returns (bool) {
return flags & 0x04 != 0;
}

function isSignatureValidation(ValidationConfig config) internal pure returns (bool) {
return uint8(ValidationConfig.unwrap(config)[25]) == 1;
return ValidationConfig.unwrap(config) & _VALIDATION_FLAG_IS_SIGNATURE != 0;
}

function isSignatureValidation(uint8 flags) internal pure returns (bool) {
return flags & 0x02 != 0;
}

function isUserOpValidation(ValidationConfig config) internal pure returns (bool) {
return ValidationConfig.unwrap(config) & _VALIDATION_FLAG_IS_USER_OP != 0;
}

function isUserOpValidation(uint8 flags) internal pure returns (bool) {
return flags & 0x01 != 0;
}
}
19 changes: 18 additions & 1 deletion src/interfaces/IModularAccount.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,25 @@ pragma solidity ^0.8.25;
import {ExecutionManifest} from "./IExecutionModule.sol";

type ModuleEntity is bytes24;
// ModuleEntity is a packed representation of a module function
// Layout:
// 0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA________________________ // Address
// 0x________________________________________BBBBBBBB________________ // Entity ID
// 0x________________________________________________0000000000000000 // unused

type ValidationConfig is bytes26;
type ValidationConfig is bytes25;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: would be helpful to describe these UDVTs inline with NatSpec comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, will port over the comment blocks from the corresponding libraries

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another things I find that will be helpful is have a line comments on what the 25 bytes for in the IModularAccount. Each time I have to find the lib and figure out where each flag/data locate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated! I think this includes what you're describing @fangting-alchemy

// ValidationConfig is a packed representation of a validation function and flags for its configuration.
// Layout:
// 0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA________________________ // Address
// 0x________________________________________BBBBBBBB________________ // Entity ID
// 0x________________________________________________CC______________ // validation flags
// 0x__________________________________________________00000000000000 // unused
//
// Validation flags layout:
// 0b00000___ // unused
// 0b_____A__ // isGlobal
// 0b______B_ // isSignatureValidation
// 0b_______C // isUserOpValidation

type HookConfig is bytes26;

Expand Down
2 changes: 2 additions & 0 deletions src/interfaces/IModularAccountView.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ struct ValidationDataView {
bool isGlobal;
// Whether or not this validation is a signature validator.
bool isSignatureValidation;
// Whether or not this validation is a user operation validator.
bool isUserOpValidation;
// The pre validation hooks for this validation function.
ModuleEntity[] preValidationHooks;
// Permission hooks for this validation function.
Expand Down
4 changes: 3 additions & 1 deletion test/account/AccountReturnData.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ contract AccountReturnDataTest is AccountTestBase {
bytes4[] memory selectors = new bytes4[](1);
selectors[0] = IModularAccount.execute.selector;
account1.installValidation(
ValidationConfigLib.pack(address(resultConsumerModule), DIRECT_CALL_VALIDATION_ENTITYID, false, false),
ValidationConfigLib.pack(
address(resultConsumerModule), DIRECT_CALL_VALIDATION_ENTITYID, false, false, true
), // todo: does this need UO validation permission?
selectors,
"",
new bytes[](0)
Expand Down
6 changes: 3 additions & 3 deletions test/account/DirectCallsFromModule.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ contract DirectCallsFromModuleTest is AccountTestBase {
vm.prank(address(entryPoint));

account1.installValidation(
ValidationConfigLib.pack(extraOwner, DIRECT_CALL_VALIDATION_ENTITYID, false, false),
ValidationConfigLib.pack(extraOwner, DIRECT_CALL_VALIDATION_ENTITYID, false, false, false),
selectors,
"",
new bytes[](0)
Expand All @@ -154,7 +154,7 @@ contract DirectCallsFromModuleTest is AccountTestBase {

vm.prank(address(entryPoint));

ValidationConfig validationConfig = ValidationConfigLib.pack(_moduleEntity, false, false);
ValidationConfig validationConfig = ValidationConfigLib.pack(_moduleEntity, false, false, false);

account1.installValidation(validationConfig, selectors, "", hooks);
}
Expand All @@ -168,7 +168,7 @@ contract DirectCallsFromModuleTest is AccountTestBase {

vm.prank(address(entryPoint));

ValidationConfig validationConfig = ValidationConfigLib.pack(_moduleEntity, true, false);
ValidationConfig validationConfig = ValidationConfigLib.pack(_moduleEntity, true, false, false);

account1.installValidation(validationConfig, new bytes4[](0), "", hooks);
}
Expand Down
5 changes: 3 additions & 2 deletions test/account/ModularAccountView.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ contract ModularAccountViewTest is CustomValidationTestBase {

assertTrue(data.isGlobal);
assertTrue(data.isSignatureValidation);
assertTrue(data.isUserOpValidation);
assertEq(data.preValidationHooks.length, 2);
assertEq(
ModuleEntity.unwrap(data.preValidationHooks[0]),
Expand Down Expand Up @@ -131,7 +132,7 @@ contract ModularAccountViewTest is CustomValidationTestBase {
internal
virtual
override
returns (ModuleEntity, bool, bool, bytes4[] memory, bytes memory, bytes[] memory)
returns (ModuleEntity, bool, bool, bool, bytes4[] memory, bytes memory, bytes[] memory)
{
bytes[] memory hooks = new bytes[](2);
hooks[0] = abi.encodePacked(
Expand All @@ -148,6 +149,6 @@ contract ModularAccountViewTest is CustomValidationTestBase {
bytes4[] memory selectors = new bytes4[](1);
selectors[0] = comprehensiveModule.foo.selector;

return (comprehensiveModuleValidation, true, true, selectors, bytes(""), hooks);
return (comprehensiveModuleValidation, true, true, true, selectors, bytes(""), hooks);
}
}
2 changes: 1 addition & 1 deletion test/account/MultiValidation.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ contract MultiValidationTest is AccountTestBase {
function test_overlappingValidationInstall() public {
vm.prank(address(entryPoint));
account1.installValidation(
ValidationConfigLib.pack(address(validator2), TEST_DEFAULT_VALIDATION_ENTITY_ID, true, true),
ValidationConfigLib.pack(address(validator2), TEST_DEFAULT_VALIDATION_ENTITY_ID, true, true, true),
new bytes4[](0),
abi.encode(TEST_DEFAULT_VALIDATION_ENTITY_ID, owner2),
new bytes[](0)
Expand Down
8 changes: 5 additions & 3 deletions test/account/PerHookData.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ contract PerHookDataTest is CustomValidationTestBase {
);
vm.prank(address(entryPoint));
account1.installValidation(
ValidationConfigLib.pack(_signerValidation, true, false), new bytes4[](0), "", hooks
ValidationConfigLib.pack(_signerValidation, true, false, true), new bytes4[](0), "", hooks
);
}

Expand Down Expand Up @@ -523,7 +523,7 @@ contract PerHookDataTest is CustomValidationTestBase {
internal
virtual
override
returns (ModuleEntity, bool, bool, bytes4[] memory, bytes memory, bytes[] memory)
returns (ModuleEntity, bool, bool, bool, bytes4[] memory, bytes memory, bytes[] memory)
{
bytes[] memory hooks = new bytes[](1);
hooks[0] = abi.encodePacked(
Expand All @@ -532,6 +532,8 @@ contract PerHookDataTest is CustomValidationTestBase {
);
// patched to also work during SMA tests by differentiating the validation
_signerValidation = ModuleEntityLib.pack(address(singleSignerValidationModule), _VALIDATION_ENTITY_ID);
return (_signerValidation, true, true, new bytes4[](0), abi.encode(_VALIDATION_ENTITY_ID, owner1), hooks);
return (
_signerValidation, true, true, true, new bytes4[](0), abi.encode(_VALIDATION_ENTITY_ID, owner1), hooks
);
}
}
Loading
Loading