fix: address zero module

defi-wonderland · Dec 18, 2024 · 71ee837 · 71ee837
1 parent 3e1078a
commit 71ee837
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 5 deletions.
diff --git a/solidity/contracts/Oracle.sol b/solidity/contracts/Oracle.sol
@@ -443,6 +443,14 @@ contract Oracle is IOracle, OracleAccessController {
     hasAccess(_request.accessModule, OracleTypehash.CREATE_TYPEHASH, abi.encode(_request), _accessControl)
     returns (bytes32 _requestId)
   {
+    // Check that the request, response and dispute modules are set
+    if (
+      _request.requestModule == address(0) || _request.responseModule == address(0)
+        || _request.disputeModule == address(0)
+    ) {
+      revert Oracle_ZeroAddressModule();
+    }
+
     uint256 _requestNonce = totalRequestCount++;
 
     if (_request.nonce == 0) _request.nonce = uint96(_requestNonce);
@@ -455,16 +463,32 @@ contract Oracle is IOracle, OracleAccessController {
     nonceToRequestId[_requestNonce] = _requestId;
     requestCreatedAt[_requestId] = block.timestamp;
 
-    allowedModule[_requestId][_request.requestModule] = true;
-    allowedModule[_requestId][_request.responseModule] = true;
-    allowedModule[_requestId][_request.disputeModule] = true;
-    allowedModule[_requestId][_request.resolutionModule] = true;
-    allowedModule[_requestId][_request.finalityModule] = true;
+    _allowModules(_requestId, _request);
 
     isParticipant[_requestId][_accessControl.user] = true;
 
     IRequestModule(_request.requestModule).createRequest(_requestId, _request.requestModuleData, _accessControl.user);
 
     emit RequestCreated(_requestId, _request, _ipfsHash);
   }
+
+  /**
+   * @notice Allows the modules to interact with each other
+   *
+   * @param _requestId The id of the request
+   * @param _request The request struct
+   */
+  function _allowModules(bytes32 _requestId, Request memory _request) internal {
+    allowedModule[_requestId][_request.requestModule] = true;
+    allowedModule[_requestId][_request.responseModule] = true;
+    allowedModule[_requestId][_request.disputeModule] = true;
+
+    if (_request.resolutionModule != address(0)) {
+      allowedModule[_requestId][_request.resolutionModule] = true;
+    }
+
+    if (_request.finalityModule != address(0)) {
+      allowedModule[_requestId][_request.finalityModule] = true;
+    }
+  }
 }
diff --git a/solidity/interfaces/IOracle.sol b/solidity/interfaces/IOracle.sol
@@ -128,6 +128,11 @@ interface IOracle is IOracleAccessController {
    */
   error Oracle_InvalidRequestBody();
 
+  /**
+   * @notice Thrown when trying to create a request with a zero address module
+   */
+  error Oracle_ZeroAddressModule();
+
   /**
    * @notice Thrown when trying to propose a response with invalid parameters
    */

diff --git a/solidity/test/unit/Oracle.t.sol b/solidity/test/unit/Oracle.t.sol
@@ -214,6 +214,42 @@ contract Oracle_Unit_CreateRequest is BaseTest {
     oracle.createRequest(mockRequest, _ipfsHash, mockAccessControl);
   }
 
+  /**
+   * @notice Check that creating a request with a zero address request module reverts
+   */
+  function test_createRequest_revertsIfRequestModuleZeroAddress() public happyPath {
+    // Check: revert?
+    vm.expectRevert(IOracle.Oracle_ZeroAddressModule.selector);
+
+    mockRequest.requestModule = address(0);
+    // Test: try to create the request
+    oracle.createRequest(mockRequest, _ipfsHash, mockAccessControl);
+  }
+
+  /**
+   * @notice Check that creating a request with a zero address dispute module reverts
+   */
+  function test_createRequest_revertsIfDisputeModuleZeroAddress() public happyPath {
+    // Check: revert?
+    vm.expectRevert(IOracle.Oracle_ZeroAddressModule.selector);
+
+    mockRequest.disputeModule = address(0);
+    // Test: try to create the request
+    oracle.createRequest(mockRequest, _ipfsHash, mockAccessControl);
+  }
+
+  /**
+   * @notice Check that creating a request with a zero address response module reverts
+   */
+  function test_createRequest_revertsIfResponseModuleZeroAddress() public happyPath {
+    // Check: revert?
+    vm.expectRevert(IOracle.Oracle_ZeroAddressModule.selector);
+
+    mockRequest.responseModule = address(0);
+    // Test: try to create the request
+    oracle.createRequest(mockRequest, _ipfsHash, mockAccessControl);
+  }
+
   /**
    * @notice Check that reverts if the access module returns false
    */
@@ -262,6 +298,41 @@ contract Oracle_Unit_CreateRequest is BaseTest {
     // Test: try to create the request
     oracle.createRequest(mockRequest, _ipfsHash, mockAccessControl);
   }
+
+  /**
+   * @notice Check that creating a request with a zero address resolution module does not add zero address to the allowed modules
+   */
+  function test_createRequest_zeroAddressResolutionModule() public happyPath {
+    // Create the request
+    mockRequest.requester = requester;
+    mockRequest.nonce = uint96(oracle.totalRequestCount());
+    // Set the resolution module to zero address
+    mockRequest.resolutionModule = address(0);
+
+    // Test: create the request
+    bytes32 _requestId = oracle.createRequest(mockRequest, _ipfsHash, mockAccessControl);
+
+    // Check: zero address not added to the allowed modules
+    assertFalse(oracle.allowedModule(_requestId, address(0)));
+  }
+
+  /**
+   * @notice Check that creating a request with a zero address finality module does not add zero address to the allowed modules
+   */
+  function test_createRequest_zeroAddressFinalityModule() public happyPath {
+    // Create the request
+    mockRequest.requester = requester;
+    mockRequest.nonce = uint96(oracle.totalRequestCount());
+
+    // Set the finality module to zero address
+    mockRequest.finalityModule = address(0);
+
+    // Test: create the request
+    bytes32 _requestId = oracle.createRequest(mockRequest, _ipfsHash, mockAccessControl);
+
+    // Check: zero address not added to the allowed modules
+    assertFalse(oracle.allowedModule(_requestId, address(0)));
+  }
 }
 
 contract Oracle_Unit_CreateRequests is BaseTest {