From 3f4d2c57beefde31b5d6cee9769e2acc108f72b8 Mon Sep 17 00:00:00 2001 From: Gas One Cent <86567384+gas1cent@users.noreply.github.com> Date: Wed, 1 Nov 2023 14:58:02 +0400 Subject: [PATCH] perf: optimize the `BondedResponseModule` --- .../modules/response/BondedResponseModule.sol | 66 +++++------------- .../response/IBondedResponseModule.sol | 63 +++++++---------- .../test/integration/ResponseProposal.t.sol | 36 ---------- .../response/BondedResponseModule.t.sol | 69 +------------------ 4 files changed, 45 insertions(+), 189 deletions(-) diff --git a/solidity/contracts/modules/response/BondedResponseModule.sol b/solidity/contracts/modules/response/BondedResponseModule.sol index b8567c96..60f627af 100644 --- a/solidity/contracts/modules/response/BondedResponseModule.sol +++ b/solidity/contracts/modules/response/BondedResponseModule.sol @@ -15,84 +15,61 @@ contract BondedResponseModule is Module, IBondedResponseModule { } /// @inheritdoc IBondedResponseModule - function decodeRequestData(bytes32 _requestId) public view returns (RequestParameters memory _params) { - _params = abi.decode(requestData[_requestId], (RequestParameters)); + function decodeRequestData(bytes calldata _data) public pure returns (RequestParameters memory _params) { + _params = abi.decode(_data, (RequestParameters)); } /// @inheritdoc IBondedResponseModule function propose( - bytes32 _requestId, - address _proposer, - bytes calldata _responseData, + IOracle.Request calldata _request, + IOracle.Response calldata _response, address _sender - ) external onlyOracle returns (IOracle.Response memory _response) { - RequestParameters memory _params = decodeRequestData(_requestId); + ) external onlyOracle { + RequestParameters memory _params = decodeRequestData(_request.responseModuleData); // Cannot propose after the deadline if (block.timestamp >= _params.deadline) revert BondedResponseModule_TooLateToPropose(); // Cannot propose to a request with a response, unless the response is being disputed - bytes32[] memory _responseIds = ORACLE.getResponseIds(_requestId); + bytes32[] memory _responseIds = ORACLE.getResponseIds(_response.requestId); uint256 _responsesLength = _responseIds.length; if (_responsesLength != 0) { - bytes32 _disputeId = ORACLE.getResponse(_responseIds[_responsesLength - 1]).disputeId; + bytes32 _disputeId = ORACLE.disputeOf(_responseIds[_responsesLength - 1]); // Allowing one undisputed response at a time if (_disputeId == bytes32(0)) revert BondedResponseModule_AlreadyResponded(); - IOracle.Dispute memory _dispute = ORACLE.getDispute(_disputeId); + IOracle.DisputeStatus _status = ORACLE.disputeStatus(_disputeId); // TODO: leaving a note here to re-check this check if a new status is added // If the dispute was lost, we assume the proposed answer was correct. DisputeStatus.None should not be reachable due to the previous check. - if (_dispute.status == IOracle.DisputeStatus.Lost) revert BondedResponseModule_AlreadyResponded(); + if (_status == IOracle.DisputeStatus.Lost) revert BondedResponseModule_AlreadyResponded(); } - _response = IOracle.Response({ - requestId: _requestId, - disputeId: bytes32(0), - proposer: _proposer, - response: _responseData, - createdAt: block.timestamp - }); - _params.accountingExtension.bond({ _bonder: _response.proposer, - _requestId: _requestId, + _requestId: _response.requestId, _token: _params.bondToken, _amount: _params.bondSize, _sender: _sender }); - emit ProposeResponse(_requestId, _proposer, _responseData); - } - - /// @inheritdoc IBondedResponseModule - function deleteResponse(bytes32 _requestId, bytes32, address _proposer) external onlyOracle { - RequestParameters memory _params = decodeRequestData(_requestId); - - if (block.timestamp > _params.deadline) revert BondedResponseModule_TooLateToDelete(); - - _params.accountingExtension.release({ - _bonder: _proposer, - _requestId: _requestId, - _token: _params.bondToken, - _amount: _params.bondSize - }); + emit ResponseProposed(_response.requestId, _response, block.number); } /// @inheritdoc IBondedResponseModule function finalizeRequest( - bytes32 _requestId, + IOracle.Request calldata _request, + IOracle.Response calldata _response, address _finalizer ) external override(IBondedResponseModule, Module) onlyOracle { - RequestParameters memory _params = decodeRequestData(_requestId); + RequestParameters memory _params = decodeRequestData(_response.requestId); - bool _isModule = ORACLE.allowedModule(_requestId, _finalizer); + bool _isModule = ORACLE.allowedModule(_response.requestId, _finalizer); if (!_isModule && block.timestamp < _params.deadline) { revert BondedResponseModule_TooEarlyToFinalize(); } - IOracle.Response memory _response = ORACLE.getFinalizedResponse(_requestId); if (_response.createdAt != 0) { if (!_isModule && block.timestamp < _response.createdAt + _params.disputeWindow) { revert BondedResponseModule_TooEarlyToFinalize(); @@ -100,19 +77,12 @@ contract BondedResponseModule is Module, IBondedResponseModule { _params.accountingExtension.release({ _bonder: _response.proposer, - _requestId: _requestId, + _requestId: _response.requestId, _token: _params.bondToken, _amount: _params.bondSize }); } - emit RequestFinalized(_requestId, _finalizer); - } - /// @inheritdoc Module - function _afterSetupRequest(bytes32, bytes calldata _data) internal view override { - RequestParameters memory _params = abi.decode(_data, (RequestParameters)); - if (_params.deadline <= block.timestamp) { - revert BondedResponseModule_InvalidRequest(); - } + emit RequestFinalized(_response.requestId, _response, _finalizer); } } diff --git a/solidity/interfaces/modules/response/IBondedResponseModule.sol b/solidity/interfaces/modules/response/IBondedResponseModule.sol index 5ddcdc51..1b460221 100644 --- a/solidity/interfaces/modules/response/IBondedResponseModule.sol +++ b/solidity/interfaces/modules/response/IBondedResponseModule.sol @@ -10,20 +10,21 @@ import {IAccountingExtension} from '../../extensions/IAccountingExtension.sol'; /* * @title BondedResponseModule - * @notice Module allowing users to propose a response for a request - * by bonding tokens. + * @notice Module allowing users to propose a response for a request by bonding tokens */ interface IBondedResponseModule is IResponseModule { /*/////////////////////////////////////////////////////////////// - ERRORS + EVENTS //////////////////////////////////////////////////////////////*/ /** * @notice Emitted when a response is proposed + * * @param _requestId The ID of the request that the response was proposed - * @param _proposer The user that proposed the response - * @param _responseData The data for the response + * @param _response The proposed response + * @param _blockNumber The number of the block in which the response was proposed */ - event ProposeResponse(bytes32 indexed _requestId, address _proposer, bytes _responseData); + event ResponseProposed(bytes32 indexed _requestId, IOracle.Response _response, uint256 indexed _blockNumber); + /*/////////////////////////////////////////////////////////////// ERRORS //////////////////////////////////////////////////////////////*/ @@ -43,22 +44,13 @@ interface IBondedResponseModule is IResponseModule { */ error BondedResponseModule_AlreadyResponded(); - /** - * @notice Thrown when trying to delete a response after the proposing deadline - */ - error BondedResponseModule_TooLateToDelete(); - - /** - * @notice Thrown when trying to create an invalid request - */ - error BondedResponseModule_InvalidRequest(); - /*/////////////////////////////////////////////////////////////// STRUCTS //////////////////////////////////////////////////////////////*/ /** * @notice Parameters of the request as stored in the module + * * @param accountingExtension The accounting extension used to bond and release tokens * @param bondToken The token used for bonds in the request * @param bondSize The amount of `_bondToken` to bond to propose a response and dispute @@ -79,39 +71,32 @@ interface IBondedResponseModule is IResponseModule { /** * @notice Returns the decoded data for a request - * @param _requestId The ID of the request + * + * @param _data The encoded data * @return _params The struct containing the parameters for the request */ - function decodeRequestData(bytes32 _requestId) external view returns (RequestParameters memory _params); + function decodeRequestData(bytes calldata _data) external pure returns (RequestParameters memory _params); /** * @notice Proposes a response for a request, bonding the proposer's tokens + * * @dev The user must have previously deposited tokens into the accounting extension - * @param _requestId The ID of the request to propose a response for - * @param _proposer The user proposing the response - * @param _responseData The data for the response - * @param _sender The address calling propose on the Oracle - * @return _response The struct of proposed response - */ - function propose( - bytes32 _requestId, - address _proposer, - bytes calldata _responseData, - address _sender - ) external returns (IOracle.Response memory _response); - - /** - * @notice Allows a user to delete an undisputed response they proposed before the deadline, releasing the bond - * @param _requestId The ID of the request to delete the response from - * @param _responseId The ID of the response to delete - * @param _proposer The user who proposed the response + * @param _request The request to propose a response to + * @param _response The response being proposed + * @param _sender The address that initiated the transaction */ - function deleteResponse(bytes32 _requestId, bytes32 _responseId, address _proposer) external; + function propose(IOracle.Request calldata _request, IOracle.Response calldata _response, address _sender) external; /** * @notice Finalizes the request by releasing the bond of the proposer - * @param _requestId The ID of the request to finalize + * + * @param _request The request that is being finalized + * @param _response The final response * @param _finalizer The user who triggered the finalization */ - function finalizeRequest(bytes32 _requestId, address _finalizer) external; + function finalizeRequest( + IOracle.Request calldata _request, + IOracle.Response calldata _response, + address _finalizer + ) external; } diff --git a/solidity/test/integration/ResponseProposal.t.sol b/solidity/test/integration/ResponseProposal.t.sol index f329ff57..30feff8a 100644 --- a/solidity/test/integration/ResponseProposal.t.sol +++ b/solidity/test/integration/ResponseProposal.t.sol @@ -149,40 +149,4 @@ contract Integration_ResponseProposal is IntegrationBase { assertEq(_deletedResponse.createdAt, 0); assertEq(_deletedResponse.disputeId, bytes32(0)); } - - function test_deleteResponse_afterDeadline(bytes memory _responseData, uint256 _timestamp) public { - vm.assume(_timestamp > _expectedDeadline); - - _forBondDepositERC20(_accountingExtension, proposer, usdc, _expectedBondSize, _expectedBondSize); - - vm.startPrank(proposer); - _accountingExtension.approveModule(address(_responseModule)); - bytes32 _responseId = oracle.proposeResponse(_requestId, _responseData); - vm.stopPrank(); - - vm.warp(_timestamp); - - vm.expectRevert(IBondedResponseModule.BondedResponseModule_TooLateToDelete.selector); - - vm.prank(proposer); - oracle.deleteResponse(_responseId); - } - - function test_proposeResponse_finalizedRequest(bytes memory _responseData, uint256 _timestamp) public { - vm.assume(_timestamp > _expectedDeadline + _baseDisputeWindow); - - _forBondDepositERC20(_accountingExtension, proposer, usdc, _expectedBondSize, _expectedBondSize); - - vm.startPrank(proposer); - _accountingExtension.approveModule(address(_responseModule)); - bytes32 _responseId = oracle.proposeResponse(_requestId, _responseData); - vm.stopPrank(); - - vm.warp(_timestamp); - oracle.finalize(_requestId, _responseId); - - vm.expectRevert(abi.encodeWithSelector(IOracle.Oracle_AlreadyFinalized.selector, _requestId)); - vm.prank(proposer); - oracle.proposeResponse(_requestId, _responseData); - } } diff --git a/solidity/test/unit/modules/response/BondedResponseModule.t.sol b/solidity/test/unit/modules/response/BondedResponseModule.t.sol index de5ff122..5ed0754e 100644 --- a/solidity/test/unit/modules/response/BondedResponseModule.t.sol +++ b/solidity/test/unit/modules/response/BondedResponseModule.t.sol @@ -40,8 +40,8 @@ contract BaseTest is Test, Helpers { // Base dispute window uint256 internal _baseDisputeWindow = 12 hours; - event ProposeResponse(bytes32 indexed _requestId, address _proposer, bytes _responseData); - event RequestFinalized(bytes32 indexed _requestId, address _finalizer); + event ResponseProposed(bytes32 indexed _requestId, IOracle.Response _response, uint256 indexed _blockNumber); + event RequestFinalized(bytes32 indexed _requestId, IOracle.Request _response, address indexed _finalizer); /** * @notice Deploy the target and mock oracle+accounting extension @@ -93,31 +93,6 @@ contract BondedResponseModule_Unit_ModuleData is BaseTest { } } -contract BondedResponseModule_Unit_Setup is BaseTest { - function test_setupRequestRevertsIfInvalidRequest( - IAccountingExtension _accounting, - IERC20 _token, - uint256 _deadline, - uint256 _bondSize - ) public { - _deadline = bound(_deadline, 0, block.timestamp); - - IBondedResponseModule.RequestParameters memory _requestParams = IBondedResponseModule.RequestParameters({ - accountingExtension: _accounting, - bondToken: _token, - bondSize: _bondSize, - deadline: _deadline, - disputeWindow: _baseDisputeWindow - }); - - // Check: does it revert if the provided deadline is invalid? - vm.expectRevert(IBondedResponseModule.BondedResponseModule_InvalidRequest.selector); - vm.prank(address(oracle)); - - bondedResponseModule.setupRequest(bytes32(0), abi.encode(_requestParams)); - } -} - contract BondedResponseModule_Unit_Propose is BaseTest { /** * @notice Test that the propose function is only callable by the oracle @@ -220,7 +195,7 @@ contract BondedResponseModule_Unit_Propose is BaseTest { // Check: is the event emitted? vm.expectEmit(true, true, true, true, address(bondedResponseModule)); - emit ProposeResponse(_requestId, _proposer, _responseData); + emit ResponseProposed(_requestId, _proposer, _responseData); vm.prank(address(oracle)); bondedResponseModule.propose(_requestId, _proposer, _responseData, _sender); @@ -421,41 +396,3 @@ contract BondedResponseModule_Unit_FinalizeRequest is BaseTest { bondedResponseModule.finalizeRequest(_requestId, _finalizer); } } - -contract BondedResponseModule_Unit_DeleteResponse is BaseTest { - /** - * @notice Test that the delete response function triggers bond release. - */ - function test_deleteResponse( - bytes32 _requestId, - bytes32 _responseId, - uint256 _bondSize, - uint256 _deadline, - uint256 _timestamp, - IERC20 _token, - address _proposer - ) public { - _timestamp = bound(_timestamp, 1, type(uint248).max); - - // Create and set some mock request data - bytes memory _data = abi.encode(accounting, _token, _bondSize, _deadline, _baseDisputeWindow); - bondedResponseModule.forTest_setRequestData(_requestId, _data); - - vm.warp(_timestamp); - - if (_deadline >= _timestamp) { - // Mock and expect IAccountingExtension.release to be called - _mockAndExpect( - address(accounting), - abi.encodeCall(IAccountingExtension.release, (_proposer, _requestId, _token, _bondSize)), - abi.encode() - ); - } else { - // Check: does it revert if the deadline has passed? - vm.expectRevert(IBondedResponseModule.BondedResponseModule_TooLateToDelete.selector); - } - - vm.prank(address(oracle)); - bondedResponseModule.deleteResponse(_requestId, _responseId, _proposer); - } -}