Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: address zero module #73

Open
wants to merge 1 commit into
base: fix/internal-review
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions solidity/contracts/Oracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}
}
}
5 changes: 5 additions & 0 deletions solidity/interfaces/IOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
71 changes: 71 additions & 0 deletions solidity/test/unit/Oracle.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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 {
Expand Down
Loading