diff --git a/solidity/contracts/modules/dispute/BondEscalationModule.sol b/solidity/contracts/modules/dispute/BondEscalationModule.sol index af6d0f34..12e0c53a 100644 --- a/solidity/contracts/modules/dispute/BondEscalationModule.sol +++ b/solidity/contracts/modules/dispute/BondEscalationModule.sol @@ -33,15 +33,14 @@ contract BondEscalationModule is Module, IBondEscalationModule { IOracle.Response calldata _response, IOracle.Dispute calldata _dispute ) external onlyOracle { + bytes32 _disputeId = _getId(_dispute); RequestParameters memory _params = decodeRequestData(_request.disputeModuleData); + BondEscalation storage _escalation = _escalations[_dispute.requestId]; if (block.number > ORACLE.createdAt(_dispute.responseId) + _params.disputeWindow) { revert BondEscalationModule_DisputeWindowOver(); } - BondEscalation storage _escalation = _escalations[_dispute.requestId]; - bytes32 _disputeId = _getId(_dispute); - _params.accountingExtension.bond({ _bonder: _dispute.disputer, _requestId: _dispute.requestId, @@ -212,7 +211,7 @@ contract BondEscalationModule is Module, IBondEscalationModule { /// @inheritdoc IBondEscalationModule function pledgeForDispute(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute) external { bytes32 _disputeId = _getId(_dispute); - RequestParameters memory _params = _pledgeChecks(_disputeId, _request, _dispute, true); + RequestParameters memory _params = _pledgeChecks(_request, _dispute, true); _escalations[_dispute.requestId].amountOfPledgesForDispute += 1; pledgesForDispute[_dispute.requestId][msg.sender] += 1; @@ -230,7 +229,7 @@ contract BondEscalationModule is Module, IBondEscalationModule { /// @inheritdoc IBondEscalationModule function pledgeAgainstDispute(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute) external { bytes32 _disputeId = _getId(_dispute); - RequestParameters memory _params = _pledgeChecks(_disputeId, _request, _dispute, false); + RequestParameters memory _params = _pledgeChecks(_request, _dispute, false); _escalations[_dispute.requestId].amountOfPledgesAgainstDispute += 1; pledgesAgainstDispute[_dispute.requestId][msg.sender] += 1; @@ -251,9 +250,9 @@ contract BondEscalationModule is Module, IBondEscalationModule { IOracle.Response calldata _response, IOracle.Dispute calldata _dispute ) external { - bytes32 _requestId = _getId(_request); + (, bytes32 _disputeId) = _validateResponseAndDispute(_request, _response, _dispute); RequestParameters memory _params = decodeRequestData(_request.disputeModuleData); - BondEscalation storage _escalation = _escalations[_requestId]; + BondEscalation storage _escalation = _escalations[_dispute.requestId]; if (block.timestamp <= _params.bondEscalationDeadline + _params.tyingBuffer) { revert BondEscalationModule_BondEscalationNotOver(); @@ -273,7 +272,7 @@ contract BondEscalationModule is Module, IBondEscalationModule { bool _disputersWon = _pledgesForDispute > _pledgesAgainstDispute; _escalation.status = _disputersWon ? BondEscalationStatus.DisputerWon : BondEscalationStatus.DisputerLost; - emit BondEscalationStatusUpdated(_requestId, _escalation.disputeId, _escalation.status); + emit BondEscalationStatusUpdated(_dispute.requestId, _disputeId, _escalation.status); ORACLE.updateDisputeStatus( _request, _response, _dispute, _disputersWon ? IOracle.DisputeStatus.Won : IOracle.DisputeStatus.Lost @@ -282,18 +281,17 @@ contract BondEscalationModule is Module, IBondEscalationModule { /** * @notice Checks the necessary conditions for pledging - * @param _disputeId The ID of the dispute to pledge for or against * @param _request The request data * @param _dispute The dispute data * @param _forDispute Whether the pledge is for or against the dispute * @return _params The decoded parameters for the request */ function _pledgeChecks( - bytes32 _disputeId, IOracle.Request calldata _request, IOracle.Dispute calldata _dispute, bool _forDispute ) internal view returns (RequestParameters memory _params) { + bytes32 _disputeId = _validateDispute(_request, _dispute); BondEscalation memory _escalation = _escalations[_dispute.requestId]; if (_disputeId != _escalation.disputeId) { diff --git a/solidity/contracts/modules/resolution/BondEscalationResolutionModule.sol b/solidity/contracts/modules/resolution/BondEscalationResolutionModule.sol index 4ce18f4a..1246b792 100644 --- a/solidity/contracts/modules/resolution/BondEscalationResolutionModule.sol +++ b/solidity/contracts/modules/resolution/BondEscalationResolutionModule.sol @@ -83,8 +83,6 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu IOracle.Response calldata _response, IOracle.Dispute calldata _dispute ) external onlyOracle { - bytes32 _requestId = _dispute.requestId; - Escalation storage _escalation = escalations[_disputeId]; if (_escalation.resolution != Resolution.Unresolved) revert BondEscalationResolutionModule_AlreadyResolved(); @@ -119,13 +117,12 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu } ORACLE.updateDisputeStatus(_request, _response, _dispute, _disputeStatus); - emit DisputeResolved(_requestId, _disputeId, _disputeStatus); + emit DisputeResolved(_dispute.requestId, _disputeId, _disputeStatus); } /// @inheritdoc IBondEscalationResolutionModule function claimPledge(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute) external { - bytes32 _disputeId = _getId(_dispute); - bytes32 _requestId = _dispute.requestId; + bytes32 _disputeId = _validateDispute(_request, _dispute); Escalation storage _escalation = escalations[_disputeId]; if (_escalation.resolution == Resolution.Unresolved) revert BondEscalationResolutionModule_NotResolved(); @@ -143,7 +140,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu _reward = FixedPointMathLib.mulDivDown(_escalation.pledgesAgainst, _pledgerProportion, BASE); _amountToRelease = _reward + _pledgerBalanceBefore; _claimPledge({ - _requestId: _requestId, + _requestId: _dispute.requestId, _disputeId: _disputeId, _amountToRelease: _amountToRelease, _resolution: _escalation.resolution, @@ -156,7 +153,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu _reward = FixedPointMathLib.mulDivDown(_escalation.pledgesFor, _pledgerProportion, BASE); _amountToRelease = _reward + _pledgerBalanceBefore; _claimPledge({ - _requestId: _requestId, + _requestId: _dispute.requestId, _disputeId: _disputeId, _amountToRelease: _amountToRelease, _resolution: _escalation.resolution, @@ -169,7 +166,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu if (_pledgerBalanceFor > 0) { pledgesForDispute[_disputeId][msg.sender] -= _pledgerBalanceFor; _claimPledge({ - _requestId: _requestId, + _requestId: _dispute.requestId, _disputeId: _disputeId, _amountToRelease: _pledgerBalanceFor, _resolution: _escalation.resolution, @@ -180,7 +177,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu if (_pledgerBalanceAgainst > 0) { pledgesAgainstDispute[_disputeId][msg.sender] -= _pledgerBalanceAgainst; _claimPledge({ - _requestId: _requestId, + _requestId: _dispute.requestId, _disputeId: _disputeId, _amountToRelease: _pledgerBalanceAgainst, _resolution: _escalation.resolution, @@ -204,8 +201,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu uint256 _pledgeAmount, bool _pledgingFor ) internal { - bytes32 _disputeId = _getId(_dispute); - bytes32 _requestId = _dispute.requestId; + bytes32 _disputeId = _validateDispute(_request, _dispute); Escalation storage _escalation = escalations[_disputeId]; if (_escalation.startTime == 0) revert BondEscalationResolutionModule_NotEscalated(); @@ -224,7 +220,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu _params.accountingExtension.pledge({ _pledger: msg.sender, - _requestId: _requestId, + _requestId: _dispute.requestId, _disputeId: _disputeId, _token: _params.bondToken, _amount: _pledgeAmount @@ -237,7 +233,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu _escalation.pledgesFor += _pledgeAmount; pledgesForDispute[_disputeId][msg.sender] += _pledgeAmount; - emit PledgedForDispute(msg.sender, _requestId, _disputeId, _pledgeAmount); + emit PledgedForDispute(msg.sender, _dispute.requestId, _disputeId, _pledgeAmount); } else { if (_inequalityData.inequalityStatus == InequalityStatus.ForTurnToEqualize) { revert BondEscalationResolutionModule_ForTurnToEqualize(); @@ -245,7 +241,7 @@ contract BondEscalationResolutionModule is Module, IBondEscalationResolutionModu _escalation.pledgesAgainst += _pledgeAmount; pledgesAgainstDispute[_disputeId][msg.sender] += _pledgeAmount; - emit PledgedAgainstDispute(msg.sender, _requestId, _disputeId, _pledgeAmount); + emit PledgedAgainstDispute(msg.sender, _dispute.requestId, _disputeId, _pledgeAmount); } if (_escalation.pledgesFor + _escalation.pledgesAgainst >= _params.pledgeThreshold) { diff --git a/solidity/contracts/modules/resolution/ERC20ResolutionModule.sol b/solidity/contracts/modules/resolution/ERC20ResolutionModule.sol index 6917b9f7..2f80211e 100644 --- a/solidity/contracts/modules/resolution/ERC20ResolutionModule.sol +++ b/solidity/contracts/modules/resolution/ERC20ResolutionModule.sol @@ -56,7 +56,7 @@ contract ERC20ResolutionModule is Module, IERC20ResolutionModule { IOracle.Dispute calldata _dispute, uint256 _numberOfVotes ) public { - bytes32 _disputeId = _getId(_dispute); + bytes32 _disputeId = _validateDispute(_request, _dispute); Escalation memory _escalation = escalations[_disputeId]; if (_escalation.startTime == 0) revert ERC20ResolutionModule_DisputeNotEscalated(); if (ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.Escalated) { @@ -111,7 +111,7 @@ contract ERC20ResolutionModule is Module, IERC20ResolutionModule { /// @inheritdoc IERC20ResolutionModule function claimVote(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute) external { - bytes32 _disputeId = _getId(_dispute); + bytes32 _disputeId = _validateDispute(_request, _dispute); Escalation memory _escalation = escalations[_disputeId]; // Check that voting deadline is over diff --git a/solidity/contracts/modules/resolution/PrivateERC20ResolutionModule.sol b/solidity/contracts/modules/resolution/PrivateERC20ResolutionModule.sol index f0c0cfd1..ff64a8fe 100644 --- a/solidity/contracts/modules/resolution/PrivateERC20ResolutionModule.sol +++ b/solidity/contracts/modules/resolution/PrivateERC20ResolutionModule.sol @@ -52,7 +52,7 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule { /// @inheritdoc IPrivateERC20ResolutionModule function commitVote(IOracle.Request calldata _request, IOracle.Dispute calldata _dispute, bytes32 _commitment) public { - bytes32 _disputeId = _getId(_dispute); + bytes32 _disputeId = _validateDispute(_request, _dispute); if (ORACLE.createdAt(_disputeId) == 0) revert PrivateERC20ResolutionModule_NonExistentDispute(); if (ORACLE.disputeStatus(_disputeId) != IOracle.DisputeStatus.Escalated) { revert PrivateERC20ResolutionModule_AlreadyResolved(); @@ -78,7 +78,7 @@ contract PrivateERC20ResolutionModule is Module, IPrivateERC20ResolutionModule { uint256 _numberOfVotes, bytes32 _salt ) public { - bytes32 _disputeId = _getId(_dispute); + bytes32 _disputeId = _validateDispute(_request, _dispute); Escalation memory _escalation = escalations[_disputeId]; if (_escalation.startTime == 0) revert PrivateERC20ResolutionModule_DisputeNotEscalated(); diff --git a/solidity/interfaces/modules/resolution/IERC20ResolutionModule.sol b/solidity/interfaces/modules/resolution/IERC20ResolutionModule.sol index e586f6a4..86429c23 100644 --- a/solidity/interfaces/modules/resolution/IERC20ResolutionModule.sol +++ b/solidity/interfaces/modules/resolution/IERC20ResolutionModule.sol @@ -48,11 +48,6 @@ interface IERC20ResolutionModule is IResolutionModule { ERRORS //////////////////////////////////////////////////////////////*/ - /** - * @notice Throws if the caller is not the dispute module - */ - error ERC20ResolutionModule_OnlyDisputeModule(); - /** * @notice Throws if the dispute doesn't exist or has not been escalated */ diff --git a/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol b/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol index 551d66ed..6fc55656 100644 --- a/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol +++ b/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol @@ -212,9 +212,10 @@ contract BondEscalationModule_Unit_EscalateDispute is BaseTest { { // Set _bondEscalationDeadline to be the current timestamp to reach the second condition. _params.bondEscalationDeadline = block.timestamp; - mockRequest.disputeModuleData = abi.encode(_params); + mockRequest.disputeModuleData = abi.encode(_params); bytes32 _requestId = _getId(mockRequest); + mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); @@ -255,8 +256,8 @@ contract BondEscalationModule_Unit_EscalateDispute is BaseTest { _params.bondEscalationDeadline = block.timestamp - 1; mockRequest.disputeModuleData = abi.encode(_params); - bytes32 _requestId = _getId(mockRequest); + mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); @@ -295,9 +296,10 @@ contract BondEscalationModule_Unit_EscalateDispute is BaseTest { _params.tyingBuffer = 1000; // Set bond escalation deadline to be the current timestamp. We will warp this. _params.bondEscalationDeadline = block.timestamp; - mockRequest.disputeModuleData = abi.encode(_params); + mockRequest.disputeModuleData = abi.encode(_params); bytes32 _requestId = _getId(mockRequest); + mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); @@ -455,7 +457,6 @@ contract BondEscalationModule_Unit_DisputeResponse is BaseTest { mockResponse.requestId = _requestId; bytes32 _responseId = _getId(mockResponse); - mockDispute.responseId = _responseId; mockDispute.requestId = _requestId; @@ -872,6 +873,7 @@ contract BondEscalationModule_Unit_OnDisputeStatusChange is BaseTest { // Set to Lost so the proposer and againstDisputePledgers win IOracle.DisputeStatus _status = IOracle.DisputeStatus.Lost; + mockRequest.disputeModuleData = abi.encode(_params); bytes32 _requestId = _getId(mockRequest); @@ -929,20 +931,24 @@ contract BondEscalationModule_Unit_OnDisputeStatusChange is BaseTest { contract BondEscalationModule_Unit_PledgeForDispute is BaseTest { /** - * @notice Tests that pledgeForDispute reverts if the dispute is not going through the bond escalation mechanism. + * @notice Tests that pledgeForDispute reverts if the dispute body is invalid. */ - function test_revertIfTheDisputeIsNotGoingThroughTheBondEscalationProcess( - bytes32 _disputeId, - bytes32 _requestId, - IOracle.Request calldata _request - ) public { - vm.assume(_disputeId > 0); + function test_revertIfInvalidDisputeBody() public { + // Check: does it revert if the dispute body is invalid? + vm.expectRevert(IModule.Module_InvalidDisputeBody.selector); + bondEscalationModule.pledgeForDispute(mockRequest, mockDispute); + } + /** + * @notice Tests that pledgeForDispute reverts if the dispute is not going through the bond escalation mechanism. + */ + function test_revertIfTheDisputeIsNotGoingThroughTheBondEscalationProcess() public { + bytes32 _requestId = _getId(mockRequest); mockDispute.requestId = _requestId; // Check: does it revert if the dispute is not escalated yet? vm.expectRevert(IBondEscalationModule.BondEscalationModule_InvalidDispute.selector); - bondEscalationModule.pledgeForDispute(_request, mockDispute); + bondEscalationModule.pledgeForDispute(mockRequest, mockDispute); } /** @@ -1111,20 +1117,24 @@ contract BondEscalationModule_Unit_PledgeForDispute is BaseTest { contract BondEscalationModule_Unit_PledgeAgainstDispute is BaseTest { /** - * @notice Tests that pledgeAgainstDispute reverts if the dispute is not going through the bond escalation mechanism. + * @notice Tests that pledgeAgainstDispute reverts if the dispute body is invalid. */ - function test_revertIfTheDisputeIsNotGoingThroughTheBondEscalationProcess( - bytes32 _disputeId, - bytes32 _requestId, - IOracle.Request calldata _request - ) public { - vm.assume(_disputeId > 0); + function test_revertIfInvalidDisputeBody() public { + // Check: does it revert if the dispute body is invalid? + vm.expectRevert(IModule.Module_InvalidDisputeBody.selector); + bondEscalationModule.pledgeAgainstDispute(mockRequest, mockDispute); + } + /** + * @notice Tests that pledgeAgainstDispute reverts if the dispute is not going through the bond escalation mechanism. + */ + function test_revertIfTheDisputeIsNotGoingThroughTheBondEscalationProcess() public { + bytes32 _requestId = _getId(mockRequest); mockDispute.requestId = _requestId; // Check: does it revert if the dispute is not escalated yet? vm.expectRevert(IBondEscalationModule.BondEscalationModule_InvalidDispute.selector); - bondEscalationModule.pledgeAgainstDispute(_request, mockDispute); + bondEscalationModule.pledgeAgainstDispute(mockRequest, mockDispute); } /** @@ -1296,6 +1306,27 @@ contract BondEscalationModule_Unit_PledgeAgainstDispute is BaseTest { } contract BondEscalationModule_Unit_SettleBondEscalation is BaseTest { + /** + * @notice Tests that settleBondEscalation reverts if the response body is invalid. + */ + function test_revertIfInvalidResponseBody() public { + // Check: does it revert if the response body is invalid? + vm.expectRevert(IModule.Module_InvalidResponseBody.selector); + bondEscalationModule.settleBondEscalation(mockRequest, mockResponse, mockDispute); + } + + /** + * @notice Tests that settleBondEscalation reverts if the dispute body is invalid. + */ + function test_revertIfInvalidDisputeBody() public { + bytes32 _requestId = _getId(mockRequest); + mockResponse.requestId = _requestId; + + // Check: does it revert if the dispute body is invalid? + vm.expectRevert(IModule.Module_InvalidDisputeBody.selector); + bondEscalationModule.settleBondEscalation(mockRequest, mockResponse, mockDispute); + } + /** * @notice Tests that settleBondEscalation reverts if someone tries to settle the escalation before the tying buffer * has elapsed. @@ -1307,6 +1338,13 @@ contract BondEscalationModule_Unit_SettleBondEscalation is BaseTest { _params.tyingBuffer = bound(_params.tyingBuffer, 0, type(uint128).max); _params.bondEscalationDeadline = block.timestamp; mockRequest.disputeModuleData = abi.encode(_params); + bytes32 _requestId = _getId(mockRequest); + + mockResponse.requestId = _requestId; + bytes32 _responseId = _getId(mockResponse); + + mockDispute.requestId = _requestId; + mockDispute.responseId = _responseId; // Check: does it revert if the bond escalation is not over? vm.expectRevert(IBondEscalationModule.BondEscalationModule_BondEscalationNotOver.selector); @@ -1324,9 +1362,14 @@ contract BondEscalationModule_Unit_SettleBondEscalation is BaseTest { _params.bondEscalationDeadline = block.timestamp; _params.tyingBuffer = 1000; mockRequest.disputeModuleData = abi.encode(_params); - bytes32 _requestId = _getId(mockRequest); + mockResponse.requestId = _requestId; + bytes32 _responseId = _getId(mockResponse); + + mockDispute.requestId = _requestId; + mockDispute.responseId = _responseId; + vm.warp(_params.bondEscalationDeadline + _params.tyingBuffer + 1); bondEscalationModule.forTest_setBondEscalationStatus(_requestId, IBondEscalationModule.BondEscalationStatus.None); @@ -1349,7 +1392,11 @@ contract BondEscalationModule_Unit_SettleBondEscalation is BaseTest { mockRequest.disputeModuleData = abi.encode(_params); bytes32 _requestId = _getId(mockRequest); + mockResponse.requestId = _requestId; + bytes32 _responseId = _getId(mockResponse); + mockDispute.requestId = _requestId; + mockDispute.responseId = _responseId; bytes32 _disputeId = _getId(mockDispute); vm.warp(_params.bondEscalationDeadline + _params.tyingBuffer + 1); @@ -1378,9 +1425,13 @@ contract BondEscalationModule_Unit_SettleBondEscalation is BaseTest { _params.bondEscalationDeadline = block.timestamp; _params.tyingBuffer = 1000; mockRequest.disputeModuleData = abi.encode(_params); - bytes32 _requestId = _getId(mockRequest); + + mockResponse.requestId = _requestId; + bytes32 _responseId = _getId(mockResponse); + mockDispute.requestId = _requestId; + mockDispute.responseId = _responseId; bytes32 _disputeId = _getId(mockDispute); vm.warp(_params.bondEscalationDeadline + _params.tyingBuffer + 1); @@ -1422,9 +1473,13 @@ contract BondEscalationModule_Unit_SettleBondEscalation is BaseTest { _params.bondEscalationDeadline = block.timestamp; _params.tyingBuffer = 1000; mockRequest.disputeModuleData = abi.encode(_params); - bytes32 _requestId = _getId(mockRequest); + + mockResponse.requestId = _requestId; + bytes32 _responseId = _getId(mockResponse); + mockDispute.requestId = _requestId; + mockDispute.responseId = _responseId; bytes32 _disputeId = _getId(mockDispute); vm.warp(_params.bondEscalationDeadline + _params.tyingBuffer + 1); diff --git a/solidity/test/unit/modules/resolution/BondEscalationResolutionModule.t.sol b/solidity/test/unit/modules/resolution/BondEscalationResolutionModule.t.sol index 4f669c1e..7eb0e1ac 100644 --- a/solidity/test/unit/modules/resolution/BondEscalationResolutionModule.t.sol +++ b/solidity/test/unit/modules/resolution/BondEscalationResolutionModule.t.sol @@ -249,14 +249,16 @@ contract BondEscalationResolutionModule_Unit_PledgeForDispute is BaseTest { // block.timestamp < _startTime + _timeUntilDeadline _startTime = uint128(block.timestamp - timeUntilDeadline + 1); - - (_requestId,, _disputeId) = _setResolutionModuleData(requestParameters); } - function test_reverts( + function test_pledgeForDisputeReverts( uint256 _pledgeAmount, IBondEscalationResolutionModule.RequestParameters memory _params ) public assumeFuzzable(address(_params.accountingExtension)) { + // Check: does it revert if the dispute body is invalid? + vm.expectRevert(IModule.Module_InvalidDisputeBody.selector); + module.pledgeForDispute(mockRequest, mockDispute, _pledgeAmount); + // 1. BondEscalationResolutionModule_NotEscalated (_requestId,, _disputeId) = _setResolutionModuleData(_params); @@ -382,6 +384,8 @@ contract BondEscalationResolutionModule_Unit_PledgeForDispute is BaseTest { uint256 _pledgesFor = 0; uint256 _pledgesAgainst = _pledgeAmount * 200 / 300; + (_requestId,, _disputeId) = _setResolutionModuleData(requestParameters); + // Set all data module.forTest_setEscalation( _disputeId, IBondEscalationResolutionModule.Resolution.Unresolved, _startTime, _pledgesFor, _pledgesAgainst @@ -431,6 +435,8 @@ contract BondEscalationResolutionModule_Unit_PledgeForDispute is BaseTest { uint256 _pledgesFor = 100_000; uint256 _pledgesAgainst = (_pledgeAmount + _pledgesFor) * 301 / 200; + (_requestId,, _disputeId) = _setResolutionModuleData(requestParameters); + // Set the data module.forTest_setEscalation( _disputeId, IBondEscalationResolutionModule.Resolution.Unresolved, _startTime, _pledgesFor, _pledgesAgainst @@ -476,6 +482,8 @@ contract BondEscalationResolutionModule_Unit_PledgeForDispute is BaseTest { uint256 _pledgesFor = 100_000; uint256 _pledgesAgainst = (_pledgeAmount + _pledgesFor); + (_requestId,, _disputeId) = _setResolutionModuleData(requestParameters); + // Resetting the pledges values module.forTest_setEscalation( _disputeId, IBondEscalationResolutionModule.Resolution.Unresolved, _startTime, _pledgesFor, _pledgesAgainst @@ -526,14 +534,16 @@ contract BondEscalationResolutionModule_Unit_PledgeAgainstDispute is BaseTest { // block.timestamp < _startTime + _timeUntilDeadline _startTime = uint128(block.timestamp - timeUntilDeadline + 1); - - (_requestId,, _disputeId) = _setResolutionModuleData(requestParameters); } - function test_reverts( + function test_pledgeAgainstDisputeReverts( uint256 _pledgeAmount, IBondEscalationResolutionModule.RequestParameters memory _params ) public assumeFuzzable(address(_params.accountingExtension)) { + // Check: does it revert if the dispute body is invalid? + vm.expectRevert(IModule.Module_InvalidDisputeBody.selector); + module.pledgeAgainstDispute(mockRequest, mockDispute, _pledgeAmount); + // 1. BondEscalationResolutionModule_NotEscalated (_requestId,, _disputeId) = _setResolutionModuleData(_params); @@ -658,6 +668,8 @@ contract BondEscalationResolutionModule_Unit_PledgeAgainstDispute is BaseTest { uint256 _pledgesAgainst = 0; uint256 _pledgesFor = _pledgeAmount * 200 / 300; + (_requestId,, _disputeId) = _setResolutionModuleData(requestParameters); + // Set all data module.forTest_setEscalation( _disputeId, IBondEscalationResolutionModule.Resolution.Unresolved, _startTime, _pledgesFor, _pledgesAgainst @@ -707,6 +719,8 @@ contract BondEscalationResolutionModule_Unit_PledgeAgainstDispute is BaseTest { uint256 _pledgesAgainst = 100_000; uint256 _pledgesFor = (_pledgeAmount + _pledgesAgainst) * 301 / 200; + (_requestId,, _disputeId) = _setResolutionModuleData(requestParameters); + // Set the data module.forTest_setEscalation( _disputeId, IBondEscalationResolutionModule.Resolution.Unresolved, _startTime, _pledgesFor, _pledgesAgainst @@ -752,6 +766,8 @@ contract BondEscalationResolutionModule_Unit_PledgeAgainstDispute is BaseTest { uint256 _pledgesAgainst = 100_000; uint256 _pledgesFor = (_pledgeAmount + _pledgesAgainst); + (_requestId,, _disputeId) = _setResolutionModuleData(requestParameters); + // Resetting the pledges values module.forTest_setEscalation( _disputeId, IBondEscalationResolutionModule.Resolution.Unresolved, _startTime, _pledgesFor, _pledgesAgainst @@ -804,7 +820,7 @@ contract BondEscalationResolutionModule_Unit_ResolveDispute is BaseTest { the disputer. */ - function test_reverts(IBondEscalationResolutionModule.RequestParameters memory _params) + function test_resolveDisputeReverts(IBondEscalationResolutionModule.RequestParameters memory _params) public assumeFuzzable(address(_params.accountingExtension)) { @@ -990,14 +1006,19 @@ contract BondEscalationResolutionModule_Unit_ResolveDispute is BaseTest { } contract BondEscalationResolutionModule_Unit_ClaimPledge is BaseTest { - function test_reverts( - bytes32 _disputeId, + function test_claimPledgeReverts( uint256 _pledgesFor, uint256 _pledgesAgainst, uint128 _startTime, address _randomPledger, - IOracle.Request calldata _request - ) public { + IBondEscalationResolutionModule.RequestParameters memory _params + ) public assumeFuzzable(address(_params.accountingExtension)) { + // Check: does it revert if the dispute body is invalid? + vm.expectRevert(IModule.Module_InvalidDisputeBody.selector); + module.claimPledge(mockRequest, mockDispute); + + (,, bytes32 _disputeId) = _setResolutionModuleData(_params); + // Set a mock escalation with resolution == Unresolved module.forTest_setEscalation( _disputeId, IBondEscalationResolutionModule.Resolution.Unresolved, _startTime, _pledgesFor, _pledgesAgainst @@ -1008,7 +1029,7 @@ contract BondEscalationResolutionModule_Unit_ClaimPledge is BaseTest { // Check: does it revert if trying to claim a pledge of a not resolved escalation? vm.expectRevert(IBondEscalationResolutionModule.BondEscalationResolutionModule_NotResolved.selector); - module.claimPledge(_request, mockDispute); + module.claimPledge(mockRequest, mockDispute); } function test_disputerWon( diff --git a/solidity/test/unit/modules/resolution/ERC20ResolutionModule.t.sol b/solidity/test/unit/modules/resolution/ERC20ResolutionModule.t.sol index 45ad4563..ebbe6419 100644 --- a/solidity/test/unit/modules/resolution/ERC20ResolutionModule.t.sol +++ b/solidity/test/unit/modules/resolution/ERC20ResolutionModule.t.sol @@ -202,6 +202,15 @@ contract ERC20ResolutionModule_Unit_CastVote is BaseTest { assertEq(module.votes(_disputeId, _voter), _amountOfVotes); } + /** + * @notice Test that `castVote` reverts if the dispute body is invalid. + */ + function test_revertIfInvalidDisputeBody(uint256 _numberOfVotes) public { + // Check: does it revert if the dispute body is invalid? + vm.expectRevert(IModule.Module_InvalidDisputeBody.selector); + module.castVote(mockRequest, mockDispute, _numberOfVotes); + } + /** * @notice Test that `castVote` reverts if called with `_disputeId` of a non-escalated dispute. */ @@ -256,7 +265,6 @@ contract ERC20ResolutionModule_Unit_CastVote is BaseTest { timeUntilDeadline: votingTimeWindow }) ); - bytes32 _requestId = _getId(mockRequest); mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); @@ -289,7 +297,6 @@ contract ERC20ResolutionModule_Unit_ResolveDispute is BaseTest { timeUntilDeadline: votingTimeWindow }) ); - bytes32 _requestId = _getId(mockRequest); mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); @@ -363,7 +370,6 @@ contract ERC20ResolutionModule_Unit_ResolveDispute is BaseTest { ); mockDispute.requestId = _getId(mockRequest); bytes32 _disputeId = _getId(mockDispute); - module.forTest_setStartTime(_disputeId, 500_000); _mockAndExpect( @@ -381,6 +387,15 @@ contract ERC20ResolutionModule_Unit_ResolveDispute is BaseTest { } contract ERC20ResolutionModule_Unit_ClaimVote is BaseTest { + /** + * @notice Reverts if the dispute body is invalid + */ + function test_revertIfInvalidDisputeBody() public { + // Check: does it revert if the dispute body is invalid? + vm.expectRevert(IModule.Module_InvalidDisputeBody.selector); + module.claimVote(mockRequest, mockDispute); + } + /** * @notice Reverts if the vote is still ongoing */ @@ -393,7 +408,6 @@ contract ERC20ResolutionModule_Unit_ClaimVote is BaseTest { timeUntilDeadline: 1000 }) ); - mockDispute.requestId = _getId(mockRequest); module.forTest_setStartTime(_getId(mockDispute), block.timestamp); diff --git a/solidity/test/unit/modules/resolution/PrivateERC20ResolutionModule.t.sol b/solidity/test/unit/modules/resolution/PrivateERC20ResolutionModule.t.sol index a6ec44a2..bf628d53 100644 --- a/solidity/test/unit/modules/resolution/PrivateERC20ResolutionModule.t.sol +++ b/solidity/test/unit/modules/resolution/PrivateERC20ResolutionModule.t.sol @@ -226,10 +226,20 @@ contract PrivateERC20ResolutionModule_Unit_CommitVote is BaseTest { } /** - * @notice Test that `commitVote` reverts if there is no dispute with the given`_disputeId` + * @notice Test that `commitVote` reverts if the dispute body is invalid. */ - function test_revertIfNonExistentDispute(bytes32 _requestId, bytes32 _commitment) public { + function test_revertIfInvalidDisputeBody(bytes32 _commitment) public { + // Check: does it revert if the dispute body is invalid? + vm.expectRevert(IModule.Module_InvalidDisputeBody.selector); + module.commitVote(mockRequest, mockDispute, _commitment); + } + + /** + * @notice Test that `commitVote` reverts if there is no dispute with the given`_disputeId`. + */ + function test_revertIfNonExistentDispute(bytes32 _commitment) public { // Compute proper IDs + bytes32 _requestId = _getId(mockRequest); mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); @@ -244,8 +254,9 @@ contract PrivateERC20ResolutionModule_Unit_CommitVote is BaseTest { /** * @notice Test that `commitVote` reverts if called with `_disputeId` of an already active dispute. */ - function test_revertIfActive(bytes32 _requestId, bytes32 _commitment) public { + function test_revertIfActive(bytes32 _commitment) public { // Computer proper IDs + bytes32 _requestId = _getId(mockRequest); mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); @@ -264,8 +275,9 @@ contract PrivateERC20ResolutionModule_Unit_CommitVote is BaseTest { /** * @notice Test that `commitVote` reverts if called with `_disputeId` of a dispute with no resolution. */ - function test_revertIfNoResolution(bytes32 _requestId, bytes32 _commitment) public { + function test_revertIfNoResolution(bytes32 _commitment) public { // Computer proper IDs + bytes32 _requestId = _getId(mockRequest); mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); @@ -286,8 +298,9 @@ contract PrivateERC20ResolutionModule_Unit_CommitVote is BaseTest { /** * @notice Test that `commitVote` reverts if called with `_disputeId` of a dispute that has already been won. */ - function test_revertIfWon(bytes32 _requestId, bytes32 _commitment) public { + function test_revertIfWon(bytes32 _commitment) public { // Computer proper IDs + bytes32 _requestId = _getId(mockRequest); mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); @@ -306,8 +319,9 @@ contract PrivateERC20ResolutionModule_Unit_CommitVote is BaseTest { /** * @notice Test that `commitVote` reverts if called with `_disputeId` of an already resolved dispute. */ - function test_revertIfAlreadyResolved(bytes32 _requestId, bytes32 _commitment) public { + function test_revertIfAlreadyResolved(bytes32 _commitment) public { // Computer proper IDs + bytes32 _requestId = _getId(mockRequest); mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); @@ -326,8 +340,9 @@ contract PrivateERC20ResolutionModule_Unit_CommitVote is BaseTest { /** * @notice Test that `commitVote` reverts if called with `_disputeId` of a non-escalated dispute. */ - function test_revertIfNotEscalated(bytes32 _requestId, bytes32 _commitment) public { + function test_revertIfNotEscalated(bytes32 _commitment) public { // Compute proper IDs + bytes32 _requestId = _getId(mockRequest); mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); @@ -439,10 +454,23 @@ contract PrivateERC20ResolutionModule_Unit_RevealVote is BaseTest { assertEq(_voterData.numOfVotes, _amountOfVotes); } + /** + * @notice Test that `revealVote` reverts if the dispute body is invalid. + */ + function test_revertIfInvalidDisputeBody(uint256 _numberOfVotes, bytes32 _salt) public { + // Check: does it revert if the dispute body is invalid? + vm.expectRevert(IModule.Module_InvalidDisputeBody.selector); + module.revealVote(mockRequest, mockDispute, _numberOfVotes, _salt); + } + /** * @notice Test that `revealVote` reverts if called with `_disputeId` of a non-escalated dispute. */ function test_revertIfNotEscalated(uint256 _numberOfVotes, bytes32 _salt) public { + // Compute proper id + bytes32 _requestId = _getId(mockRequest); + mockDispute.requestId = _requestId; + // Check: does it revert if the dispute is not escalated? vm.expectRevert(IPrivateERC20ResolutionModule.PrivateERC20ResolutionModule_DisputeNotEscalated.selector); module.revealVote(mockRequest, mockDispute, _numberOfVotes, _salt);