Separate out delegation revoking methods from setters?
Closed this issue · 4 comments
Following up from #11, most specifically #11 (comment), I think it is probably clearer for consuming clients of the registry to have separate setter methods for delegating permissions and for revoking delegations, rather than combining this functionality into the setters.
WDYT @0xfoobar @0xrusowsky @ryley-o?
yes, I think that makes a lot of sense. Since we are now using enumerable maps for delegations, I think explicitly removing the delegate key would cleanly fit in a revoke function as well. We can also add revokeAll method(s) that revoke all delegations if we remove keys.
I think it can make sense to add explicit functionalities. Nevertheless, I don't dislike the current minimalistic setup, since, with good explainers of the parameters, it shouldn't be an issue.
Overall I don't have a strong opinion on either side (both of them have benefits). So I would wait to see what @0xfoobar thinks of this since he is the repo owner.
Currently working to implement this on top of the expiration logic since I had the green light from foobar.
revokeAll is already implemented with revokeDelegate see #13
In the new implementation I get rid of expiration = 0 as an input value and add revokeForContract and revokeForToken as the required methods to revoke previously existing delegations.
Can someone review #17 and see if any further development is needed regarding revokings?
@0xfoobar @jakerockland @ryley-o