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

[3/n permissions] feat: add execute user operation #77

Merged
merged 14 commits into from
Jul 9, 2024

Conversation

howydev
Copy link
Collaborator

@howydev howydev commented Jun 19, 2024

changes:

  1. Add executeUserOp
  2. If hooks require UO context, enforce that calls are made to executeUserOp in UO validation
  3. Send abi.encode(PackedUserOperation) if UO context is required, else send abi.encodePacked(msg.sender, msg.value, msg.data)
  4. Add permission hook installation/uninstallation via installValidation and uninstallValidation

@howydev howydev force-pushed the howy/add-permissions branch from 8c35ead to c4fa5d4 Compare June 24, 2024 21:29
@howydev howydev force-pushed the howy/add-permissions branch from c4fa5d4 to 990a842 Compare June 24, 2024 21:31
@howydev howydev requested a review from a team June 24, 2024 21:35
@@ -51,6 +54,26 @@ abstract contract PluginManager2 {
}
}

if (permissionHooks.length > 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The name is growing on me. : )

@0xrubes
Copy link

0xrubes commented Jun 26, 2024

In case we are doing the UO flow without UO context, aren't we skipping all permission hooks currently? Permission hooks currently get only invoked in executeWithAuthorization() (so runtime context) and executeUserOp() (UO operation with UO context for execution), but not in UO operation without UO context for execution.

The information on this would however only be available in UO validation and not in execution. Should we forward this info somehow? It seems tricky to do permission hooks in this case.

@howydev howydev force-pushed the howy/add-permissions branch from 8b7aa5a to ce2330e Compare June 26, 2024 21:07
src/account/PluginManager2.sol Outdated Show resolved Hide resolved
src/interfaces/IPermissionHook.sol Outdated Show resolved Hide resolved
src/interfaces/IPermissionHook.sol Outdated Show resolved Hide resolved
Copy link
Contributor

@adamegyed adamegyed left a comment

Choose a reason for hiding this comment

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

Looks good, just have 1 clarifying question.

src/account/UpgradeableModularAccount.sol Show resolved Hide resolved
Base automatically changed from howy/add-permissions to v0.8-develop July 9, 2024 00:55
@howydev howydev merged commit f63137b into v0.8-develop Jul 9, 2024
3 checks passed
@howydev howydev deleted the howy/add-execUO branch July 9, 2024 18:55
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.

5 participants