diff --git a/docs/src/content/modules/finality/multiple_callbacks_module.md b/docs/src/content/modules/finality/multiple_callbacks_module.md index e9412209..1b4134a3 100644 --- a/docs/src/content/modules/finality/multiple_callbacks_module.md +++ b/docs/src/content/modules/finality/multiple_callbacks_module.md @@ -10,8 +10,8 @@ The `MultipleCallbacksModule` is a finality module that allows users to make mul ### Key Methods -- `decodeRequestData(bytes32 _requestId)`: Returns the decoded data for a request. The returned data includes the target addresses for the callback and the calldata forwarded to the targets. -- `finalizeRequest(bytes32 _requestId, address)`: Finalizes the request by executing the callback calls on the targets. +- `decodeRequestData`: Returns the decoded data for a request. The returned data includes the target addresses for the callback and the calldata forwarded to the targets. +- `finalizeRequest`: Finalizes the request by executing the callback calls on the targets. ### Request Parameters diff --git a/package.json b/package.json index dcb5ecad..d3eac0c9 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "package.json": "sort-package-json" }, "dependencies": { - "@defi-wonderland/prophet-core-contracts": "0.0.0-a1d2cc55", + "@defi-wonderland/prophet-core-contracts": "0.0.0-1ae08a81", "@defi-wonderland/solidity-utils": "0.0.0-3e9c8e8b", "@openzeppelin/contracts": "^4.9.3", "ds-test": "https://github.com/dapphub/ds-test.git#e282159d5170298eb2455a6c05280ab5a73a4ef0", diff --git a/solidity/contracts/modules/finality/MultipleCallbacksModule.sol b/solidity/contracts/modules/finality/MultipleCallbacksModule.sol index 77d8a311..b2406538 100644 --- a/solidity/contracts/modules/finality/MultipleCallbacksModule.sol +++ b/solidity/contracts/modules/finality/MultipleCallbacksModule.sol @@ -1,58 +1,42 @@ -// // 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'; - -// import {IMultipleCallbacksModule} from '../../../interfaces/modules/finality/IMultipleCallbacksModule.sol'; - -// contract MultipleCallbacksModule is Module, IMultipleCallbacksModule { -// constructor(IOracle _oracle) Module(_oracle) {} - -// /// @inheritdoc IMultipleCallbacksModule -// function decodeRequestData(bytes32 _requestId) public view returns (RequestParameters memory _params) { -// _params = abi.decode(requestData[_requestId], (RequestParameters)); -// } - -// /// @inheritdoc IModule -// function moduleName() public pure returns (string memory _moduleName) { -// _moduleName = 'MultipleCallbacksModule'; -// } - -// /** -// * @notice Checks if the target addresses have code and the calldata amount matches the targets amount -// * @param _data The ABI encoded address of the target contracts and the calldata to be executed -// */ -// function _afterSetupRequest(bytes32, bytes calldata _data) internal view override { -// RequestParameters memory _params = abi.decode(_data, (RequestParameters)); -// uint256 _length = _params.targets.length; -// if (_length != _params.data.length) revert MultipleCallbackModule_InvalidParameters(); - -// for (uint256 _i; _i < _length;) { -// if (_params.targets[_i].code.length == 0) revert MultipleCallbackModule_TargetHasNoCode(); -// unchecked { -// ++_i; -// } -// } -// } - -// /// @inheritdoc IMultipleCallbacksModule -// function finalizeRequest( -// bytes32 _requestId, -// address _finalizer -// ) external override(IMultipleCallbacksModule, Module) onlyOracle { -// RequestParameters memory _params = decodeRequestData(_requestId); -// uint256 _length = _params.targets.length; - -// for (uint256 _i; _i < _length;) { -// _params.targets[_i].call(_params.data[_i]); -// emit Callback(_requestId, _params.targets[_i], _params.data[_i]); -// unchecked { -// ++_i; -// } -// } - -// emit RequestFinalized(_requestId, _finalizer); -// } -// } +// 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'; + +import {IMultipleCallbacksModule} from '../../../interfaces/modules/finality/IMultipleCallbacksModule.sol'; + +contract MultipleCallbacksModule is Module, IMultipleCallbacksModule { + constructor(IOracle _oracle) Module(_oracle) {} + + /// @inheritdoc IMultipleCallbacksModule + function decodeRequestData(bytes calldata _data) public pure returns (RequestParameters memory _params) { + _params = abi.decode(_data, (RequestParameters)); + } + + /// @inheritdoc IModule + function moduleName() public pure returns (string memory _moduleName) { + _moduleName = 'MultipleCallbacksModule'; + } + + /// @inheritdoc IMultipleCallbacksModule + function finalizeRequest( + IOracle.Request calldata _request, + IOracle.Response calldata _response, + address _finalizer + ) external override(IMultipleCallbacksModule, Module) onlyOracle { + RequestParameters memory _params = decodeRequestData(_request.finalityModuleData); + uint256 _length = _params.targets.length; + + for (uint256 _i; _i < _length;) { + _params.targets[_i].call(_params.data[_i]); + emit Callback(_response.requestId, _params.targets[_i], _params.data[_i]); + unchecked { + ++_i; + } + } + + emit RequestFinalized(_response.requestId, _response, _finalizer); + } +} diff --git a/solidity/interfaces/modules/finality/IMultipleCallbacksModule.sol b/solidity/interfaces/modules/finality/IMultipleCallbacksModule.sol index f6de01cc..ea4546b3 100644 --- a/solidity/interfaces/modules/finality/IMultipleCallbacksModule.sol +++ b/solidity/interfaces/modules/finality/IMultipleCallbacksModule.sol @@ -1,69 +1,65 @@ -// // SPDX-License-Identifier: MIT -// pragma solidity ^0.8.19; +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; -// import {IFinalityModule} from -// '@defi-wonderland/prophet-core-contracts/solidity/interfaces/modules/finality/IFinalityModule.sol'; +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 MultipleCallbackModule -// * @notice Module allowing users to make multiple calls to different contracts -// * as a result of a request being finalized. -// */ -// interface IMultipleCallbacksModule is IFinalityModule { -// /*/////////////////////////////////////////////////////////////// -// EVENTS -// //////////////////////////////////////////////////////////////*/ +/** + * @title MultipleCallbackModule + * @notice Module allowing users to make multiple calls to different contracts + * as a result of a request being finalized. + */ +interface IMultipleCallbacksModule 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); + /** + * @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 -// //////////////////////////////////////////////////////////////*/ + /*/////////////////////////////////////////////////////////////// + STRUCTS + //////////////////////////////////////////////////////////////*/ -// /** -// * @notice Thrown when then target address has no code (i.e. is not a contract) -// */ -// error MultipleCallbackModule_TargetHasNoCode(); + /** + * @notice Parameters of the request as stored in the module + * @param targets The target addresses for the callback + * @param data The calldata forwarded to the targets + */ + struct RequestParameters { + address[] targets; + bytes[] data; + } -// /** -// * @notice Thrown when the targets array and the data array have different lengths -// */ -// error MultipleCallbackModule_InvalidParameters(); + /*/////////////////////////////////////////////////////////////// + LOGIC + //////////////////////////////////////////////////////////////*/ -// /*/////////////////////////////////////////////////////////////// -// STRUCTS -// //////////////////////////////////////////////////////////////*/ + /** + * @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 Parameters of the request as stored in the module -// * @param targets The target addresses for the callback -// * @param data The calldata forwarded to the targets -// */ -// struct RequestParameters { -// address[] targets; -// 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 calls on the targets -// * @dev The success of the callback calls is purposely not checked -// * @param _requestId The id of the request -// */ -// function finalizeRequest(bytes32 _requestId, address) external; -// } + /** + * @notice Finalizes the request by executing the callback calls on the targets + * + * @dev The success of the callback calls is purposely not checked + * @param _request The request being finalized + * @param _response The response + * @param _finalizer The address finalizing the request + */ + function finalizeRequest( + IOracle.Request calldata _request, + IOracle.Response calldata _response, + address _finalizer + ) external; +} diff --git a/solidity/test/unit/modules/dispute/CircuitResolverModule.t.sol b/solidity/test/unit/modules/dispute/CircuitResolverModule.t.sol index 5cc4878a..2af65544 100644 --- a/solidity/test/unit/modules/dispute/CircuitResolverModule.t.sol +++ b/solidity/test/unit/modules/dispute/CircuitResolverModule.t.sol @@ -40,7 +40,7 @@ contract BaseTest is Test, Helpers { MockVerifier public mockVerifier; // Events - event DisputeStatusChanged(bytes32 _disputeId, IOracle.Dispute _dispute, IOracle.DisputeStatus _status); + event DisputeStatusChanged(bytes32 indexed _disputeId, IOracle.Dispute _dispute, IOracle.DisputeStatus _status); event ResponseDisputed( bytes32 indexed _requestId, bytes32 indexed _responseId, diff --git a/solidity/test/unit/modules/finality/MultipleCallbacksModule.t.sol b/solidity/test/unit/modules/finality/MultipleCallbacksModule.t.sol index 3e888cf2..376cbe44 100644 --- a/solidity/test/unit/modules/finality/MultipleCallbacksModule.t.sol +++ b/solidity/test/unit/modules/finality/MultipleCallbacksModule.t.sol @@ -1,156 +1,99 @@ -// // 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 { -// MultipleCallbacksModule, -// IMultipleCallbacksModule -// } from '../../../../contracts/modules/finality/MultipleCallbacksModule.sol'; - -// /** -// * @dev Harness to set an entry in the requestData mapping, without triggering setup request hooks -// */ -// contract ForTest_MultipleCallbacksModule is MultipleCallbacksModule { -// constructor(IOracle _oracle) MultipleCallbacksModule(_oracle) {} - -// function forTest_setRequestData(bytes32 _requestId, address[] calldata _targets, bytes[] calldata _data) public { -// requestData[_requestId] = abi.encode(IMultipleCallbacksModule.RequestParameters({targets: _targets, data: _data})); -// } -// } - -// contract BaseTest is Test, Helpers { -// // The target contract -// ForTest_MultipleCallbacksModule public multipleCallbackModule; -// // 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'); - -// multipleCallbackModule = new ForTest_MultipleCallbacksModule(oracle); -// } -// } - -// /** -// * @title MultipleCallback Module Unit tests -// */ -// contract MultipleCallbacksModule_Unit_ModuleData is BaseTest { -// /** -// * @notice Test that the moduleName function returns the correct name -// */ -// function test_moduleNameReturnsName() public { -// assertEq(multipleCallbackModule.moduleName(), 'MultipleCallbacksModule'); -// } -// } - -// contract MultipleCallbacksModule_Unit_FinalizeRequests is BaseTest { -// /** -// * @notice Test that finalizeRequests calls the _target.callback with the correct data -// */ -// function test_finalizeRequest(bytes32 _requestId, address[1] calldata _targets, bytes[1] calldata __data) public { -// address _target = _targets[0]; -// bytes calldata _data = __data[0]; - -// assumeNotPrecompile(_target); -// vm.assume(_target != address(vm)); - -// // Create and set some mock request data -// address[] memory _targetParams = new address[](1); -// _targetParams[0] = _targets[0]; -// bytes[] memory _dataParams = new bytes[](1); -// _dataParams[0] = __data[0]; -// multipleCallbackModule.forTest_setRequestData(_requestId, _targetParams, _dataParams); - -// _mockAndExpect(_target, _data, abi.encode()); - -// // Check: is the event emitted? -// vm.expectEmit(true, true, true, true, address(multipleCallbackModule)); -// emit Callback(_requestId, _target, _data); - -// // Check: is the event emitted? -// vm.expectEmit(true, true, true, true, address(multipleCallbackModule)); -// emit RequestFinalized(_requestId, address(oracle)); - -// vm.prank(address(oracle)); -// multipleCallbackModule.finalizeRequest(_requestId, address(oracle)); -// } - -// /** -// * @notice Test that the finalizeRequests 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(IModule.Module_OnlyOracle.selector); -// vm.prank(_caller); -// multipleCallbackModule.finalizeRequest(_requestId, address(_caller)); -// } - -// function test_revertIfInvalidParameters(bytes32 _requestId, address[] memory _targets, bytes[] memory _data) public { -// vm.assume(_targets.length != _data.length); - -// bytes memory _requestData = abi.encode(IMultipleCallbacksModule.RequestParameters({targets: _targets, data: _data})); - -// // Check: does it revert if arrays length mismatch? -// vm.expectRevert(IMultipleCallbacksModule.MultipleCallbackModule_InvalidParameters.selector); -// vm.prank(address(oracle)); -// multipleCallbackModule.setupRequest(_requestId, _requestData); -// } -// } - -// contract MultipleCallbacksModule_Unit_Setup is BaseTest { -// function test_revertsIfInvalidTarget(bytes32 _requestId, address[] memory _targets, bytes memory _data) public { -// vm.assume(_targets.length > 1); - -// // Hardcoding data (as it is not the case tested) to avoid vm.assume issues -// bytes[] memory _targetData = new bytes[](_targets.length); -// for (uint256 _i = 0; _i < _targets.length; _i++) { -// _targetData[_i] = abi.encodeWithSelector(bytes4(keccak256('callback(bytes32,bytes)')), _requestId, _data); -// } - -// bytes memory _requestData = -// abi.encode(IMultipleCallbacksModule.RequestParameters({targets: _targets, data: _targetData})); - -// // Check: does it revert if the target has no code? -// vm.expectRevert(IMultipleCallbacksModule.MultipleCallbackModule_TargetHasNoCode.selector); -// vm.prank(address(oracle)); -// multipleCallbackModule.setupRequest(_requestId, _requestData); -// } - -// function test_setUpMultipleTargets(bytes32 _requestId, address[] memory _targets, bytes memory _data) public { -// vm.assume(_targets.length > 1); - -// // Hardcoding data (as it is not the case tested) to avoid vm.assume issues -// bytes[] memory _targetData = new bytes[](_targets.length); -// for (uint256 _i = 0; _i < _targets.length; _i++) { -// _assumeFuzzable(_targets[_i]); -// vm.etch(_targets[_i], hex'069420'); -// _targetData[_i] = abi.encodeWithSelector(bytes4(keccak256('callback(bytes32,bytes)')), _requestId, _data); -// } - -// bytes memory _requestData = -// abi.encode(IMultipleCallbacksModule.RequestParameters({targets: _targets, data: _targetData})); - -// vm.prank(address(oracle)); -// multipleCallbackModule.setupRequest(_requestId, _requestData); - -// IMultipleCallbacksModule.RequestParameters memory _storedParams = -// multipleCallbackModule.decodeRequestData(_requestId); -// // Check: is the data properly stored? -// assertEq(_storedParams.targets, _targets); -// } -// } +// 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 { + MultipleCallbacksModule, + IMultipleCallbacksModule +} from '../../../../contracts/modules/finality/MultipleCallbacksModule.sol'; + +contract BaseTest is Test, Helpers { + // The target contract + MultipleCallbacksModule public multipleCallbackModule; + // 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'); + + multipleCallbackModule = new MultipleCallbacksModule(oracle); + } +} + +/** + * @title MultipleCallback Module Unit tests + */ +contract MultipleCallbacksModule_Unit_ModuleData is BaseTest { + /** + * @notice Test that the moduleName function returns the correct name + */ + function test_moduleNameReturnsName() public { + assertEq(multipleCallbackModule.moduleName(), 'MultipleCallbacksModule'); + } +} + +contract MultipleCallbacksModule_Unit_FinalizeRequests is BaseTest { + /** + * @notice Test that finalizeRequests calls the _target.callback with the correct data + */ + function test_finalizeRequest(address[10] calldata _fuzzedTargets, bytes[10] calldata _fuzzedData) public { + address[] memory _targets = new address[](_fuzzedTargets.length); + bytes[] memory _data = new bytes[](_fuzzedTargets.length); + + // Copying the values to fresh arrays that we can use to build `RequestParameters` + for (uint256 _i; _i < _fuzzedTargets.length; _i++) { + _targets[_i] = _fuzzedTargets[_i]; + _data[_i] = _fuzzedData[_i]; + } + + mockRequest.finalityModuleData = + abi.encode(IMultipleCallbacksModule.RequestParameters({targets: _targets, data: _data})); + bytes32 _requestId = _getId(mockRequest); + mockResponse.requestId = _requestId; + + for (uint256 _i; _i < _targets.length; _i++) { + address _target = _targets[_i]; + bytes memory _calldata = _data[_i]; + + // Skip precompiles, VM, console.log addresses, etc + _assumeFuzzable(_target); + _mockAndExpect(_target, _calldata, abi.encode()); + + // Check: is the event emitted? + vm.expectEmit(true, true, true, true, address(multipleCallbackModule)); + emit Callback(_requestId, _target, _calldata); + } + + // Check: is the event emitted? + vm.expectEmit(true, true, true, true, address(multipleCallbackModule)); + emit RequestFinalized(_requestId, mockResponse, address(oracle)); + + vm.prank(address(oracle)); + multipleCallbackModule.finalizeRequest(mockRequest, mockResponse, address(oracle)); + } + + /** + * @notice Test that the finalizeRequests reverts if caller is not the oracle + */ + function test_revertsIfWrongCaller(IOracle.Request calldata _request, address _caller) public { + vm.assume(_caller != address(oracle)); + + // Check: does it revert if not called by the Oracle? + vm.expectRevert(IModule.Module_OnlyOracle.selector); + vm.prank(_caller); + multipleCallbackModule.finalizeRequest(_request, mockResponse, address(_caller)); + } +} diff --git a/yarn.lock b/yarn.lock index f3d82dcb..b61d241d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -192,10 +192,10 @@ dependencies: "@jridgewell/trace-mapping" "0.3.9" -"@defi-wonderland/prophet-core-contracts@0.0.0-a1d2cc55": - version "0.0.0-a1d2cc55" - resolved "https://registry.yarnpkg.com/@defi-wonderland/prophet-core-contracts/-/prophet-core-contracts-0.0.0-a1d2cc55.tgz#e0bba63cdb143ffba6721049d2b0577eb39329fb" - integrity sha512-gl+8QvkzPd144yESzXhl2ceJ5blZczKh7HLioSfJ1uZnrJAR90z/MJD1fCb/1Q3oDyaq6WMWnJSK9VvMkadTtQ== +"@defi-wonderland/prophet-core-contracts@0.0.0-1ae08a81": + version "0.0.0-1ae08a81" + resolved "https://registry.yarnpkg.com/@defi-wonderland/prophet-core-contracts/-/prophet-core-contracts-0.0.0-1ae08a81.tgz#77d8f91303c8496556c0cedc12eeaa7abaa5f6fb" + integrity sha512-hMOBUsXR39NXfhExV3+xDBaUYv/6EYdyTrS/VMMSh0Gpbeqql4cIlUHnJzhmc6b+sbIgREq1K44v++8Aminrvw== dependencies: "@defi-wonderland/solidity-utils" "0.0.0-3e9c8e8b" "@openzeppelin/contracts" "^4.9.3"