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

ACL: create optimized version of ACL that is only used to set up new organizations #546

Open
sohkai opened this issue Jul 19, 2019 · 2 comments

Comments

@sohkai
Copy link
Contributor

sohkai commented Jul 19, 2019

A lot of templates use acl.createPermission() and acl.grantPermission() many, many times.

The overhead of each call is quite high, requiring 10-30 duplicate auth, noPermissionManager, and onlyPermissionManager checks, as well as proxy call overheads. Each of these adds some 10k-50k gas in overhead.

Moreover, multiple times we assign "temporary" permissions to the factory to allow it to access functionality, each time costing upwards of 200-300k in gas.

Instead, we can provide a more efficient version of the ACL that allows you to set multiple permissions at the same time, while also simplifying the authentication checks, and swap out its implementation for the full version of the ACL at the end. For this more efficient ACL, I would consider any entity holding the "CREATE_PERMISSIONS_ROLE" as having full root access over the organization.

@izqui
Copy link
Contributor

izqui commented Jul 23, 2019

I really like this idea and I think we should definitely explore it.

From your proposal, I'm a bit hesitant about using the CREATE_PERMISSIONS_ROLE to denote whether an entity is root in the organization, as the CREATE_PERMISSIONS_ROLE is a fairly harmless role once an organization has been set up. Adding the explicit concept of an entity being 'root' would be more clear, although I understand that you don't want to have additional storage that needs to be cleaned up before transitioning to the regular ACL.

Another option could be to just some of these features to the regular ACL:

  • Explicit root entities: the ACL would allow setting entities as root, for which the canPerform() check will always return true. A root entity can set other entities as root and remove root permissions from any entity (all root entities have the same permissions). The deployer of the organization (normally the deployment template) would be set as root and can set other entities as root. Any entity with root permissions could remove other entities as root, including itself, which could be used to disable/burn the root feature.

    • Pros: after the initial deployment of the organization, it could be used to run 'installation scripts' for an app or set of apps. An entity with root permissions in a running organization could execute a CallsScript that sets the installation contract as root, and then calls the contract which removes itself as root after performing the actions. It would also make building modular templates easier.
    • Cons: not all organization structures have the notion of root (making it hard to standardize the above as the way to install apps). Although it could be argued that the permission manager for MANAGE_APPS_ROLE in the Kernel has indirect root authority over the organization already.
  • Batched setPermissions: a new function that could be used to set many permissions at the same time in the ACL. Its parameters would be an array of the same parameters as createPermission, and for each item decide whether to execute as grantPermission or createPermission depending on the value of manager (if set to a certain magic value, it would be interpreted as grantPermission). If instead of using auth in grantPermission we checked the permission in the ACL directly we would save a lot of gas.

An alternative to having explicit root entities could be to assume that an entity with the CREATE_PERMISSIONS_ROLE can perform actions if the permission hasn't been created yet. An entity with CREATE_PERMISSIONS_ROLE could create and grant themselves the permission and perform the action anyway, but this way we could remove some the setup and clean up for some temporary permissions. This interpretation of 'implicit permissions' could also be extended to permission managers as they can also grant themselves a permission.

@lkngtn
Copy link

lkngtn commented Sep 12, 2019

Explicit root entities: the ACL would allow setting entities as root, for which the canPerform() check will always return true. A root entity can set other entities as root and remove root permissions from any entity (all root entities have the same permissions). The deployer of the organization (normally the deployment template) would be set as root and can set other entities as root. Any entity with root permissions could remove other entities as root, including itself, which could be used to disable/burn the root feature.

I feel like this would be a useful pattern, and organizations could presumably opt-out of using the feature in some way similar to how you can burn permissions now.

Most organizations have some entity (or process) which can be considered the root authority anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants