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

Refactor PermissionRegistry #191

Merged
merged 13 commits into from
Jul 15, 2022

Conversation

AugustoL
Copy link
Member

@AugustoL AugustoL commented Jun 29, 2022

  • Wildcard for addresses and signatures removed.
  • Asset removed from permission mapping, not the permission mapping is used only for ETH calls. New format: from -> to -> functionSignature -> permission
  • Added ERC20 limits mapping, where each sender and an array of ERC20 amounts to be configured, and they are set by index, sender can edit existent elements in the array or add new ones. ERC20 limits can be added or removed.
  • The ERC20 limit removal process is a two step process, which means that first the removal needs to be called by the sender calling removeERC20Limit and after the permission delay time passes the function removeERC20Limit can be called publicly to effectively delete the permission, that space in the array can later be used by other ERC20 limit.
  • To do ERC20 transfers and approvals besides the ERC20 limits the tokens also need to have an ETH permission approved, this would be done for transfer or approve methods most likely.

So now everytime a proposal will execute the first thing that needs to be done is to call the permissionRegistry to save the balances of the tokens to be tracked, then the proposal executes only approved ETH functions and ETH transfers and once all executions are done the permissionRegistry is called to check the ERC20 values, if the difference between the initial value and the value after execution is higher than the allowed, the proposal would revert.

AugustoL added 3 commits June 29, 2022 10:34
Allow to remove permissions, owner can override permissionDelays, removed ERC20 and asset from
permissions and allow only ETH functions and values to be used.
…d default allowed permissions

Added functions to set the ERC20Limits by index in array owned by msg.sender, add function to set
the initial value in block and check value transfered againts this value before call ends, also
allowed calls to permission regsitry itself and internal calls by default but with no value
@AugustoL
Copy link
Member Author

I want to add that this simplified the code to be deployed and reduced the code size.

image

@AugustoL
Copy link
Member Author

fix #170 #178 #99

@AugustoL
Copy link
Member Author

AugustoL commented Jul 4, 2022

fix #160

@AugustoL AugustoL marked this pull request as ready for review July 13, 2022 22:12
Copy link
Collaborator

@rossneilson rossneilson left a comment

Choose a reason for hiding this comment

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

Amazing refactor!

@rossneilson rossneilson requested a review from a team July 15, 2022 10:34
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.

3 participants