fix: low-level calls (#56)

* fix: low-level calls

* feat: separate callback interface for verifier

solidity/contracts/modules/dispute/CircuitResolverModule.sol

@@ -5,6 +5,7 @@ pragma solidity ^0.8.19;
 import {IModule, Module} from '@defi-wonderland/prophet-core-contracts/solidity/contracts/Module.sol';
 import {IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/IOracle.sol';
 
+import {IProphetVerifier} from '../../../interfaces/IProphetVerifier.sol';
 import {ICircuitResolverModule} from '../../../interfaces/modules/dispute/ICircuitResolverModule.sol';
 
 contract CircuitResolverModule is Module, ICircuitResolverModule {
@@ -68,17 +69,18 @@ contract CircuitResolverModule is Module, ICircuitResolverModule {
     IOracle.Dispute calldata _dispute
   ) external onlyOracle {
     RequestParameters memory _params = decodeRequestData(_request.disputeModuleData);
+    IOracle.DisputeStatus _status;
 
-    (bool _success, bytes memory _correctResponse) = _params.verifier.call(_params.callData);
+    try IProphetVerifier(_params.verifier).prophetVerify(_params.callData) returns (bytes memory _correctResponse) {
+      _correctResponses[_response.requestId] = _correctResponse;
 
-    if (!_success) revert CircuitResolverModule_VerificationFailed();
-
-    _correctResponses[_response.requestId] = _correctResponse;
-
-    IOracle.DisputeStatus _status = _response.response.length != _correctResponse.length
-      || keccak256(_response.response) != keccak256(_correctResponse)
-      ? IOracle.DisputeStatus.Won
-      : IOracle.DisputeStatus.Lost;
+      _status = _response.response.length != _correctResponse.length
+        || keccak256(_response.response) != keccak256(_correctResponse)
+        ? IOracle.DisputeStatus.Won
+        : IOracle.DisputeStatus.Lost;
+    } catch {
+      revert CircuitResolverModule_VerificationFailed();
+    }
 
     emit ResponseDisputed({
       _requestId: _response.requestId,

solidity/contracts/modules/finality/CallbackModule.sol

@@ -5,6 +5,7 @@ pragma solidity ^0.8.19;
 import {IModule, Module} from '@defi-wonderland/prophet-core-contracts/solidity/contracts/Module.sol';
 import {IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/IOracle.sol';
 
+import {IProphetCallback} from '../../../interfaces/IProphetCallback.sol';
 import {ICallbackModule} from '../../../interfaces/modules/finality/ICallbackModule.sol';
 
 contract CallbackModule is Module, ICallbackModule {
@@ -28,7 +29,7 @@ contract CallbackModule is Module, ICallbackModule {
   ) external override(Module, ICallbackModule) onlyOracle {
     RequestParameters memory _params = decodeRequestData(_request.finalityModuleData);
 
-    _params.target.call(_params.data);
+    IProphetCallback(_params.target).prophetCallback(_params.data);
     emit Callback(_response.requestId, _params.target, _params.data);
     emit RequestFinalized(_response.requestId, _response, _finalizer);
   }

solidity/contracts/modules/finality/MultipleCallbacksModule.sol

@@ -5,6 +5,7 @@ pragma solidity ^0.8.19;
 import {IModule, Module} from '@defi-wonderland/prophet-core-contracts/solidity/contracts/Module.sol';
 import {IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/interfaces/IOracle.sol';
 
+import {IProphetCallback} from '../../../interfaces/IProphetCallback.sol';
 import {IMultipleCallbacksModule} from '../../../interfaces/modules/finality/IMultipleCallbacksModule.sol';
 
 contract MultipleCallbacksModule is Module, IMultipleCallbacksModule {
@@ -30,7 +31,7 @@ contract MultipleCallbacksModule is Module, IMultipleCallbacksModule {
     uint256 _length = _params.targets.length;
 
     for (uint256 _i; _i < _length;) {
-      _params.targets[_i].call(_params.data[_i]);
+      IProphetCallback(_params.targets[_i]).prophetCallback(_params.data[_i]);
       emit Callback(_response.requestId, _params.targets[_i], _params.data[_i]);
       unchecked {
         ++_i;

solidity/interfaces/IProphetCallback.sol

@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.19;
+
+interface IProphetCallback {
+  /**
+   * @notice Callback function for the Prophet modules
+   * @param _callData The encoded data for the callback
+   * @return _callResponse The encoded response for the callback
+   */
+  function prophetCallback(bytes calldata _callData) external returns (bytes memory _callResponse);
+}

solidity/interfaces/IProphetVerifier.sol

@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.19;
+
+interface IProphetVerifier {
+  /**
+   * @notice Verification function for the Prophet modules
+   * @param _callData The encoded data for the verification
+   * @return _callResponse The encoded response for the verification
+   */
+  function prophetVerify(bytes calldata _callData) external returns (bytes memory _callResponse);
+}

solidity/test/integration/Finalization.t.sol

@@ -5,58 +5,22 @@ import './IntegrationBase.sol';
 
 contract Integration_Finalization is IntegrationBase {
   address internal _finalizer = makeAddr('finalizer');
-  address internal _callbackTarget = makeAddr('target');
 
   function setUp() public override {
     super.setUp();
 
-    vm.etch(_callbackTarget, hex'069420');
-
-    _setFinalizationModule(
-      address(_callbackModule),
-      abi.encode(ICallbackModule.RequestParameters({target: _callbackTarget, data: bytes('')}))
-    );
-
     _deposit(_accountingExtension, requester, usdc, _expectedReward);
     _deposit(_accountingExtension, proposer, usdc, _expectedBondSize);
     _deposit(_accountingExtension, disputer, usdc, _expectedBondSize);
   }
 
-  /**
-   * @notice Test to check if another module can be set as callback module.
-   */
-  function test_targetIsAnotherModule() public {
-    _setFinalizationModule(
-      address(_callbackModule),
-      abi.encode(
-        ICallbackModule.RequestParameters({
-          target: address(_callbackModule),
-          data: abi.encodeWithSignature('callback()')
-        })
-      )
-    );
-
-    _createRequest();
-    _proposeResponse();
-
-    // Traveling to the end of the dispute window
-    vm.roll(_expectedDeadline + 1 + _baseDisputeWindow);
-
-    vm.prank(_finalizer);
-    oracle.finalize(mockRequest, mockResponse);
-
-    // Check: is response finalized?
-    bytes32 _finalizedResponseId = oracle.finalizedResponseId(_getId(mockRequest));
-    assertEq(_finalizedResponseId, _getId(mockResponse));
-  }
-
   /**
    * @notice Finalization data is set and callback calls are made.
    */
   function test_makeAndIgnoreLowLevelCalls(bytes memory _calldata) public {
     _setFinalizationModule(
       address(_callbackModule),
-      abi.encode(ICallbackModule.RequestParameters({target: _callbackTarget, data: _calldata}))
+      abi.encode(ICallbackModule.RequestParameters({target: address(_mockCallback), data: _calldata}))
     );
 
     _createRequest();
@@ -65,8 +29,8 @@ contract Integration_Finalization is IntegrationBase {
     // Traveling to the end of the dispute window
     vm.roll(_expectedDeadline + 1 + _baseDisputeWindow);
 
-    // Check: all low-level calls are made?
-    vm.expectCall(_callbackTarget, _calldata);
+    // Check: all external calls are made?
+    vm.expectCall(address(_mockCallback), abi.encodeWithSelector(IProphetCallback.prophetCallback.selector, _calldata));
 
     vm.prank(_finalizer);
     oracle.finalize(mockRequest, mockResponse);
@@ -126,7 +90,7 @@ contract Integration_Finalization is IntegrationBase {
   function test_finalizeWithUndisputedResponse(bytes calldata _calldata) public {
     _setFinalizationModule(
       address(_callbackModule),
-      abi.encode(ICallbackModule.RequestParameters({target: _callbackTarget, data: _calldata}))
+      abi.encode(ICallbackModule.RequestParameters({target: address(_mockCallback), data: _calldata}))
     );
 
     _createRequest();
@@ -135,7 +99,7 @@ contract Integration_Finalization is IntegrationBase {
     // Traveling to the end of the dispute window
     vm.roll(_expectedDeadline + 1 + _baseDisputeWindow);
 
-    vm.expectCall(_callbackTarget, _calldata);
+    vm.expectCall(address(_mockCallback), abi.encodeWithSelector(IProphetCallback.prophetCallback.selector, _calldata));
     vm.prank(_finalizer);
     oracle.finalize(mockRequest, mockResponse);
 
@@ -150,10 +114,10 @@ contract Integration_Finalization is IntegrationBase {
   function test_revertFinalizeBeforeDeadline(bytes calldata _calldata) public {
     _setFinalizationModule(
       address(_callbackModule),
-      abi.encode(ICallbackModule.RequestParameters({target: _callbackTarget, data: _calldata}))
+      abi.encode(ICallbackModule.RequestParameters({target: address(_mockCallback), data: _calldata}))
     );
 
-    vm.expectCall(_callbackTarget, _calldata);
+    vm.expectCall(address(_mockCallback), abi.encodeWithSelector(IProphetCallback.prophetCallback.selector, _calldata));
 
     _createRequest();
     _proposeResponse();

solidity/test/integration/IntegrationBase.sol

@@ -34,6 +34,8 @@ import {HttpRequestModule, IHttpRequestModule} from '../../contracts/modules/req
 import {ArbitratorModule, IArbitratorModule} from '../../contracts/modules/resolution/ArbitratorModule.sol';
 import {BondedResponseModule, IBondedResponseModule} from '../../contracts/modules/response/BondedResponseModule.sol';
 
+import {IProphetCallback} from '../../interfaces/IProphetCallback.sol';
+
 import {MockArbitrator} from '../mocks/MockArbitrator.sol';
 import {MockCallback} from '../mocks/MockCallback.sol';
 

solidity/test/mocks/MockCallback.sol

@@ -1,11 +1,10 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.19;
 
-contract MockCallback {
-  uint256 public randomValue;
+import {IProphetCallback} from '../../interfaces/IProphetCallback.sol';
 
-  function callback(bytes32, /* _requestId */ bytes calldata _data) external {
-    uint256 _randomValue = abi.decode(_data, (uint256));
-    randomValue = _randomValue;
+contract MockCallback is IProphetCallback {
+  function prophetCallback(bytes calldata /* _callData */ ) external pure returns (bytes memory _callResponse) {
+    _callResponse = abi.encode(true);
   }
 }

solidity/test/unit/modules/dispute/CircuitResolverModule.t.sol

@@ -14,6 +14,7 @@ import {
   ICircuitResolverModule
 } from '../../../../contracts/modules/dispute/CircuitResolverModule.sol';
 
+import {IProphetVerifier} from '../../../../interfaces/IProphetVerifier.sol';
 import {IAccountingExtension} from '../../../../interfaces/extensions/IAccountingExtension.sol';
 import {MockVerifier} from '../../../mocks/MockVerifier.sol';
 
@@ -136,8 +137,6 @@ contract CircuitResolverModule_Unit_DisputeResponse is BaseTest {
     uint256 _bondSize,
     bytes memory _callData
   ) public {
-    _callData = abi.encodeWithSelector(mockVerifier.calculateRoot.selector, _callData);
-
     mockRequest.disputeModuleData = abi.encode(
       ICircuitResolverModule.RequestParameters({
         callData: _callData,
@@ -148,14 +147,18 @@ contract CircuitResolverModule_Unit_DisputeResponse is BaseTest {
       })
     );
 
-    bool _correctResponse = false;
+    bytes memory _correctResponse = abi.encode(false);
 
     mockResponse.requestId = _getId(mockRequest);
     mockDispute.requestId = mockResponse.requestId;
     mockDispute.responseId = _getId(mockResponse);
 
     // Mock and expect the call to the verifier
-    _mockAndExpect(address(mockVerifier), _callData, abi.encode(_correctResponse));
+    _mockAndExpect(
+      address(mockVerifier),
+      abi.encodeWithSelector(IProphetVerifier.prophetVerify.selector, _callData),
+      abi.encode(_correctResponse)
+    );
 
     // Mock and expect the call the oracle, updating the dispute's status
     _mockAndExpect(
@@ -187,14 +190,17 @@ contract CircuitResolverModule_Unit_DisputeResponse is BaseTest {
       })
     );
 
-    bytes32 _requestId = _getId(mockRequest);
-    bool _correctResponse = false;
+    bytes memory _correctResponse = abi.encode(false);
 
-    mockResponse.requestId = _requestId;
+    mockResponse.requestId = _getId(mockRequest);
     mockResponse.response = abi.encode(true);
 
     // Mock and expect the call to the verifier
-    _mockAndExpect(address(mockVerifier), _callData, abi.encode(_correctResponse));
+    _mockAndExpect(
+      address(mockVerifier),
+      abi.encodeWithSelector(IProphetVerifier.prophetVerify.selector, _callData),
+      abi.encode(_correctResponse)
+    );
 
     // Mock and expect the call the oracle, updating the dispute's status
     _mockAndExpect(
@@ -228,8 +234,6 @@ contract CircuitResolverModule_Unit_DisputeResponse is BaseTest {
     uint256 _bondSize,
     bytes memory _callData
   ) public {
-    _callData = abi.encodeWithSelector(mockVerifier.calculateRoot.selector, _callData);
-
     mockRequest.disputeModuleData = abi.encode(
       ICircuitResolverModule.RequestParameters({
         callData: _callData,
@@ -240,13 +244,17 @@ contract CircuitResolverModule_Unit_DisputeResponse is BaseTest {
       })
     );
 
-    bytes memory _encodedCorrectResponse = abi.encode(true);
+    bytes memory _correctResponse = abi.encode(true);
 
     mockResponse.requestId = _getId(mockRequest);
-    mockResponse.response = _encodedCorrectResponse;
+    mockResponse.response = _correctResponse;
 
     // Mock and expect the call to the verifier
-    _mockAndExpect(address(mockVerifier), _callData, _encodedCorrectResponse);
+    _mockAndExpect(
+      address(mockVerifier),
+      abi.encodeWithSelector(IProphetVerifier.prophetVerify.selector, _callData),
+      abi.encode(_correctResponse)
+    );
 
     // Mock and expect the call the oracle, updating the dispute's status
     _mockAndExpect(

solidity/test/unit/modules/finality/CallbackModule.t.sol

@@ -10,6 +10,8 @@ import {IOracle} from '@defi-wonderland/prophet-core-contracts/solidity/interfac
 
 import {CallbackModule, ICallbackModule} from '../../../../contracts/modules/finality/CallbackModule.sol';
 
+import {IProphetCallback} from '../../../../interfaces/IProphetCallback.sol';
+
 /**
  * @title Callback Module Unit tests
  */
@@ -84,6 +86,9 @@ contract CallbackModule_Unit_FinalizeRequest is BaseTest {
     mockRequest.finalityModuleData = abi.encode(ICallbackModule.RequestParameters({target: _target, data: _data}));
     mockResponse.requestId = _getId(mockRequest);
 
+    // Mock and expect the callback
+    _mockAndExpect(_target, abi.encodeWithSelector(IProphetCallback.prophetCallback.selector, _data), abi.encode(''));
+
     // Check: is the event emitted?
     vm.expectEmit(true, true, true, true, address(callbackModule));
     emit Callback(mockResponse.requestId, _target, _data);
@@ -108,7 +113,7 @@ contract CallbackModule_Unit_FinalizeRequest is BaseTest {
     mockResponse.requestId = _getId(mockRequest);
 
     // Mock and expect the callback
-    _mockAndExpect(_target, _data, abi.encode(''));
+    _mockAndExpect(_target, abi.encodeWithSelector(IProphetCallback.prophetCallback.selector, _data), abi.encode(''));
 
     vm.prank(address(oracle));
     callbackModule.finalizeRequest(mockRequest, mockResponse, _proposer);

solidity/test/unit/modules/finality/MultipleCallbacksModule.t.sol

@@ -13,6 +13,8 @@ import {
   MultipleCallbacksModule
 } from '../../../../contracts/modules/finality/MultipleCallbacksModule.sol';
 
+import {IProphetCallback} from '../../../../interfaces/IProphetCallback.sol';
+
 contract BaseTest is Test, Helpers {
   // The target contract
   MultipleCallbacksModule public multipleCallbackModule;
@@ -102,7 +104,9 @@ contract MultipleCallbacksModule_Unit_FinalizeRequests is BaseTest {
 
       // Skip precompiles, VM, console.log addresses, etc
       _assumeFuzzable(_target);
-      _mockAndExpect(_target, _calldata, abi.encode());
+      _mockAndExpect(
+        _target, abi.encodeWithSelector(IProphetCallback.prophetCallback.selector, _calldata), abi.encode('')
+      );
 
       // Check: is the event emitted?
       vm.expectEmit(true, true, true, true, address(multipleCallbackModule));