diff --git a/docs/src/content/modules/finality/callback_module.md b/docs/src/content/modules/finality/callback_module.md index e2c1ce9c..78409c9a 100644 --- a/docs/src/content/modules/finality/callback_module.md +++ b/docs/src/content/modules/finality/callback_module.md @@ -10,8 +10,8 @@ The Callback Module is a finality module that allows users to call a function on ### Key Methods -- `decodeRequestData(bytes32 _requestId)`: Returns the decoded data for a request. -- `finalizeRequest(bytes32 _requestId, address)`: Executing the callback call on the target. +- `decodeRequestData`: Returns the decoded data for a request. +- `finalizeRequest`: Executing the callback call on the target. ### Request Parameters @@ -25,4 +25,3 @@ As any finality module, the `CallbackModule` implements the `finalizeRequest` fu ## 4. Gotchas - The success of the callback call in `finalizeRequest` is purposely not checked, specifying a function or parameters that lead to a revert will not stop the request from being finalized. -- The target must be a contract. diff --git a/solidity/contracts/modules/finality/CallbackModule.sol b/solidity/contracts/modules/finality/CallbackModule.sol index 3f3faf9d..f3eea3d9 100644 --- a/solidity/contracts/modules/finality/CallbackModule.sol +++ b/solidity/contracts/modules/finality/CallbackModule.sol @@ -1,42 +1,34 @@ -// // SPDX-License-Identifier: MIT -// pragma solidity ^0.8.19; +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; -// // solhint-disable-next-line no-unused-import -// import {Module, IModule} from '@defi-wonderland/prophet-core-contracts/solidity/contracts/Module.sol'; -// import {IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/IOracle.sol'; +// solhint-disable-next-line no-unused-import +import {Module, IModule} from '@defi-wonderland/prophet-core-contracts/solidity/contracts/Module.sol'; +import {IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/IOracle.sol'; -// import {ICallbackModule} from '../../../interfaces/modules/finality/ICallbackModule.sol'; +import {ICallbackModule} from '../../../interfaces/modules/finality/ICallbackModule.sol'; -// contract CallbackModule is Module, ICallbackModule { -// constructor(IOracle _oracle) Module(_oracle) {} +contract CallbackModule is Module, ICallbackModule { + constructor(IOracle _oracle) Module(_oracle) {} -// /// @inheritdoc IModule -// function moduleName() public pure returns (string memory _moduleName) { -// _moduleName = 'CallbackModule'; -// } + /// @inheritdoc IModule + function moduleName() public pure returns (string memory _moduleName) { + _moduleName = 'CallbackModule'; + } -// /// @inheritdoc ICallbackModule -// function decodeRequestData(bytes32 _requestId) public view returns (RequestParameters memory _params) { -// _params = abi.decode(requestData[_requestId], (RequestParameters)); -// } + /// @inheritdoc ICallbackModule + function decodeRequestData(bytes calldata _data) public pure returns (RequestParameters memory _params) { + _params = abi.decode(_data, (RequestParameters)); + } -// /** -// * @notice Checks if the target address has code (i.e. is a contract) -// * @param _data The encoded data for the request -// */ -// function _afterSetupRequest(bytes32, bytes calldata _data) internal view override { -// RequestParameters memory _params = abi.decode(_data, (RequestParameters)); -// if (_params.target.code.length == 0) revert CallbackModule_TargetHasNoCode(); -// } - -// /// @inheritdoc ICallbackModule -// function finalizeRequest( -// bytes32 _requestId, -// address _finalizer -// ) external override(Module, ICallbackModule) onlyOracle { -// RequestParameters memory _params = decodeRequestData(_requestId); -// _params.target.call(_params.data); -// emit Callback(_requestId, _params.target, _params.data); -// emit RequestFinalized(_requestId, _finalizer); -// } -// } + /// @inheritdoc ICallbackModule + function finalizeRequest( + IOracle.Request calldata _request, + IOracle.Response calldata _response, + address _finalizer + ) external override(Module, ICallbackModule) onlyOracle { + RequestParameters memory _params = decodeRequestData(_request.finalityModuleData); + _params.target.call(_params.data); + emit Callback(_response.requestId, _params.target, _params.data); + emit RequestFinalized(_response.requestId, _response, _finalizer); + } +} diff --git a/solidity/interfaces/modules/finality/ICallbackModule.sol b/solidity/interfaces/modules/finality/ICallbackModule.sol index 99b298ac..d5cb064f 100644 --- a/solidity/interfaces/modules/finality/ICallbackModule.sol +++ b/solidity/interfaces/modules/finality/ICallbackModule.sol @@ -1,65 +1,63 @@ -// // SPDX-License-Identifier: MIT -// pragma solidity ^0.8.19; - -// import {IFinalityModule} from -// '@defi-wonderland/prophet-core-contracts/solidity/interfaces/modules/finality/IFinalityModule.sol'; - -// /** -// * @title CallbackModule -// * @notice Module allowing users to call a function on a contract -// * as a result of a request being finalized. -// */ -// interface ICallbackModule is IFinalityModule { -// /*/////////////////////////////////////////////////////////////// -// EVENTS -// //////////////////////////////////////////////////////////////*/ - -// /** -// * @notice A callback has been executed -// * @param _requestId The id of the request being finalized -// * @param _target The target address for the callback -// * @param _data The calldata forwarded to the _target -// */ -// event Callback(bytes32 indexed _requestId, address indexed _target, bytes _data); - -// /*/////////////////////////////////////////////////////////////// -// ERRORS -// //////////////////////////////////////////////////////////////*/ - -// /** -// * @notice Thrown when the target address has no code (i.e. is not a contract) -// */ -// error CallbackModule_TargetHasNoCode(); - -// /*/////////////////////////////////////////////////////////////// -// STRUCTS -// //////////////////////////////////////////////////////////////*/ - -// /** -// * @notice Parameters of the request as stored in the module -// * @param target The target address for the callback -// * @param data The calldata forwarded to the _target -// */ -// struct RequestParameters { -// address target; -// bytes data; -// } - -// /*/////////////////////////////////////////////////////////////// -// LOGIC -// //////////////////////////////////////////////////////////////*/ - -// /** -// * @notice Returns the decoded data for a request -// * @param _requestId The ID of the request -// * @return _params The struct containing the parameters for the request -// */ -// function decodeRequestData(bytes32 _requestId) external view returns (RequestParameters memory _params); - -// /** -// * @notice Finalizes the request by executing the callback call on the target -// * @dev The success of the callback call is purposely not checked -// * @param _requestId The id of the request -// */ -// function finalizeRequest(bytes32 _requestId, address) external; -// } +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +import {IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/IOracle.sol'; +import {IFinalityModule} from + '@defi-wonderland/prophet-core-contracts/solidity/interfaces/modules/finality/IFinalityModule.sol'; + +/** + * @title CallbackModule + * @notice Module allowing users to call a function on a contract + * as a result of a request being finalized. + */ +interface ICallbackModule is IFinalityModule { + /*/////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + /** + * @notice A callback has been executed + * @param _requestId The id of the request being finalized + * @param _target The target address for the callback + * @param _data The calldata forwarded to the _target + */ + event Callback(bytes32 indexed _requestId, address indexed _target, bytes _data); + + /*/////////////////////////////////////////////////////////////// + STRUCTS + //////////////////////////////////////////////////////////////*/ + + /** + * @notice Parameters of the request as stored in the module + * @param target The target address for the callback + * @param data The calldata forwarded to the _target + */ + struct RequestParameters { + address target; + bytes data; + } + + /*/////////////////////////////////////////////////////////////// + LOGIC + //////////////////////////////////////////////////////////////*/ + + /** + * @notice Returns the decoded data for a request + * @param _data The encoded request parameters + * @return _params The struct containing the parameters for the request + */ + function decodeRequestData(bytes calldata _data) external view returns (RequestParameters memory _params); + + /** + * @notice Finalizes the request by executing the callback call on the target + * @dev The success of the callback call is purposely not checked + * @param _request The request being finalized + * @param _response The final response + * @param _finalizer The address that initiated the finalization + */ + function finalizeRequest( + IOracle.Request calldata _request, + IOracle.Response calldata _response, + address _finalizer + ) external; +} diff --git a/solidity/test/unit/modules/finality/CallbackModule.t.sol b/solidity/test/unit/modules/finality/CallbackModule.t.sol index 77bf4c8a..6cbf2076 100644 --- a/solidity/test/unit/modules/finality/CallbackModule.t.sol +++ b/solidity/test/unit/modules/finality/CallbackModule.t.sol @@ -1,147 +1,113 @@ -// // SPDX-License-Identifier: AGPL-3.0-only -// pragma solidity ^0.8.19; - -// import 'forge-std/Test.sol'; - -// import {Helpers} from '../../../utils/Helpers.sol'; - -// import {IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/IOracle.sol'; -// import {IModule} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/IModule.sol'; - -// import {CallbackModule, ICallbackModule} from '../../../../contracts/modules/finality/CallbackModule.sol'; - -// /** -// * @dev Harness to set an entry in the requestData mapping, without triggering setup request hooks -// */ -// contract ForTest_CallbackModule is CallbackModule { -// constructor(IOracle _oracle) CallbackModule(_oracle) {} - -// function forTest_setRequestData(bytes32 _requestId, bytes memory _data) public { -// requestData[_requestId] = _data; -// } -// } - -// /** -// * @title Callback Module Unit tests -// */ -// contract BaseTest is Test, Helpers { -// // The target contract -// ForTest_CallbackModule public callbackModule; -// // A mock oracle -// IOracle public oracle; - -// event Callback(bytes32 indexed _request, address indexed _target, bytes _data); -// event RequestFinalized(bytes32 indexed _requestId, address _finalizer); - -// /** -// * @notice Deploy the target and mock oracle+accounting extension -// */ -// function setUp() public { -// oracle = IOracle(makeAddr('Oracle')); -// vm.etch(address(oracle), hex'069420'); - -// callbackModule = new ForTest_CallbackModule(oracle); -// } -// } - -// contract CallbackModule_Unit_ModuleData is BaseTest { -// /** -// * @notice Test that the moduleName function returns the correct name -// */ -// function test_moduleNameReturnsName() public { -// assertEq(callbackModule.moduleName(), 'CallbackModule'); -// } - -// /** -// * @notice Test that the decodeRequestData function returns the correct values -// */ -// function test_decodeRequestData(bytes32 _requestId, address _target, bytes memory _data) public { -// // Create and set some mock request data -// bytes memory _requestData = abi.encode(ICallbackModule.RequestParameters({target: _target, data: _data})); -// callbackModule.forTest_setRequestData(_requestId, _requestData); - -// // Decode the given request data -// ICallbackModule.RequestParameters memory _params = callbackModule.decodeRequestData(_requestId); - -// // Check: decoded values match original values? -// assertEq(_params.target, _target); -// assertEq(_params.data, _data); -// } -// } - -// contract CallbackModule_Unit_FinalizeRequest is BaseTest { -// /** -// * @notice Test that finalizeRequest calls the _target.callback with the correct data -// */ -// function test_triggersCallback( -// bytes32 _requestId, -// address _target, -// bytes calldata _data -// ) public assumeFuzzable(_target) { -// vm.assume(_target != address(vm)); - -// // Create and set some mock request data -// bytes memory _requestData = abi.encode(ICallbackModule.RequestParameters({target: _target, data: _data})); -// callbackModule.forTest_setRequestData(_requestId, _requestData); - -// // Check: is the event emitted? -// vm.expectEmit(true, true, true, true, address(callbackModule)); -// emit Callback(_requestId, _target, _data); - -// vm.prank(address(oracle)); -// callbackModule.finalizeRequest(_requestId, address(oracle)); -// } - -// function test_emitsEvent(bytes32 _requestId, address _target, bytes calldata _data) public assumeFuzzable(_target) { -// vm.assume(_target != address(vm)); - -// // Create and set some mock request data -// bytes memory _requestData = abi.encode(ICallbackModule.RequestParameters({target: _target, data: _data})); -// callbackModule.forTest_setRequestData(_requestId, _requestData); - -// // Check: is the event emitted? -// vm.expectEmit(true, true, true, true, address(callbackModule)); -// emit RequestFinalized(_requestId, address(oracle)); - -// vm.prank(address(oracle)); -// callbackModule.finalizeRequest(_requestId, address(oracle)); -// } - -// /** -// * @notice Test that the finalizeRequest reverts if caller is not the oracle -// */ -// function test_revertsIfWrongCaller(bytes32 _requestId, address _caller) public { -// vm.assume(_caller != address(oracle)); - -// // Check: does it revert if not called by the Oracle? -// vm.expectRevert(abi.encodeWithSelector(IModule.Module_OnlyOracle.selector)); - -// vm.prank(_caller); -// callbackModule.finalizeRequest(_requestId, address(_caller)); -// } -// } - -// contract CallbackModule_Unit_Setup is BaseTest { -// /** -// * @notice Test that _afterSetupRequest checks if the target address is a contract. -// */ -// function test_revertIfTargetNotContract( -// bytes32 _requestId, -// address _target, -// bool _hasCode, -// bytes calldata _data -// ) public assumeFuzzable(_target) { -// vm.assume(_target.code.length == 0); -// bytes memory _requestData = abi.encode(ICallbackModule.RequestParameters({target: _target, data: _data})); - -// if (_hasCode) { -// vm.etch(_target, hex'069420'); -// } else { -// // Check: does it revert if the target has no code? -// vm.expectRevert(ICallbackModule.CallbackModule_TargetHasNoCode.selector); -// } - -// vm.prank(address(oracle)); -// callbackModule.setupRequest(_requestId, _requestData); -// } -// } +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity ^0.8.19; + +import 'forge-std/Test.sol'; + +import {Helpers} from '../../../utils/Helpers.sol'; + +import {IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/IOracle.sol'; +import {IModule} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/IModule.sol'; + +import {CallbackModule, ICallbackModule} from '../../../../contracts/modules/finality/CallbackModule.sol'; + +/** + * @title Callback Module Unit tests + */ +contract BaseTest is Test, Helpers { + // The target contract + CallbackModule public callbackModule; + // A mock oracle + IOracle public oracle; + + // Events + event Callback(bytes32 indexed _request, address indexed _target, bytes _data); + + /** + * @notice Deploy the target and mock oracle+accounting extension + */ + function setUp() public { + oracle = IOracle(makeAddr('Oracle')); + vm.etch(address(oracle), hex'069420'); + + callbackModule = new CallbackModule(oracle); + } +} + +contract CallbackModule_Unit_ModuleData is BaseTest { + /** + * @notice Test that the moduleName function returns the correct name + */ + function test_moduleNameReturnsName() public { + assertEq(callbackModule.moduleName(), 'CallbackModule'); + } + + /** + * @notice Test that the decodeRequestData function returns the correct values + */ + function test_decodeRequestData(address _target, bytes memory _data) public { + // Create and set some mock request data + bytes memory _requestData = abi.encode(ICallbackModule.RequestParameters({target: _target, data: _data})); + + // Decode the given request data + ICallbackModule.RequestParameters memory _params = callbackModule.decodeRequestData(_requestData); + + // Check: decoded values match original values? + assertEq(_params.target, _target); + assertEq(_params.data, _data); + } +} + +contract CallbackModule_Unit_FinalizeRequest is BaseTest { + /** + * @notice Test that finalizeRequest emits events + */ + function test_emitsEvents(address _proposer, address _target, bytes calldata _data) public assumeFuzzable(_target) { + mockRequest.finalityModuleData = abi.encode(ICallbackModule.RequestParameters({target: _target, data: _data})); + mockResponse.requestId = _getId(mockRequest); + + // Check: is the event emitted? + vm.expectEmit(true, true, true, true, address(callbackModule)); + emit Callback(mockResponse.requestId, _target, _data); + + // Check: is the event emitted? + vm.expectEmit(true, true, true, true, address(callbackModule)); + emit RequestFinalized(mockResponse.requestId, mockResponse, _proposer); + + vm.prank(address(oracle)); + callbackModule.finalizeRequest(mockRequest, mockResponse, _proposer); + } + + /** + * @notice Test that finalizeRequest triggers the callback + */ + function test_triggersCallback( + address _proposer, + address _target, + bytes calldata _data + ) public assumeFuzzable(_target) { + mockRequest.finalityModuleData = abi.encode(ICallbackModule.RequestParameters({target: _target, data: _data})); + mockResponse.requestId = _getId(mockRequest); + + // Mock and expect the callback + _mockAndExpect(_target, _data, abi.encode('')); + + vm.prank(address(oracle)); + callbackModule.finalizeRequest(mockRequest, mockResponse, _proposer); + } + + /** + * @notice Test that the finalizeRequest reverts if caller is not the oracle + */ + function test_revertsIfWrongCaller(ICallbackModule.RequestParameters calldata _data, address _caller) public { + vm.assume(_caller != address(oracle)); + + mockRequest.finalityModuleData = abi.encode(_data); + mockResponse.requestId = _getId(mockRequest); + + // Check: does it revert if not called by the Oracle? + vm.expectRevert(abi.encodeWithSelector(IModule.Module_OnlyOracle.selector)); + + vm.prank(_caller); + callbackModule.finalizeRequest(mockRequest, mockResponse, _caller); + } +} diff --git a/solidity/test/utils/Helpers.sol b/solidity/test/utils/Helpers.sol index 213a0064..4f22491b 100644 --- a/solidity/test/utils/Helpers.sol +++ b/solidity/test/utils/Helpers.sol @@ -53,7 +53,6 @@ contract Helpers is DSTestPlus, TestConstants { vm.expectCall(_receiver, _calldata); } - // TODO: What does _balanceIncrease do? function _forBondDepositERC20( IAccountingExtension _accountingExtension, address _depositor, diff --git a/yarn.lock b/yarn.lock index 18287d95..4e936a9c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -274,9 +274,9 @@ antlr4ts "^0.5.0-alpha.4" "@solidity-parser/parser@^0.16.0": - version "0.16.1" - resolved "https://registry.yarnpkg.com/@solidity-parser/parser/-/parser-0.16.1.tgz#f7c8a686974e1536da0105466c4db6727311253c" - integrity sha512-PdhRFNhbTtu3x8Axm0uYpqOy/lODYQK+MlYSgqIsq2L8SFYEHJPHNUiOTAJbDGzNjjr1/n9AcIayxafR/fWmYw== + version "0.16.2" + resolved "https://registry.yarnpkg.com/@solidity-parser/parser/-/parser-0.16.2.tgz#42cb1e3d88b3e8029b0c9befff00b634cd92d2fa" + integrity sha512-PI9NfoA3P8XK2VBkK5oIfRgKDsicwDZfkVq9ZTBCQYGOP1N2owgY2dyLGyU5/J/hQs8KRk55kdmvTLjy3Mu3vg== dependencies: antlr4ts "^0.5.0-alpha.4"