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

fix: ModularAccountBase cleanup, coverage, custom error types #257

Merged
merged 1 commit into from
Oct 16, 2024
Merged

fix: ModularAccountBase cleanup, coverage, custom error types #257

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 .forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Counter.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
79509
79479
2 changes: 1 addition & 1 deletion .forge-snapshots/ModularAccount_Runtime_UseSessionKey_Case1_Token.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
113067
113037
2 changes: 1 addition & 1 deletion .forge-snapshots/ModularAccount_UserOp_BatchTransfers.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
198425
198437
2 changes: 1 addition & 1 deletion .forge-snapshots/ModularAccount_UserOp_Erc20Transfer.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
185394
185406
2 changes: 1 addition & 1 deletion .forge-snapshots/ModularAccount_UserOp_InstallSessionKey_Case1.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
532140
532152
2 changes: 1 addition & 1 deletion .forge-snapshots/ModularAccount_UserOp_NativeTransfer.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
161675
161687
2 changes: 1 addition & 1 deletion .forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Counter.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
195886
195938
2 changes: 1 addition & 1 deletion .forge-snapshots/ModularAccount_UserOp_UseSessionKey_Case1_Token.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
227010
227062
2 changes: 1 addition & 1 deletion .forge-snapshots/ModularAccount_UserOp_deferredValidation.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
261242
261264
2 changes: 1 addition & 1 deletion .forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Counter.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
79925
79895
2 changes: 1 addition & 1 deletion .forge-snapshots/SemiModularAccount_Runtime_UseSessionKey_Case1_Token.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
113483
113453
2 changes: 1 addition & 1 deletion .forge-snapshots/SemiModularAccount_UserOp_BatchTransfers.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
193634
193639
2 changes: 1 addition & 1 deletion .forge-snapshots/SemiModularAccount_UserOp_Erc20Transfer.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
180730
180735
2 changes: 1 addition & 1 deletion .forge-snapshots/SemiModularAccount_UserOp_InstallSessionKey_Case1.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
529720
529725
2 changes: 1 addition & 1 deletion .forge-snapshots/SemiModularAccount_UserOp_NativeTransfer.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
157005
157010
2 changes: 1 addition & 1 deletion .forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Counter.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
196127
196232
2 changes: 1 addition & 1 deletion .forge-snapshots/SemiModularAccount_UserOp_UseSessionKey_Case1_Token.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
227263
227368
2 changes: 1 addition & 1 deletion .forge-snapshots/SemiModularAccount_UserOp_deferredValidation.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
257938
257961
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"lint:gas": "solhint --max-warnings 0 -c ./config/solhint-gas.json './gas/**/*.sol'",
"lint:script": "solhint --max-warnings 0 -c ./config/solhint-script.json './script/**/*.sol'",
"prep": "pnpm fmt && forge b --deny-warnings && pnpm lint && pnpm test && pnpm gas",
"sizes": "FOUNDRY_PROFILE=optimized-build forge b --sizes | grep '^|' | grep -v -e '| 17 |' -e 'Lib'",
"test": "forge test"
}
}
33 changes: 13 additions & 20 deletions src/account/ModularAccountBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,21 +91,16 @@ abstract contract ModularAccountBase is

event DeferredActionNonceInvalidated(uint256 nonce);

error PostExecHookReverted(address module, uint32 entityId, bytes revertReason);
error PreExecHookReverted(address module, uint32 entityId, bytes revertReason);
error PreRuntimeValidationHookFailed(address module, uint32 entityId, bytes revertReason);
error CreateFailed();
error DeferredActionNonceInvalid();
error DeferredActionSignatureInvalid();
error RequireUserOperationContext();
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 SignatureValidationInvalid(ModuleEntity validationFunction);
error UserOpValidationInvalid(ModuleEntity validationFunction);
error UnexpectedAggregator(ModuleEntity validationFunction, address aggregator);
Comment on lines +99 to +101
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are theseModuleEntity while the execution lib errors below are unpacked?

Also nit: Mind sorting this entire block of errors? (see CreateFailed at the bottom)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Last I tested, this was both cheaper and smaller in codesize, but let me re-test to be sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok it is actually ever so slightly less codesize - updated!

error UnrecognizedFunction(bytes4 selector);
error ValidationFunctionMissing(bytes4 selector);
error DeferredActionNonceInvalid();
error DeferredActionSignatureInvalid();
// This error is trigged in performCreate and performCreate2
error CreateFailed();

// Wraps execution of a native function with runtime validation and hooks
// Used for upgradeTo, upgradeToAndCall, execute, executeBatch, installExecution, uninstallExecution,
Expand Down Expand Up @@ -577,14 +572,14 @@ abstract contract ModularAccountBase is
(currentSignatureSlice, signature) =
signature.advanceSegmentIfAtIndex(uint8(preUserOpValidationHooks.length - i - 1));

uint256 currentValidationRes = ExecutionLib.invokeUserOpCallBuffer(
userOpCallBuffer, preUserOpValidationHooks[i].moduleEntity(), currentSignatureSlice
);
ModuleEntity uoValidationHook = preUserOpValidationHooks[i].moduleEntity();

uint256 currentValidationRes =
ExecutionLib.invokeUserOpCallBuffer(userOpCallBuffer, uoValidationHook, currentSignatureSlice);

if (uint160(currentValidationRes) > 1) {
// If the aggregator is not 0 or 1, it is an unexpected value
(address module, uint32 entityId) = preUserOpValidationHooks[i].moduleEntity().unpack();
revert UnexpectedAggregator(module, entityId, address(uint160(currentValidationRes)));
revert UnexpectedAggregator(uoValidationHook, address(uint160(currentValidationRes)));
}
validationRes = _coalescePreValidation(validationRes, currentValidationRes);
}
Expand Down Expand Up @@ -730,8 +725,7 @@ abstract contract ModularAccountBase is
AccountStorage storage _storage = getAccountStorage();

if (!_storage.validationStorage[userOpValidationFunction].isUserOpValidation) {
(address module, uint32 entityId) = userOpValidationFunction.unpack();
revert UserOpValidationInvalid(module, entityId);
revert UserOpValidationInvalid(userOpValidationFunction);
}

ExecutionLib.convertToValidationBuffer(callBuffer);
Expand Down Expand Up @@ -789,8 +783,7 @@ abstract contract ModularAccountBase is
AccountStorage storage _storage = getAccountStorage();

if (!_storage.validationStorage[sigValidation].isSignatureValidation) {
(address module, uint32 entityId) = sigValidation.unpack();
revert SignatureValidationInvalid(module, entityId);
revert SignatureValidationInvalid(sigValidation);
}

if (ExecutionLib.invokeSignatureValidation(buffer, sigValidation, signatureSegment) == _1271_MAGIC_VALUE) {
Expand Down
Loading
Loading