feat: add access control for authorized callers

[xorsal]

🤖 Linear

Closes OPO-646

[gas1cent]

Just a note here that we don't have any integration tests for the BondEscalationResolutionModule, only for the dispute one. Hence we're not adding it to the list of authorized callers.

[xorsal]

I changed vm.prank to vm.startPrank here because I was testing something but it doesn't hurt tbh

[0xJabberwock]

I think the erased expectation of reversion should still be tested, since the attacker could theoretically be an authorized caller.

[xorsal]

Good point.
What do you think about mocking authorizedCallers immediately after the line you selected, something like this:

stdstore
  .target(address(_bondEscalationAccounting))
  .sig('authorizedCallers(address)')
  .with_key(_attacker)
  .checked_write(true);

vm.expectRevert(ValidatorLib.ValidatorLib_InvalidDisputeBody.selector);
_bondEscalationAccounting.releasePledge(mockRequest, mockDispute, _attacker, usdc, _pledgeSize * 4);

[0xJabberwock]

That could work, although I'd try to avoid mocking in integration tests—a workaround could be to redeploy BondEscalationAccounting with the attacker as an authorized caller (maybe in a separate test case).

[xorsal]

Perfect 👌
Yes, I thought about that but redeploying the BondEscalationAccounting is somewhat cumbersome because integration tests rely on common variables like mockRequest.

[0xJabberwock]

I think the whole dispute escalation flow should be replicated in test_authorizedAttackerAllowedModules, as these tests are meant to simulate a specific attack which was once possible.

[xorsal]

I think this further escalation is only meant to exemplify a particular scenario (a request/dispute gathers tokens from multiple parties/sources) so it won't change the final result (revert when the attacker tries to finalize the attack) but I can add it to replicate the exact same scenario of attackerAllowedModules if that makes it clearer.

[0xJabberwock]

I think the purpose of these test cases in particular should be to be anti-regressive with a specific bug, hence why I wouldn't disesteem the featured scenario in any of them. However, we can invoke its creator's opinion: @0xShaito.

[xorsal]

Anyway, I agree with your reasoning. I'll revert that commit and redo it.

[linear]

OPO-646 WOND-5 Bond escalation accounting access control