diff --git a/solidity/contracts/Oracle.sol b/solidity/contracts/Oracle.sol index 9283cb9..a940b28 100644 --- 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,11 +463,7 @@ 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; @@ -467,4 +471,24 @@ contract Oracle is IOracle, OracleAccessController { 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 index 6246de7..5eabe61 100644 --- 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 index 6034398..592f227 100644 --- 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 {