From a5b3e2068e5c8636474baefadc670a453d766129 Mon Sep 17 00:00:00 2001 From: Ashitaka Date: Mon, 8 Jul 2024 23:16:19 -0300 Subject: [PATCH 1/2] fix: checks address proposer --- .../contracts/modules/dispute/BondedDisputeModule.sol | 3 ++- .../interfaces/modules/dispute/IBondedDisputeModule.sol | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/solidity/contracts/modules/dispute/BondedDisputeModule.sol b/solidity/contracts/modules/dispute/BondedDisputeModule.sol index 9c43ea64..bc4482de 100644 --- a/solidity/contracts/modules/dispute/BondedDisputeModule.sol +++ b/solidity/contracts/modules/dispute/BondedDisputeModule.sol @@ -48,7 +48,7 @@ contract BondedDisputeModule is Module, IBondedDisputeModule { function onDisputeStatusChange( bytes32 _disputeId, IOracle.Request calldata _request, - IOracle.Response calldata, /* _response */ + IOracle.Response calldata _response, /* _response */ IOracle.Dispute calldata _dispute ) external onlyOracle { RequestParameters memory _params = decodeRequestData(_request.disputeModuleData); @@ -70,6 +70,7 @@ contract BondedDisputeModule is Module, IBondedDisputeModule { _amount: _params.bondSize }); } else if (_status == IOracle.DisputeStatus.Won) { + if (_dispute.proposer != _response.proposer) revert BondedDisputeModule_OnlyResponseProposer(); // Disputer won, we pay the disputer and release their bond _params.accountingExtension.pay({ _requestId: _dispute.requestId, diff --git a/solidity/interfaces/modules/dispute/IBondedDisputeModule.sol b/solidity/interfaces/modules/dispute/IBondedDisputeModule.sol index cfe2ff61..455c8c3a 100644 --- a/solidity/interfaces/modules/dispute/IBondedDisputeModule.sol +++ b/solidity/interfaces/modules/dispute/IBondedDisputeModule.sol @@ -15,6 +15,14 @@ import {IAccountingExtension} from '../../extensions/IAccountingExtension.sol'; * the tokens are either returned to the disputer or to the proposer. */ interface IBondedDisputeModule is IDisputeModule { + /*/////////////////////////////////////////////////////////////// + ERRORS + //////////////////////////////////////////////////////////////*/ + + /** + * @notice Thrown when the response proposer tries to dispute the response + */ + error BondedDisputeModule_OnlyResponseProposer(); /*/////////////////////////////////////////////////////////////// STRUCTS //////////////////////////////////////////////////////////////*/ From 4e4bebaf089e3b8878e8ea26dab001623b20838f Mon Sep 17 00:00:00 2001 From: Ashitaka Date: Wed, 10 Jul 2024 22:24:08 -0300 Subject: [PATCH 2/2] fix: comments --- .../modules/dispute/BondedDisputeModule.sol | 2 +- .../modules/dispute/IBondedDisputeModule.sol | 2 +- .../modules/dispute/BondedDisputeModule.t.sol | 26 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/solidity/contracts/modules/dispute/BondedDisputeModule.sol b/solidity/contracts/modules/dispute/BondedDisputeModule.sol index bc4482de..fc9b02bb 100644 --- a/solidity/contracts/modules/dispute/BondedDisputeModule.sol +++ b/solidity/contracts/modules/dispute/BondedDisputeModule.sol @@ -70,7 +70,7 @@ contract BondedDisputeModule is Module, IBondedDisputeModule { _amount: _params.bondSize }); } else if (_status == IOracle.DisputeStatus.Won) { - if (_dispute.proposer != _response.proposer) revert BondedDisputeModule_OnlyResponseProposer(); + if (_dispute.proposer != _response.proposer) revert BondedDisputeModule_SelfDispute(); // Disputer won, we pay the disputer and release their bond _params.accountingExtension.pay({ _requestId: _dispute.requestId, diff --git a/solidity/interfaces/modules/dispute/IBondedDisputeModule.sol b/solidity/interfaces/modules/dispute/IBondedDisputeModule.sol index 455c8c3a..235ab5e5 100644 --- a/solidity/interfaces/modules/dispute/IBondedDisputeModule.sol +++ b/solidity/interfaces/modules/dispute/IBondedDisputeModule.sol @@ -22,7 +22,7 @@ interface IBondedDisputeModule is IDisputeModule { /** * @notice Thrown when the response proposer tries to dispute the response */ - error BondedDisputeModule_OnlyResponseProposer(); + error BondedDisputeModule_SelfDispute(); /*/////////////////////////////////////////////////////////////// STRUCTS //////////////////////////////////////////////////////////////*/ diff --git a/solidity/test/unit/modules/dispute/BondedDisputeModule.t.sol b/solidity/test/unit/modules/dispute/BondedDisputeModule.t.sol index 4f663538..135cfba3 100644 --- a/solidity/test/unit/modules/dispute/BondedDisputeModule.t.sol +++ b/solidity/test/unit/modules/dispute/BondedDisputeModule.t.sol @@ -220,6 +220,32 @@ contract BondedDisputeModule_Unit_OnDisputeStatusChange is BaseTest { } } + /** + * @notice Test if onDisputeStatusChange reverts when the dispute is self-disputed + */ + function test_revertIfSelfDispute(uint256 _bondSize, IERC20 _token) public { + mockRequest.disputeModuleData = + abi.encode(IBondedDisputeModule.RequestParameters(accountingExtension, _token, _bondSize)); + bytes32 _requestId = _getId(mockRequest); + mockDispute.requestId = _requestId; + bytes32 _disputeId = _getId(mockDispute); + + // Mock and expect IOracle.disputeStatus to be called + _mockAndExpect( + address(oracle), abi.encodeCall(oracle.disputeStatus, (_disputeId)), abi.encode(IOracle.DisputeStatus.Won) + ); + + // Change the proposer to the disputer + mockDispute.proposer = makeAddr('newProposer'); + + // Check: revert if self-dispute + vm.expectRevert(IBondedDisputeModule.BondedDisputeModule_SelfDispute.selector); + + // Test: call disputeResponse with self-dispute + vm.prank(address(oracle)); + bondedDisputeModule.onDisputeStatusChange(_disputeId, mockRequest, mockResponse, mockDispute); + } + /** * @notice Test if onDisputeStatusChange reverts when called by caller who's not the oracle */