Disputables: Avoid calling close action if closed #594
Conversation
| it('does not revert', async () => { | ||
| await disputable.newAction(0, '0x00', owner) | ||
| it('submits a new action', async () => { | ||
| const { receipt } = await disputable.newAction(0, '0x00', owner) |
There was a problem hiding this comment.
Looks like some opportunities to use the EMPTY_BYTES :)
There was a problem hiding this comment.
Remember we don't have the helpers here yet haha, but I'll extract this to a constant so we don't miss it :)
|
|
||
| const logs = decodeEventsOfType(receipt, AgreementMock.abi, 'NewAction') | ||
| assertAmountOfEvents({ logs }, 'NewAction') | ||
| assertEvent({ logs }, 'NewAction', { disputableActionId: 0, context: '0x00', submitter: owner }) |
There was a problem hiding this comment.
Minor nitpick, but we could use embed use the decodeAbiFor option now :)
There was a problem hiding this comment.
We don't have the helpers here yet haha
| (,,,,, bool closed,,) = agreement.getAction(_actionId); | ||
|
|
||
| if (!closed) { | ||
| agreement.closeAction(_actionId); |
There was a problem hiding this comment.
Not opposed to it, but I think we should do a small trial run to see how much the gas difference actually is, if this is just an optimization.
The extra call to agreement.getAction() may remove the large part of the savings if closeAction() is fairly simple on already-closed actions.
There was a problem hiding this comment.
Yea! I tested this and it is not an optimizing it actually, I'll roll it back
|
Closing in favor of aragon/aragon-apps#1231 |
Related to aragon/aragon-apps#1214
This PR adds an improvement to avoid calling
IAgreement#closeActionfrom the internal helper method ofDisputableAragonAppif the requested action is already closed.