From a76c5a09db1a738a4683678d88154af79f20b0c4 Mon Sep 17 00:00:00 2001 From: shaito <106555513+0xShaito@users.noreply.github.com> Date: Wed, 13 Nov 2024 12:32:32 -0300 Subject: [PATCH] fix: bonded response release unutilized response (#71) Co-authored-by: Gas One Cent <86567384+gas1cent@users.noreply.github.com> --- .../modules/response/BondedResponseModule.sol | 2 +- solidity/test/integration/Finalization.t.sol | 30 ++++ .../ReleaseUnutilizedResponse.t.sol | 169 ++++++++++++++++++ .../response/BondedResponseModule.t.sol | 14 +- 4 files changed, 212 insertions(+), 3 deletions(-) create mode 100644 solidity/test/integration/ReleaseUnutilizedResponse.t.sol diff --git a/solidity/contracts/modules/response/BondedResponseModule.sol b/solidity/contracts/modules/response/BondedResponseModule.sol index cb52b7c..827ec06 100644 --- a/solidity/contracts/modules/response/BondedResponseModule.sol +++ b/solidity/contracts/modules/response/BondedResponseModule.sol @@ -112,7 +112,7 @@ contract BondedResponseModule is Module, IBondedResponseModule { } bytes32 _finalizedResponseId = ORACLE.finalizedResponseId(_response.requestId); - if (_finalizedResponseId == _responseId || _finalizedResponseId == bytes32(0)) { + if (_finalizedResponseId == _responseId || ORACLE.finalizedAt(_response.requestId) == 0) { revert BondedResponseModule_InvalidReleaseParameters(); } diff --git a/solidity/test/integration/Finalization.t.sol b/solidity/test/integration/Finalization.t.sol index d4a7055..d8aa3f8 100644 --- a/solidity/test/integration/Finalization.t.sol +++ b/solidity/test/integration/Finalization.t.sol @@ -4,11 +4,15 @@ pragma solidity ^0.8.19; import './IntegrationBase.sol'; contract Integration_Finalization is IntegrationBase { + MockAtomicArbitrator internal _mockAtomicArbitrator; address internal _finalizer = makeAddr('finalizer'); function setUp() public override { super.setUp(); + vm.prank(governance); + _mockAtomicArbitrator = new MockAtomicArbitrator(oracle); + _deposit(_accountingExtension, requester, usdc, _expectedReward); _deposit(_accountingExtension, proposer, usdc, _expectedBondSize); _deposit(_accountingExtension, disputer, usdc, _expectedBondSize); @@ -126,6 +130,32 @@ contract Integration_Finalization is IntegrationBase { oracle.finalize(mockRequest, mockResponse, _createAccessControl()); } + /** + * @notice Finalizing a request without a response. + */ + function test_finalizeWithoutResponse(bytes calldata _calldata) public { + _setFinalizationModule( + address(_callbackModule), + abi.encode(ICallbackModule.RequestParameters({target: address(_mockCallback), data: _calldata})) + ); + + _createRequest(); + + // Traveling to the end of the dispute window + vm.warp(block.timestamp + _expectedDeadline + 1 + _baseDisputeWindow); + + IOracle.Response memory _emptyResponse = + IOracle.Response({proposer: address(0), requestId: bytes32(0), response: bytes('')}); + + vm.expectCall(address(_mockCallback), abi.encodeWithSelector(IProphetCallback.prophetCallback.selector, _calldata)); + vm.prank(_finalizer); + oracle.finalize(mockRequest, _emptyResponse, _createAccessControl()); + + // Check: is response finalized? + bytes32 _finalizedResponseId = oracle.finalizedResponseId(_getId(mockRequest)); + assertEq(_finalizedResponseId, bytes32(0)); + } + /** * @notice Updates the finalization module and its data. */ diff --git a/solidity/test/integration/ReleaseUnutilizedResponse.t.sol b/solidity/test/integration/ReleaseUnutilizedResponse.t.sol new file mode 100644 index 0000000..536f11b --- /dev/null +++ b/solidity/test/integration/ReleaseUnutilizedResponse.t.sol @@ -0,0 +1,169 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +import './IntegrationBase.sol'; + +contract Integration_ReleaseUnutilizedResponse is IntegrationBase { + address internal _finalizer = makeAddr('finalizer'); + bytes32 _requestId; + bytes32 _responseId; + bytes32 _disputeId; + + function setUp() public override { + super.setUp(); + + _deposit(_accountingExtension, requester, usdc, _expectedReward); + _deposit(_accountingExtension, proposer, usdc, _expectedBondSize); + _deposit(_accountingExtension, disputer, usdc, _expectedBondSize); + + // Create request, response and dispute it + _requestId = _createRequest(); + mockResponse.requestId = _requestId; + + _responseId = _proposeResponse(mockResponse); + mockDispute.requestId = _requestId; + mockDispute.responseId = _responseId; + + _disputeId = _disputeResponse(); + } + + /** + * @notice Tests the release of unutilized response when the request is finalized with an empty response + */ + function test_releaseUnutilizedResponse_withoutResponse() public { + mockResponse.requestId = bytes32(0); + IOracle.Response memory _emptyResponse = mockResponse; + mockResponse.requestId = _requestId; + + // Finalizing the request with an empty response + _finalizeRequest(_emptyResponse); + + // Check: is request finalized with an empty response? + assertEq(oracle.finalizedResponseId(_requestId), bytes32(0)); + assertEq(oracle.finalizedAt(_requestId), block.timestamp); + + // Check: proposers funds are still bonded? + assertEq(_accountingExtension.bondedAmountOf(proposer, usdc, _requestId), _expectedBondSize, 'Balance check 1'); + + // The dispute is escalated and has no resolution + _escalateAndResolveDispute(IOracle.DisputeStatus.NoResolution); + + // Check: proposers funds are still bonded? + assertEq(_accountingExtension.bondedAmountOf(proposer, usdc, _requestId), _expectedBondSize, 'Balance check 2'); + + // Now the proposer should be able to release their unused response + vm.prank(proposer); + _responseModule.releaseUnutilizedResponse(mockRequest, mockResponse); + + // Check: proposers funds are not bonded anymore? + assertEq(_accountingExtension.bondedAmountOf(proposer, usdc, _requestId), 0, 'Balance check 3'); + + // Check: proposer received their bond back? + assertEq(_accountingExtension.balanceOf(proposer, usdc), _expectedBondSize); + } + + /** + * @notice Tests the release of unutilized response when the request is finalized with a correct response + */ + function test_releaseUnutilizedResponse_withResponse() public { + IOracle.Response memory _unutilizedResponse = mockResponse; + + IOracle.Response memory _acceptedResponse = mockResponse; + _acceptedResponse.response = bytes('123'); + _deposit(_accountingExtension, proposer, usdc, _expectedBondSize); + _proposeResponse(_acceptedResponse); + + // Check: impossible to release responses before the request is finalized + vm.expectRevert(IBondedResponseModule.BondedResponseModule_InvalidReleaseParameters.selector); + _responseModule.releaseUnutilizedResponse(mockRequest, _unutilizedResponse); + + // Finalizing the request with a correct response + _finalizeRequest(_acceptedResponse); + + // Check: impossible to release utilized responses + vm.expectRevert(IBondedResponseModule.BondedResponseModule_InvalidReleaseParameters.selector); + _responseModule.releaseUnutilizedResponse(mockRequest, _acceptedResponse); + + // Check: is request finalized with an empty response? + assertEq(oracle.finalizedResponseId(_requestId), _getId(_acceptedResponse)); + assertEq(oracle.finalizedAt(_requestId), block.timestamp); + + // Check: proposers funds are still bonded? + assertEq(_accountingExtension.bondedAmountOf(proposer, usdc, _requestId), _expectedBondSize, 'Balance check 1'); + + // The dispute is escalated and has no resolution + _escalateAndResolveDispute(IOracle.DisputeStatus.Lost); + + // Check: proposers funds are still bonded? + assertEq(_accountingExtension.bondedAmountOf(proposer, usdc, _requestId), _expectedBondSize, 'Balance check 2'); + + // Now the proposer should be able to release their unused response + vm.prank(proposer); + _responseModule.releaseUnutilizedResponse(mockRequest, _unutilizedResponse); + + // Check: proposers funds are not bonded anymore? + assertEq(_accountingExtension.bondedAmountOf(proposer, usdc, _requestId), 0, 'Balance check 3'); + + // Check: proposer received + // - their own bonds for 2 responses + // - disputer's bond for winning the dispute + // - the reward for the correct response + assertEq(_accountingExtension.balanceOf(proposer, usdc), _expectedBondSize * 3 + _expectedReward); + } + + /** + * @notice Tests reverts that happen when trying to release a response that is being or that has been successfully disputed + */ + function test_releaseUnutilizedResponse_withDisputedResponse() public { + mockResponse.requestId = bytes32(0); + IOracle.Response memory _emptyResponse = mockResponse; + mockResponse.requestId = _requestId; + + // Finalizing the request with an empty response + _finalizeRequest(_emptyResponse); + + // Check: is request finalized with an empty response? + assertEq(oracle.finalizedResponseId(_requestId), bytes32(0)); + assertEq(oracle.finalizedAt(_requestId), block.timestamp); + + // Check: impossible to release the disputed response while the dispute is active + vm.expectRevert(IBondedResponseModule.BondedResponseModule_InvalidReleaseParameters.selector); + _responseModule.releaseUnutilizedResponse(mockRequest, mockResponse); + + // The dispute is escalated and has no resolution + _escalateAndResolveDispute(IOracle.DisputeStatus.Won); + + // Check: impossible to release the disputed response if the disputer won + vm.expectRevert(IBondedResponseModule.BondedResponseModule_InvalidReleaseParameters.selector); + _responseModule.releaseUnutilizedResponse(mockRequest, mockResponse); + + // Check: proposers bond is slashed + assertEq(_accountingExtension.bondedAmountOf(proposer, usdc, _requestId), 0); + } + + function _escalateAndResolveDispute(IOracle.DisputeStatus _status) internal { + // Escalate dispute + vm.prank(disputer); + oracle.escalateDispute(mockRequest, mockResponse, mockDispute, _createAccessControl()); + + // Resolve with the provided status + vm.mockCall(address(_mockArbitrator), abi.encodeCall(IArbitrator.getAnswer, (_disputeId)), abi.encode(_status)); + + vm.prank(disputer); + oracle.resolveDispute(mockRequest, mockResponse, mockDispute, _createAccessControl()); + } + + function _finalizeRequest(IOracle.Response memory _response) internal { + // After the deadline has passed, finalize with the given response + vm.warp(block.timestamp + _expectedDeadline); + vm.prank(_finalizer); + oracle.finalize(mockRequest, _response, _createAccessControl()); + } + + function _proposeResponse(IOracle.Response memory _response) internal returns (bytes32 _responseId) { + vm.startPrank(proposer); + _accountingExtension.approveModule(address(_responseModule)); + _responseId = oracle.proposeResponse(mockRequest, _response, _createAccessControl()); + vm.stopPrank(); + } +} diff --git a/solidity/test/unit/modules/response/BondedResponseModule.t.sol b/solidity/test/unit/modules/response/BondedResponseModule.t.sol index 5e65732..8a9eb12 100644 --- a/solidity/test/unit/modules/response/BondedResponseModule.t.sol +++ b/solidity/test/unit/modules/response/BondedResponseModule.t.sol @@ -515,7 +515,8 @@ contract BondedResponseModule_Unit_ReleaseUnutilizedResponse is BaseTest { IERC20 _token, uint256 _bondSize, uint256 _deadline, - bytes32 _finalizedResponseId + bytes32 _finalizedResponseId, + uint256 _finalizedAt ) public { // Setting the response module data mockRequest.responseModuleData = abi.encode(accounting, _token, _bondSize, _deadline, _baseDisputeWindow); @@ -529,6 +530,7 @@ contract BondedResponseModule_Unit_ReleaseUnutilizedResponse is BaseTest { // Can't claim back the bond of the response that was finalized vm.assume(_finalizedResponseId > 0); vm.assume(_finalizedResponseId != _responseId); + vm.assume(_finalizedAt > 0); // Mock and expect IOracle.disputeOf to be called _mockAndExpect(address(oracle), abi.encodeCall(IOracle.disputeOf, (_responseId)), abi.encode(bytes32(0))); @@ -538,6 +540,8 @@ contract BondedResponseModule_Unit_ReleaseUnutilizedResponse is BaseTest { address(oracle), abi.encodeCall(IOracle.finalizedResponseId, (_requestId)), abi.encode(_finalizedResponseId) ); + _mockAndExpect(address(oracle), abi.encodeCall(IOracle.finalizedAt, (_requestId)), abi.encode(_finalizedAt)); + _mockAndExpect( address(oracle), abi.encodeCall(IOracle.responseCreatedAt, (_responseId)), abi.encode(block.timestamp) ); @@ -580,6 +584,8 @@ contract BondedResponseModule_Unit_ReleaseUnutilizedResponse is BaseTest { // Mock and expect IOracle.finalizedResponseId to be called _mockAndExpect(address(oracle), abi.encodeCall(IOracle.finalizedResponseId, (_requestId)), abi.encode(0)); + _mockAndExpect(address(oracle), abi.encodeCall(IOracle.finalizedAt, (_requestId)), abi.encode(0)); + _mockAndExpect( address(oracle), abi.encodeCall(IOracle.responseCreatedAt, (_responseId)), abi.encode(block.timestamp) ); @@ -599,7 +605,8 @@ contract BondedResponseModule_Unit_ReleaseUnutilizedResponse is BaseTest { uint256 _deadline, address _proposer, bytes32 _finalizedResponseId, - bytes32 _disputeId + bytes32 _disputeId, + uint256 _finalizedAt ) public { // Setting the response module data mockRequest.responseModuleData = abi.encode(accounting, _token, _bondSize, _deadline, _baseDisputeWindow); @@ -615,6 +622,7 @@ contract BondedResponseModule_Unit_ReleaseUnutilizedResponse is BaseTest { // Can't claim back the bond of the response that was finalized vm.assume(_finalizedResponseId > 0); vm.assume(_finalizedResponseId != _responseId); + vm.assume(_finalizedAt > 0); // Mock and expect IOracle.disputeOf to be called _mockAndExpect(address(oracle), abi.encodeCall(IOracle.disputeOf, (_responseId)), abi.encode(_disputeId)); @@ -624,6 +632,8 @@ contract BondedResponseModule_Unit_ReleaseUnutilizedResponse is BaseTest { address(oracle), abi.encodeCall(IOracle.finalizedResponseId, (_requestId)), abi.encode(_finalizedResponseId) ); + _mockAndExpect(address(oracle), abi.encodeCall(IOracle.finalizedAt, (_requestId)), abi.encode(_finalizedAt)); + _mockAndExpect( address(oracle), abi.encodeCall(IOracle.responseCreatedAt, (_responseId)), abi.encode(block.timestamp) );