Add TipCollectionToggler contract#439
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| modifier onlyActivated() { | ||
| if (expiryTimestamp == 0) { | ||
| revert NotActivated(); | ||
| } | ||
| _; | ||
| } |
godzillaba
left a comment
godzillaba
left a comment
There was a problem hiding this comment.
LGTM! just a nit about events
we should probably wait for a precompiles release before merging to develop though
| if (block.timestamp < expiryTimestamp) { | ||
| revert NotExpired(); | ||
| } | ||
| ARB_OWNER.removeChainOwner(address(this)); |
There was a problem hiding this comment.
nit but maybe we should add events to revoke and setCollectTips?
There was a problem hiding this comment.
i know ArbOwner emits them too but can't hurt
There was a problem hiding this comment.
Thanks! The removeChainOwner method already emits a specific ChainOwnerRemoved in nitro (since ArbOS 60). But I've added a CollectTipsUpdated(bool) event for the setCollectTips method, since nitro only emits the generic OwnerActs event.
What do you think?
There was a problem hiding this comment.
personally i'd just add a redundant event but this sgtm too. the ChainOwnerRemoved event alone doesn't tell you how the chain owner was removed which i guess could be a reason to include it
There was a problem hiding this comment.
That's fair yeah. I added a simple Revoked() event
3 participants
This PR adds a TipCollectionToggler contract that allows a trusted party to enable/disable tip collection in a chain (available from ArbOS 60), during a specified time. This contract should be added as a chain owner.
It also updates the pin of the
precompilessubmodule to include the ArbOwner method that needs to be called.