From b72dcda6fff764359bdc87b6e6410b0207844672 Mon Sep 17 00:00:00 2001
From: shaito <106555513+0xShaito@users.noreply.github.com>
Date: Mon, 21 Oct 2024 11:44:26 -0300
Subject: [PATCH] fix: incrrect requestid calculation (#70)

---
 .../modules/response/BondedResponseModule.sol | 10 +++--
 .../integration/ContractCallRequest.t.sol     |  8 ++--
 .../response/BondedResponseModule.t.sol       | 41 +++++++++++++++++++
 3 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/solidity/contracts/modules/response/BondedResponseModule.sol b/solidity/contracts/modules/response/BondedResponseModule.sol
index ba41b872..13d85978 100644
--- a/solidity/contracts/modules/response/BondedResponseModule.sol
+++ b/solidity/contracts/modules/response/BondedResponseModule.sol
@@ -74,9 +74,11 @@ contract BondedResponseModule is Module, IBondedResponseModule {
   ) external override(IBondedResponseModule, Module) onlyOracle {
     RequestParameters memory _params = decodeRequestData(_request.responseModuleData);
 
-    bool _isModule = ORACLE.allowedModule(_response.requestId, _finalizer);
+    bytes32 _requestId = _getId(_request);
 
-    if (!_isModule && block.timestamp < ORACLE.requestCreatedAt(_response.requestId) + _params.deadline) {
+    bool _isModule = ORACLE.allowedModule(_requestId, _finalizer);
+
+    if (!_isModule && block.timestamp < ORACLE.requestCreatedAt(_requestId) + _params.deadline) {
       revert BondedResponseModule_TooEarlyToFinalize();
     }
 
@@ -88,13 +90,13 @@ contract BondedResponseModule is Module, IBondedResponseModule {
 
       _params.accountingExtension.release({
         _bonder: _response.proposer,
-        _requestId: _response.requestId,
+        _requestId: _requestId,
         _token: _params.bondToken,
         _amount: _params.bondSize
       });
     }
 
-    emit RequestFinalized(_response.requestId, _response, _finalizer);
+    emit RequestFinalized(_requestId, _response, _finalizer);
   }
 
   /// @inheritdoc IBondedResponseModule
diff --git a/solidity/test/integration/ContractCallRequest.t.sol b/solidity/test/integration/ContractCallRequest.t.sol
index bbaf8ee5..598ff0dc 100644
--- a/solidity/test/integration/ContractCallRequest.t.sol
+++ b/solidity/test/integration/ContractCallRequest.t.sol
@@ -47,10 +47,12 @@ contract Integration_ContractCallRequest is IntegrationBase {
   function test_createRequest_finalizeEmptyResponse() public {
     vm.prank(requester);
     bytes32 _requestId = oracle.createRequest(mockRequest, _ipfsHash);
+    uint256 _requestCreatedAt = oracle.requestCreatedAt(_requestId);
 
     // mock an empty response
-    mockResponse =
-      IOracle.Response({proposer: makeAddr('not-the-proposer'), requestId: bytes32(0), response: bytes('')});
+    mockResponse = IOracle.Response({proposer: address(0), requestId: bytes32(0), response: bytes('')});
+
+    assertEq(oracle.responseCreatedAt(_getId(mockResponse)), 0);
 
     // expect call to accounting to release requester's funds
     vm.expectCall(
@@ -58,7 +60,7 @@ contract Integration_ContractCallRequest is IntegrationBase {
       abi.encodeCall(IAccountingExtension.release, (mockRequest.requester, _requestId, usdc, _expectedReward))
     );
 
-    vm.warp(block.timestamp + 2 days);
+    vm.warp(_requestCreatedAt + _expectedDeadline);
     vm.prank(_finalizer);
     oracle.finalize(mockRequest, mockResponse);
   }
diff --git a/solidity/test/unit/modules/response/BondedResponseModule.t.sol b/solidity/test/unit/modules/response/BondedResponseModule.t.sol
index 5fb16cc0..01e2ea33 100644
--- a/solidity/test/unit/modules/response/BondedResponseModule.t.sol
+++ b/solidity/test/unit/modules/response/BondedResponseModule.t.sol
@@ -464,6 +464,47 @@ contract BondedResponseModule_Unit_FinalizeRequest is BaseTest {
     vm.prank(address(oracle));
     bondedResponseModule.finalizeRequest(mockRequest, mockResponse, _finalizer);
   }
+
+  function test_finalizeWithoutResponse(
+    IERC20 _token,
+    uint256 _bondSize,
+    uint256 _disputeWindow,
+    address _proposer,
+    uint256 _deadline
+  ) public {
+    _disputeWindow = bound(_disputeWindow, 61, 365 days);
+
+    // Check correct calls are made if deadline has passed
+    _deadline = bound(_deadline, 1, type(uint248).max);
+    mockRequest.responseModuleData = abi.encode(accounting, _token, _bondSize, _deadline, _disputeWindow);
+    bytes32 _requestId = _getId(mockRequest);
+    mockResponse.requestId = _requestId;
+    mockResponse.proposer = _proposer;
+
+    // Mock and expect IOracle.allowedModule to be called
+    _mockAndExpect(
+      address(oracle), abi.encodeCall(IOracle.allowedModule, (_requestId, address(this))), abi.encode(false)
+    );
+
+    _mockAndExpect(
+      address(oracle), abi.encodeCall(IOracle.requestCreatedAt, (_getId(mockRequest))), abi.encode(requestCreatedAt)
+    );
+
+    // Empty response
+    mockResponse = IOracle.Response({proposer: address(0), requestId: bytes32(0), response: bytes('')});
+
+    // Response does not exist
+    _mockAndExpect(address(oracle), abi.encodeCall(IOracle.responseCreatedAt, (_getId(mockResponse))), abi.encode(0));
+
+    // Check: is event emitted?
+    vm.expectEmit(true, true, true, true, address(bondedResponseModule));
+    emit RequestFinalized({_requestId: _getId(mockRequest), _response: mockResponse, _finalizer: address(this)});
+
+    vm.warp(requestCreatedAt + _deadline + _disputeWindow);
+
+    vm.prank(address(oracle));
+    bondedResponseModule.finalizeRequest(mockRequest, mockResponse, address(this));
+  }
 }
 
 contract BondedResponseModule_Unit_ReleaseUnutilizedResponse is BaseTest {