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.

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 signature validation hooks [2/2] #145

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

adamegyed
Copy link
Contributor

Motivation

We have had a stub pre validation hook type for 1271 signatures for a while, but never implemented it.

Solution

Define the interface and implement the logic for calling these hooks in the base account.

Add test cases for using the hooks, and sending per-hook data.

Also removes old todo comments about the interface functions.

@adamegyed adamegyed force-pushed the adam/merge-segment-logic branch from 13beea1 to 2168a96 Compare August 15, 2024 21:51
@adamegyed adamegyed force-pushed the adam/add-signature-validation-hooks branch from 450cf0c to 0e510c6 Compare August 15, 2024 21:52
@adamegyed adamegyed force-pushed the adam/add-signature-validation-hooks branch from 0e510c6 to 9469bc6 Compare August 19, 2024 21:50
src/account/UpgradeableModularAccount.sol Outdated Show resolved Hide resolved
if (!_storage.validationData[sigValidation].isSignatureValidation) {
revert SignatureValidationInvalid(module, entityId);
(address _module, uint32 _entityId) = sigValidation.unpack();
Copy link
Collaborator

Choose a reason for hiding this comment

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

At some point would love to revisit all of our variable naming according to our style guide.

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, we should pick a standardized way to name vars when avoiding shadowing. RIght now, this pattern is ambiguous with internal-visibility variables and constants.

@adamegyed adamegyed force-pushed the adam/merge-segment-logic branch from 2168a96 to d042a7d Compare August 20, 2024 22:11
@adamegyed adamegyed force-pushed the adam/add-signature-validation-hooks branch from 9469bc6 to 19118e0 Compare August 20, 2024 22:17
Base automatically changed from adam/merge-segment-logic to develop August 20, 2024 22:22
@adamegyed adamegyed force-pushed the adam/add-signature-validation-hooks branch from 19118e0 to 496af4e Compare August 20, 2024 22:23
@adamegyed adamegyed merged commit 236a601 into develop Aug 20, 2024
3 checks passed
@adamegyed adamegyed deleted the adam/add-signature-validation-hooks branch August 20, 2024 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants