From 358b2ff2c2edbd81378c37eb23ced4f05d251f68 Mon Sep 17 00:00:00 2001 From: Ashitaka Date: Mon, 8 Jul 2024 22:49:09 -0300 Subject: [PATCH 1/4] fix: unlock without pledge --- solidity/contracts/modules/dispute/BondEscalationModule.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solidity/contracts/modules/dispute/BondEscalationModule.sol b/solidity/contracts/modules/dispute/BondEscalationModule.sol index 1e453b90..af6d0f34 100644 --- a/solidity/contracts/modules/dispute/BondEscalationModule.sol +++ b/solidity/contracts/modules/dispute/BondEscalationModule.sol @@ -126,7 +126,7 @@ contract BondEscalationModule is Module, IBondEscalationModule { uint256 _pledgesForDispute = _escalation.amountOfPledgesForDispute; uint256 _pledgesAgainstDispute = _escalation.amountOfPledgesAgainstDispute; - if (_pledgesAgainstDispute > 0) { + if (_pledgesAgainstDispute > 0 || _pledgesForDispute > 0) { uint256 _amountToPay = _won ? _params.bondSize + FixedPointMathLib.mulDivDown(_pledgesAgainstDispute, _params.bondSize, _pledgesForDispute) From 19c3ca1d2bae1bee46ba98be8733efe42263174b Mon Sep 17 00:00:00 2001 From: Ashitaka Date: Mon, 22 Jul 2024 02:47:03 -0300 Subject: [PATCH 2/4] feat: test --- .../dispute/BondEscalationModule.t.sol | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol b/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol index 02f7b83d..10057f46 100644 --- a/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol +++ b/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol @@ -677,6 +677,51 @@ contract BondEscalationModule_Unit_OnDisputeStatusChange is BaseTest { bondEscalationModule.onDisputeStatusChange(_disputeId, mockRequest, mockResponse, mockDispute); } + /** + * @notice Tests that onDisputeStatusChange pays the disputer if the disputer won + */ + function test_callPayIfNormalDisputeWonPledgesForDispute( + IBondEscalationModule.RequestParameters memory _params, + address[] memory _pledgersforDispute + ) public assumeFuzzable(address(_params.accountingExtension)) { + vm.assume(_pledgersforDispute.length > 0 && _pledgersforDispute.length < 10); + _params.accountingExtension = IBondEscalationAccounting(makeAddr('BondEscalationAccounting')); + mockRequest.disputeModuleData = abi.encode(_params); + bytes32 _requestId = _getId(mockRequest); + + mockDispute.requestId = _requestId; + bytes32 _disputeId = _getId(mockDispute); + + // Mock and expect IAccountingExtension.pay to be called + _mockAndExpect( + address(_params.accountingExtension), + abi.encodeCall( + IAccountingExtension.pay, + (_requestId, mockDispute.proposer, mockDispute.disputer, _params.bondToken, _params.bondSize) + ), + abi.encode(true) + ); + + // Mock and expect IAccountingExtension.pay to be called + _mockAndExpect( + address(_params.accountingExtension), + abi.encodeCall( + IAccountingExtension.release, (mockDispute.disputer, _requestId, _params.bondToken, _params.bondSize) + ), + abi.encode(true) + ); + + // Mock and expect IOracle.createdAt to be called + _mockAndExpect( + address(oracle), abi.encodeCall(IOracle.disputeStatus, (_disputeId)), abi.encode(IOracle.DisputeStatus.Won) + ); + + bondEscalationModule.forTest_setBondEscalation(_requestId, _pledgersforDispute, new address[](0)); + + vm.prank(address(oracle)); + bondEscalationModule.onDisputeStatusChange(_disputeId, mockRequest, mockResponse, mockDispute); + } + function test_emitsEvent(IBondEscalationModule.RequestParameters memory _params) public assumeFuzzable(address(_params.accountingExtension)) From d3b768bb014a17e868b29ac6fdf923fe8a0c6be7 Mon Sep 17 00:00:00 2001 From: Ashitaka Date: Thu, 25 Jul 2024 22:17:52 -0300 Subject: [PATCH 3/4] fix: bondsize revert --- .../dispute/BondEscalationModule.t.sol | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol b/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol index 10057f46..f09c5c98 100644 --- a/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol +++ b/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol @@ -831,9 +831,11 @@ contract BondEscalationModule_Unit_OnDisputeStatusChange is BaseTest { * the users that pledged in favor of the dispute, as they have won. */ function test_shouldChangeBondEscalationStatusAndCallPayPledgersWon( - IBondEscalationModule.RequestParameters memory _params + IBondEscalationModule.RequestParameters memory _params, + uint256 _numPledgers ) public assumeFuzzable(address(_params.accountingExtension)) { - vm.assume(_params.bondSize < type(uint256).max / 2); + vm.assume(_params.bondSize < type(uint128).max / 2); + vm.assume(_numPledgers > 0 && _numPledgers < 30); IOracle.DisputeStatus _status = IOracle.DisputeStatus.Won; @@ -843,11 +845,8 @@ contract BondEscalationModule_Unit_OnDisputeStatusChange is BaseTest { mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); - uint256 _numForPledgers = 2; - uint256 _numAgainstPledgers = 2; - // Set bond escalation data to have pledgers and to return the winning for pledgers as in this case they won the escalation - _setBondEscalation(_requestId, _numForPledgers, _numAgainstPledgers); + _setBondEscalation(_requestId, _numPledgers, _numPledgers); // Set this dispute to have gone through the bond escalation process bondEscalationModule.forTest_setEscalatedDispute(_requestId, _disputeId); @@ -884,7 +883,7 @@ contract BondEscalationModule_Unit_OnDisputeStatusChange is BaseTest { address(_params.accountingExtension), abi.encodeCall( IBondEscalationAccounting.onSettleBondEscalation, - (_requestId, _disputeId, _params.bondToken, _params.bondSize << 1, _numForPledgers) + (_requestId, _disputeId, _params.bondToken, _params.bondSize << 1, _numPledgers) ), abi.encode() ); @@ -910,9 +909,11 @@ contract BondEscalationModule_Unit_OnDisputeStatusChange is BaseTest { * the users that pledged against the dispute, as those that pledged in favor have lost . */ function test_shouldChangeBondEscalationStatusAndCallPayPledgersLost( - IBondEscalationModule.RequestParameters memory _params + IBondEscalationModule.RequestParameters memory _params, + uint256 _numPledgers ) public assumeFuzzable(address(_params.accountingExtension)) { - vm.assume(_params.bondSize < type(uint256).max / 2); + vm.assume(_params.bondSize < type(uint128).max / 2); + vm.assume(_numPledgers > 0 && _numPledgers < 30); // Set to Lost so the proposer and againstDisputePledgers win IOracle.DisputeStatus _status = IOracle.DisputeStatus.Lost; @@ -922,11 +923,8 @@ contract BondEscalationModule_Unit_OnDisputeStatusChange is BaseTest { mockDispute.requestId = _requestId; bytes32 _disputeId = _getId(mockDispute); - uint256 _numForPledgers = 2; - uint256 _numAgainstPledgers = 2; - // Set bond escalation data to have pledgers and to return the winning for pledgers as in this case they won the escalation - _setBondEscalation(_requestId, _numForPledgers, _numAgainstPledgers); + _setBondEscalation(_requestId, _numPledgers, _numPledgers); // Set this dispute to have gone through the bond escalation process bondEscalationModule.forTest_setEscalatedDispute(_requestId, _disputeId); @@ -954,7 +952,7 @@ contract BondEscalationModule_Unit_OnDisputeStatusChange is BaseTest { address(_params.accountingExtension), abi.encodeCall( IBondEscalationAccounting.onSettleBondEscalation, - (_requestId, _disputeId, _params.bondToken, _params.bondSize << 1, _numAgainstPledgers) + (_requestId, _disputeId, _params.bondToken, _params.bondSize << 1, _numPledgers) ), abi.encode(true) ); From b5d544439226b9a4a4aac012ab33178e8710f4c9 Mon Sep 17 00:00:00 2001 From: Ashitaka Date: Fri, 26 Jul 2024 03:28:56 -0300 Subject: [PATCH 4/4] fix: comments --- .../dispute/BondEscalationModule.t.sol | 49 +------------------ 1 file changed, 2 insertions(+), 47 deletions(-) diff --git a/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol b/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol index f09c5c98..551d66ed 100644 --- a/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol +++ b/solidity/test/unit/modules/dispute/BondEscalationModule.t.sol @@ -677,51 +677,6 @@ contract BondEscalationModule_Unit_OnDisputeStatusChange is BaseTest { bondEscalationModule.onDisputeStatusChange(_disputeId, mockRequest, mockResponse, mockDispute); } - /** - * @notice Tests that onDisputeStatusChange pays the disputer if the disputer won - */ - function test_callPayIfNormalDisputeWonPledgesForDispute( - IBondEscalationModule.RequestParameters memory _params, - address[] memory _pledgersforDispute - ) public assumeFuzzable(address(_params.accountingExtension)) { - vm.assume(_pledgersforDispute.length > 0 && _pledgersforDispute.length < 10); - _params.accountingExtension = IBondEscalationAccounting(makeAddr('BondEscalationAccounting')); - mockRequest.disputeModuleData = abi.encode(_params); - bytes32 _requestId = _getId(mockRequest); - - mockDispute.requestId = _requestId; - bytes32 _disputeId = _getId(mockDispute); - - // Mock and expect IAccountingExtension.pay to be called - _mockAndExpect( - address(_params.accountingExtension), - abi.encodeCall( - IAccountingExtension.pay, - (_requestId, mockDispute.proposer, mockDispute.disputer, _params.bondToken, _params.bondSize) - ), - abi.encode(true) - ); - - // Mock and expect IAccountingExtension.pay to be called - _mockAndExpect( - address(_params.accountingExtension), - abi.encodeCall( - IAccountingExtension.release, (mockDispute.disputer, _requestId, _params.bondToken, _params.bondSize) - ), - abi.encode(true) - ); - - // Mock and expect IOracle.createdAt to be called - _mockAndExpect( - address(oracle), abi.encodeCall(IOracle.disputeStatus, (_disputeId)), abi.encode(IOracle.DisputeStatus.Won) - ); - - bondEscalationModule.forTest_setBondEscalation(_requestId, _pledgersforDispute, new address[](0)); - - vm.prank(address(oracle)); - bondEscalationModule.onDisputeStatusChange(_disputeId, mockRequest, mockResponse, mockDispute); - } - function test_emitsEvent(IBondEscalationModule.RequestParameters memory _params) public assumeFuzzable(address(_params.accountingExtension)) @@ -948,13 +903,13 @@ contract BondEscalationModule_Unit_OnDisputeStatusChange is BaseTest { ); // Mock and expect IBondEscalationAccounting.onSettleBondEscalation to be called - vm.mockCall( + _mockAndExpect( address(_params.accountingExtension), abi.encodeCall( IBondEscalationAccounting.onSettleBondEscalation, (_requestId, _disputeId, _params.bondToken, _params.bondSize << 1, _numPledgers) ), - abi.encode(true) + abi.encode() ); // Check: is th event emitted?