perf: remove unreachable reversion

defi-wonderland · Aug 9, 2024 · 2e39539 · 2e39539
1 parent 172a7bc
commit 2e39539
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 39 deletions.
diff --git a/solidity/contracts/Oracle.sol b/solidity/contracts/Oracle.sol
@@ -107,9 +107,10 @@ contract Oracle is IOracle {
     Request calldata _request,
     Response calldata _response
   ) external returns (bytes32 _responseId) {
-    bytes32 _requestId = ValidatorLib._getId(_request);
     _responseId = ValidatorLib._validateResponse(_request, _response);
 
+    bytes32 _requestId = _response.requestId;
+
     if (requestCreatedAt[_requestId] == 0) {
       revert Oracle_InvalidRequest();
     }
@@ -124,16 +125,16 @@ contract Oracle is IOracle {
       revert Oracle_InvalidResponseBody();
     }
 
-    if (finalizedAt[_response.requestId] != 0) {
-      revert Oracle_AlreadyFinalized(_response.requestId);
+    if (finalizedAt[_requestId] != 0) {
+      revert Oracle_AlreadyFinalized(_requestId);
     }
 
-    _participants[_response.requestId] = abi.encodePacked(_participants[_response.requestId], _response.proposer);
+    _participants[_requestId] = abi.encodePacked(_participants[_requestId], _response.proposer);
     IResponseModule(_request.responseModule).propose(_request, _response, msg.sender);
-    _responseIds[_response.requestId] = abi.encodePacked(_responseIds[_response.requestId], _responseId);
+    _responseIds[_requestId] = abi.encodePacked(_responseIds[_requestId], _responseId);
     responseCreatedAt[_responseId] = uint128(block.number);
 
-    emit ResponseProposed(_response.requestId, _responseId, _response, block.number);
+    emit ResponseProposed(_requestId, _responseId, _response, block.number);
   }
 
   /// @inheritdoc IOracle
@@ -145,6 +146,8 @@ contract Oracle is IOracle {
     bytes32 _responseId;
     (_responseId, _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);
 
+    bytes32 _requestId = _dispute.requestId;
+
     if (responseCreatedAt[_responseId] == 0) {
       revert Oracle_InvalidResponse();
     }
@@ -157,33 +160,33 @@ contract Oracle is IOracle {
       revert Oracle_InvalidDisputeBody();
     }
 
-    if (finalizedAt[_response.requestId] != 0) {
-      revert Oracle_AlreadyFinalized(_response.requestId);
+    if (finalizedAt[_requestId] != 0) {
+      revert Oracle_AlreadyFinalized(_requestId);
     }
 
-    if (disputeOf[_dispute.responseId] != bytes32(0)) {
-      revert Oracle_ResponseAlreadyDisputed(_dispute.responseId);
+    if (disputeOf[_responseId] != bytes32(0)) {
+      revert Oracle_ResponseAlreadyDisputed(_responseId);
     }
 
-    _participants[_response.requestId] = abi.encodePacked(_participants[_response.requestId], msg.sender);
+    _participants[_requestId] = abi.encodePacked(_participants[_requestId], msg.sender);
     disputeStatus[_disputeId] = DisputeStatus.Active;
-    disputeOf[_dispute.responseId] = _disputeId;
+    disputeOf[_responseId] = _disputeId;
     disputeCreatedAt[_disputeId] = uint128(block.number);
 
     IDisputeModule(_request.disputeModule).disputeResponse(_request, _response, _dispute);
 
-    emit ResponseDisputed(_dispute.responseId, _disputeId, _dispute, block.number);
+    emit ResponseDisputed(_responseId, _disputeId, _dispute, block.number);
   }
 
   /// @inheritdoc IOracle
   function escalateDispute(Request calldata _request, Response calldata _response, Dispute calldata _dispute) external {
-    (, bytes32 _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);
+    (bytes32 _responseId, bytes32 _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);
 
     if (disputeCreatedAt[_disputeId] == 0) {
       revert Oracle_InvalidDispute();
     }
 
-    if (disputeOf[_dispute.responseId] != _disputeId) {
+    if (disputeOf[_responseId] != _disputeId) {
       revert Oracle_InvalidDisputeId(_disputeId);
     }
 
@@ -207,13 +210,13 @@ contract Oracle is IOracle {
 
   /// @inheritdoc IOracle
   function resolveDispute(Request calldata _request, Response calldata _response, Dispute calldata _dispute) external {
-    (, bytes32 _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);
+    (bytes32 _responseId, bytes32 _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);
 
     if (disputeCreatedAt[_disputeId] == 0) {
       revert Oracle_InvalidDispute();
     }
 
-    if (disputeOf[_dispute.responseId] != _disputeId) {
+    if (disputeOf[_responseId] != _disputeId) {
       revert Oracle_InvalidDisputeId(_disputeId);
     }
 
@@ -239,13 +242,13 @@ contract Oracle is IOracle {
     Dispute calldata _dispute,
     DisputeStatus _status
   ) external {
-    (, bytes32 _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);
+    (bytes32 _responseId, bytes32 _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);
 
     if (disputeCreatedAt[_disputeId] == 0) {
       revert Oracle_InvalidDispute();
     }
 
-    if (disputeOf[_dispute.responseId] != _disputeId) {
+    if (disputeOf[_responseId] != _disputeId) {
       revert Oracle_InvalidDisputeId(_disputeId);
     }
 
@@ -394,15 +397,12 @@ contract Oracle is IOracle {
   ) internal returns (bytes32 _requestId, bytes32 _responseId) {
     _responseId = ValidatorLib._validateResponse(_request, _response);
 
+    _requestId = _response.requestId;
+
     if (responseCreatedAt[_responseId] == 0) {
       revert Oracle_InvalidResponse();
     }
 
-    _requestId = _response.requestId;
-    if (!_matchBytes(_responseId, _responseIds[_requestId], 32)) {
-      revert Oracle_InvalidFinalizedResponse();
-    }
-
     DisputeStatus _status = disputeStatus[disputeOf[_responseId]];
 
     if (_status != DisputeStatus.None && _status != DisputeStatus.Lost) {
@@ -428,7 +428,7 @@ contract Oracle is IOracle {
       revert Oracle_InvalidRequestBody();
     }
 
-    _requestId = keccak256(abi.encode(_request));
+    _requestId = ValidatorLib._getId(_request);
     nonceToRequestId[_requestNonce] = _requestId;
     requestCreatedAt[_requestId] = uint128(block.number);
 

diff --git a/solidity/test/unit/Oracle.t.sol b/solidity/test/unit/Oracle.t.sol
@@ -956,7 +956,7 @@ contract Oracle_Unit_Finalize is BaseTest {
   function test_finalize_revertsIfInvalidResponse() public {
     bytes32 _requestId = _getId(mockRequest);
     oracle.mock_setRequestCreatedAt(_requestId, uint128(block.number));
-    oracle.mock_setResponseCreatedAt(_getId(mockResponse), 0);
+    oracle.mock_setResponseCreatedAt(_requestId, 0);
 
     // Check: revert?
     vm.expectRevert(IOracle.Oracle_InvalidResponse.selector);
@@ -1004,19 +1004,6 @@ contract Oracle_Unit_Finalize is BaseTest {
     oracle.finalize(mockRequest, mockResponse);
   }
 
-  /**
-   * @notice Finalizing a request with an unrelated response
-   */
-  function test_finalize_withResponse_revertsIfInvalidResponse() public {
-    oracle.mock_setRequestCreatedAt(_getId(mockRequest), uint128(block.number));
-    oracle.mock_setResponseCreatedAt(_getId(mockResponse), uint128(block.number));
-
-    // Test: finalize the request
-    vm.expectRevert(IOracle.Oracle_InvalidFinalizedResponse.selector);
-    vm.prank(requester);
-    oracle.finalize(mockRequest, mockResponse);
-  }
-
   /**
    * @notice Finalizing a request with a successfully disputed response should revert
    */