aragon / buidler-aragon

(Aragon 1) Buidler plugin for Aragon App development
24 stars 22 forks source link

Script Hooks: assign custom permission to the proxy app #143

Open 0xGabi opened 4 years ago

0xGabi commented 4 years ago

🚀 Feature

sohkai commented 4 years ago

Just wondering, is it not possible to do this at the moment? Or do we not have good utilities to make this easy?

dapplion commented 4 years ago

Proposal

Expose a hook called setPermissions which allows the user to customize the permission setting. The API could be similar to the app installer where a custom utility function is passed as an argument to minimize code. For example:

hooks: {
  setPermissions: async function ( { setPermission }, bre) {
    await setPermission("INCREMENT_ROLE", anotherAppInstance.address)
    await setPermission("DECREMENT_ROLE", "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")

    // Possible keyword shortcuts
    await setPermission("ANOTHER_ROLE", "me")
    await setPermission("ANOTHER_ROLE", "rootAccount")
    await setPermission("ANOTHER_ROLE", "any")
    await setPermission("ANOTHER_ROLE", "anyEntity")
  }
}
0xGabi commented 4 years ago

is it not possible to do this at the moment?

At this moment we assign all permission open for the proxy app. Then you can customize it on the client but not from the script.

@dapplion I like that approach. In my opinion just allowing any, anyEntity keyword. And otherwise an entity address.

sohkai commented 4 years ago

Expose a hook called setPermissions

When in the hooks lifecycle would this happen? I'm primarily curious if this hook would work for setting permissions in any other apps installed rather than only scoped to the current app.

We should also default to the normal behaviour if this hook is not defined (and probably log that we're defaulting the behaviour?).

await setPermission("INCREMENT_ROLE", anotherAppInstance.address)

We may want to be careful about just using a string, but almost all apps should be exposing these as getters and so we could do an eth_call to see what the return is, and, failing that, use the hash (and warn that we couldn't find it on the app!)

// Possible keyword shortcuts

I like these! Depending on how we've communicated, I would use only one of me or rootAccount. Technically you can still get these values from the BRE, right?

However, I wonder if it'd be nicer to use an object mapping for the set of allowed values, so it's easier for the person to know if they're using a correct value. E.g. we've previously made a proxy for error mappings and could do something similar here that will throw if an incorrect value was attempted.