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

[2/n permissions] feat: add permission hooks #76

Merged
merged 7 commits into from
Jul 9, 2024

Conversation

howydev
Copy link
Collaborator

@howydev howydev commented Jun 19, 2024

Changelog in this PR:

  1. Add execution hooks associated with a validator
  2. Add getters in the loupe for these hooks
  3. Add string[] to PluginMetadata for permission plugins to declare ERC-7715 permission strings

@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/remove-permissions branch from 5d439e8 to 46dd1b2 Compare June 24, 2024 21:29
@howydev howydev force-pushed the howy/remove-permissions branch from 46dd1b2 to db9960f Compare June 24, 2024 21:31
@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
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.

This workflow looks correct, but the hooks themselves aren't called anywhere in this PR. So it's just setting up the storage fields for them, and the setter/getter functions.

Also it'd be nice to revisit the naming convention of "permission hooks", but I'm ok deferring that for now.

@howydev howydev force-pushed the howy/add-permissions branch from 8b7aa5a to ce2330e Compare June 26, 2024 21:07
Base automatically changed from howy/remove-permissions to v0.8-develop June 28, 2024 00:03
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.

LGTM

@howydev howydev merged commit f22a521 into v0.8-develop Jul 9, 2024
3 checks passed
@howydev howydev deleted the howy/add-permissions branch July 9, 2024 00: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.

4 participants