feat: modifier and tests

Author: ashitakah

solidity/contracts/Oracle.sol

@@ -18,7 +18,6 @@ import {
   _ESCALATE_TYPEHASH,
   _FINALIZE_TYPEHASH,
   _PROPOSE_TYPEHASH,
-  _RESOLVE_TYPEHASH,
   _UPDATE_TYPEHASH
 } from './utils/OracleTypehash.sol';
 
@@ -66,6 +65,18 @@ contract Oracle is IOracle, AccessController {
    */
   mapping(bytes32 _requestId => bytes _responseIds) internal _responseIds;
 
+  /**
+   * @notice Modifier to check if the user approved to the access control module
+   * @param _user The address of the user
+   * @param _accessControlModule The access control module to check if approved
+   */
+  modifier isApproved(address _user, address _accessControlModule) {
+    if (_accessControlModule != address(0) && !isAccessControlApproved[_user][_accessControlModule]) {
+      revert Oracle_AccessControlModuleNotApproved();
+    }
+    _;
+  }
+
   /// @inheritdoc IOracle
   function getResponseIds(bytes32 _requestId) public view returns (bytes32[] memory _ids) {
     bytes memory _responses = _responseIds[_requestId];
@@ -107,6 +118,13 @@ contract Oracle is IOracle, AccessController {
     }
   }
 
+  /// @inheritdoc IOracle
+  function setAccessControlModule(address _accessControlModule, bool _approved) external {
+    isAccessControlApproved[msg.sender][_accessControlModule] = _approved;
+
+    emit AccessControlModuleSet(msg.sender, _accessControlModule, _approved);
+  }
+
   /// @inheritdoc IOracle
   function createRequest(
     Request calldata _request,
@@ -125,7 +143,7 @@ contract Oracle is IOracle, AccessController {
     uint256 _requestsAmount = _requestsData.length;
     _batchRequestsIds = new bytes32[](_requestsAmount);
 
-    for (uint256 _i = 0; _i < _requestsAmount;) {
+    for (uint256 _i; _i < _requestsAmount;) {
       _batchRequestsIds[_i] = _createRequest(_requestsData[_i], _ipfsHashes[_i], _accessControl[_i]);
       unchecked {
         ++_i;
@@ -140,6 +158,7 @@ contract Oracle is IOracle, AccessController {
     AccessControl calldata _accessControl
   )
     external
+    isApproved(_accessControl.user, _request.accessControlModule)
     hasAccess(_request.accessControlModule, _PROPOSE_TYPEHASH, abi.encode(_request, _response), _accessControl)
     returns (bytes32 _responseId)
   {
@@ -180,6 +199,7 @@ contract Oracle is IOracle, AccessController {
     AccessControl calldata _accessControl
   )
     external
+    isApproved(_accessControl.user, _request.accessControlModule)
     hasAccess(_request.accessControlModule, _DISPUTE_TYPEHASH, abi.encode(_request, _response, _dispute), _accessControl)
     returns (bytes32 _disputeId)
   {
@@ -225,6 +245,7 @@ contract Oracle is IOracle, AccessController {
     AccessControl calldata _accessControl
   )
     external
+    isApproved(_accessControl.user, _request.accessControlModule)
     hasAccess(_request.accessControlModule, _ESCALATE_TYPEHASH, abi.encode(_request, _response, _dispute), _accessControl)
   {
     (bytes32 _responseId, bytes32 _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);
@@ -256,15 +277,7 @@ contract Oracle is IOracle, AccessController {
   }
 
   /// @inheritdoc IOracle
-  function resolveDispute(
-    Request calldata _request,
-    Response calldata _response,
-    Dispute calldata _dispute,
-    AccessControl calldata _accessControl
-  )
-    external
-    hasAccess(_request.accessControlModule, _RESOLVE_TYPEHASH, abi.encode(_request, _response, _dispute), _accessControl)
-  {
+  function resolveDispute(Request calldata _request, Response calldata _response, Dispute calldata _dispute) external {
     (bytes32 _responseId, bytes32 _disputeId) = ValidatorLib._validateResponseAndDispute(_request, _response, _dispute);
 
     if (disputeCreatedAt[_disputeId] == 0) {
@@ -292,13 +305,14 @@ contract Oracle is IOracle, AccessController {
 
   /// @inheritdoc IOracle
   function updateDisputeStatus(
-    Request calldata _request,
-    Response calldata _response,
-    Dispute calldata _dispute,
+    Request memory _request,
+    Response memory _response,
+    Dispute memory _dispute,
     DisputeStatus _status,
     AccessControl calldata _accessControl
   )
     external
+    isApproved(_accessControl.user, _request.accessControlModule)
     hasAccess(
       _request.accessControlModule,
       _UPDATE_TYPEHASH,
@@ -316,16 +330,24 @@ contract Oracle is IOracle, AccessController {
       revert Oracle_InvalidDisputeId(_disputeId);
     }
 
+    // Needed to avoid stack too deep when try to compile
+    Request memory _currentRequest = _request;
+    Response memory _currentResponse = _response;
+    Dispute memory _currentDispute = _dispute;
+    DisputeStatus _currentStatus = _status;
+
     if (
-      _accessControl.user != address(_request.disputeModule)
-        && _accessControl.user != address(_request.resolutionModule)
+      _accessControl.user != address(_currentRequest.disputeModule)
+        && _accessControl.user != address(_currentRequest.resolutionModule)
     ) {
       revert Oracle_NotDisputeOrResolutionModule(_accessControl.user);
     }
-    disputeStatus[_disputeId] = _status;
-    IDisputeModule(_request.disputeModule).onDisputeStatusChange(_disputeId, _request, _response, _dispute);
+    disputeStatus[_disputeId] = _currentStatus;
+    IDisputeModule(_currentRequest.disputeModule).onDisputeStatusChange(
+      _disputeId, _currentRequest, _currentResponse, _currentDispute
+    );
 
-    emit DisputeStatusUpdated(_disputeId, _dispute, _status);
+    emit DisputeStatusUpdated(_disputeId, _currentDispute, _currentStatus);
   }
 
   /// @inheritdoc IOracle
@@ -445,6 +467,7 @@ contract Oracle is IOracle, AccessController {
     AccessControl calldata _accessControl
   )
     internal
+    isApproved(_accessControl.user, _request.accessControlModule)
     hasAccess(_request.accessControlModule, _CREATE_TYPEHASH, abi.encode(_request), _accessControl)
     returns (bytes32 _requestId)
   {

solidity/contracts/utils/OracleTypehash.sol

@@ -10,8 +10,6 @@ bytes32 constant _DISPUTE_TYPEHASH = keccak256('DisputeResponse(Request _request
 
 bytes32 constant _ESCALATE_TYPEHASH = keccak256('EscalateDispute(Request _request,Response _response,Dispute _dispute)');
 
-bytes32 constant _RESOLVE_TYPEHASH = keccak256('ResolveDispute(Request _request,Response _response,Dispute _dispute)');
-
 bytes32 constant _UPDATE_TYPEHASH =
   keccak256('UpdateDisputeStatus(Request _request,Response _response,Dispute _dispute,DisputeStatus _status)');
 

solidity/interfaces/IOracle.sol

@@ -12,6 +12,14 @@ interface IOracle is IAccessController {
                               EVENTS
   //////////////////////////////////////////////////////////////*/
 
+  /**
+   * @notice Emitted when the access control module is set
+   * @param _user The address of the user
+   * @param _accessControlModule The address of the access control module
+   * @param _approved If the module is approved
+   */
+  event AccessControlModuleSet(address indexed _user, address indexed _accessControlModule, bool _approved);
+
   /**
    * @notice Emitted when a request is created
    * @param _requestId The id of the created request
@@ -70,6 +78,11 @@ interface IOracle is IAccessController {
                               ERRORS
   //////////////////////////////////////////////////////////////*/
 
+  /**
+   * @notice Thrown when user didn't approve the access control module
+   */
+  error Oracle_AccessControlModuleNotApproved();
+
   /**
    * @notice Thrown when an unauthorized caller is trying to change a dispute's status
    * @param _caller The caller of the function
@@ -370,6 +383,14 @@ interface IOracle is IAccessController {
                               LOGIC
   //////////////////////////////////////////////////////////////*/
 
+  /**
+   * @notice Sets the address of the access control module
+   *
+   * @param _accessControlModule The address of the access control module
+   * @param _approved If the module is approved
+   */
+  function setAccessControlModule(address _accessControlModule, bool _approved) external;
+
   /**
    * @notice Generates the request ID and initializes the modules for the request
    *
@@ -450,14 +471,8 @@ interface IOracle is IAccessController {
    * @param _request The request
    * @param _response The disputed response
    * @param _dispute The dispute that is being resolved
-   * @param _accessControl The access control data
    */
-  function resolveDispute(
-    Request calldata _request,
-    Response calldata _response,
-    Dispute calldata _dispute,
-    AccessControl calldata _accessControl
-  ) external;
+  function resolveDispute(Request calldata _request, Response calldata _response, Dispute calldata _dispute) external;
 
   /**
    * @notice Updates the status of a dispute

solidity/test/integration/EscalateDispute.t.sol

@@ -14,12 +14,19 @@ contract Integration_EscalateDispute is IntegrationBase {
     oracle.proposeResponse(mockRequest, mockResponse, mockAccessControl);
     vm.stopPrank();
 
-    // Dispute reverts if caller is not authorized
-    vm.startPrank(badCaller);
-    vm.expectRevert(abi.encodeWithSelector(IAccessController.AccessControlData_NoAccess.selector));
+    // Revert if not approved to dispute
+    address _badDisputer = makeAddr('badDisputer');
+    mockAccessControl.user = _badDisputer;
+
+    // Reverts because caller is not approved to dispute
+    vm.expectRevert(abi.encodeWithSelector(IOracle.Oracle_AccessControlModuleNotApproved.selector));
+    oracle.disputeResponse(mockRequest, mockResponse, mockDispute, mockAccessControl);
+
+    // Dispute reverts if caller is not approved
     mockAccessControl.user = disputer;
+    vm.expectRevert(abi.encodeWithSelector(IAccessController.AccessControlData_NoAccess.selector));
+    vm.prank(badCaller);
     oracle.disputeResponse(mockRequest, mockResponse, mockDispute, mockAccessControl);
-    vm.stopPrank();
 
     // Dispute the response
     vm.startPrank(caller);

solidity/test/integration/IntegrationBase.sol

@@ -149,9 +149,10 @@ contract IntegrationBase is TestConstants, Helpers {
     mockDispute.responseId = _getId(mockResponse);
 
     // Add the allowed callers to the access control module
-    address[] memory _allowedCallers = new address[](1);
-    _allowedCallers[0] = caller;
-    _accessControlModule.setHasAccess(_allowedCallers);
+    _setAccessControlForACaller(requester, caller);
+    _setAccessControlForACaller(proposer, caller);
+    _setAccessControlForACaller(disputer, caller);
+    _setAccessControlForACaller(finalizer, caller);
 
     vm.startPrank(caller);
   }
@@ -164,4 +165,13 @@ contract IntegrationBase is TestConstants, Helpers {
     vm.warp(block.timestamp + _blocks * BLOCK_TIME);
     vm.roll(block.number + _blocks);
   }
+
+  function _setAccessControlForACaller(address _delegator, address _caller) internal {
+    vm.startPrank(_delegator);
+    address[] memory _allowedCallers = new address[](1);
+    _allowedCallers[0] = _caller;
+    _accessControlModule.setHasAccess(_allowedCallers);
+    oracle.setAccessControlModule(address(_accessControlModule), true);
+    vm.stopPrank();
+  }
 }

solidity/test/integration/ResponseDispute.t.sol

@@ -30,19 +30,28 @@ contract Integration_ResponseDispute is IntegrationBase {
     mockAccessControl.user = finalizer;
     oracle.finalize(mockRequest, mockResponse, mockAccessControl);
 
+    // Revert if the dispute is already finalized
     mockAccessControl.user = disputer;
     vm.expectRevert(abi.encodeWithSelector(IOracle.Oracle_AlreadyFinalized.selector, _requestId));
     oracle.disputeResponse(mockRequest, mockResponse, mockDispute, mockAccessControl);
   }
 
   function test_disputeResponse_alreadyDisputed() public {
+    // Dispute the response
     mockAccessControl.user = disputer;
     oracle.disputeResponse(mockRequest, mockResponse, mockDispute, mockAccessControl);
 
     address _anotherDisputer = makeAddr('anotherDisputer');
     mockDispute.disputer = _anotherDisputer;
-
     mockAccessControl.user = _anotherDisputer;
+
+    // Revert if the response is access control is not approved
+    vm.expectRevert(abi.encodeWithSelector(IOracle.Oracle_AccessControlModuleNotApproved.selector));
+    oracle.disputeResponse(mockRequest, mockResponse, mockDispute, mockAccessControl);
+
+    //  Revert if the response is already disputed
+    mockAccessControl.user = disputer;
+    mockDispute.disputer = disputer;
     vm.expectRevert(abi.encodeWithSelector(IOracle.Oracle_ResponseAlreadyDisputed.selector, _responseId));
     oracle.disputeResponse(mockRequest, mockResponse, mockDispute, mockAccessControl);
   }

solidity/test/integration/ResponseProposal.t.sol

@@ -8,23 +8,48 @@ contract Integration_ResponseProposal is IntegrationBase {
 
   function setUp() public override {
     super.setUp();
-    vm.stopPrank();
 
     mockRequest.nonce = uint96(oracle.totalRequestCount());
-    mockAccessControl.user = requester;
 
-    vm.startPrank(badCaller);
-    vm.expectRevert(abi.encodeWithSelector(IAccessController.AccessControlData_NoAccess.selector));
+    address _badRequester = makeAddr('badRequester');
+    mockAccessControl.user = _badRequester;
+
+    // Revert if not approved to create request
+    vm.expectRevert(abi.encodeWithSelector(IOracle.Oracle_AccessControlModuleNotApproved.selector));
     oracle.createRequest(mockRequest, _ipfsHash, mockAccessControl);
+
     vm.stopPrank();
 
+    // Revert if has no access to create request
+    mockAccessControl.user = requester;
+    vm.expectRevert(abi.encodeWithSelector(IAccessController.AccessControlData_NoAccess.selector));
+    vm.prank(badCaller);
+    oracle.createRequest(mockRequest, _ipfsHash, mockAccessControl);
+
+    // Create request
     vm.startPrank(caller);
     _requestId = oracle.createRequest(mockRequest, _ipfsHash, mockAccessControl);
   }
 
   function test_proposeResponse_validResponse(bytes memory _response) public {
     mockResponse.response = _response;
 
+    // Revert if not approved to create request
+    vm.expectRevert(abi.encodeWithSelector(IOracle.Oracle_AccessControlModuleNotApproved.selector));
+    address _badProposer = makeAddr('badProposer');
+    mockAccessControl.user = _badProposer;
+    oracle.proposeResponse(mockRequest, mockResponse, mockAccessControl);
+
+    vm.stopPrank();
+
+    // Revert if has no access to create request
+    mockAccessControl.user = proposer;
+    vm.expectRevert(abi.encodeWithSelector(IAccessController.AccessControlData_NoAccess.selector));
+    vm.prank(badCaller);
+    oracle.proposeResponse(mockRequest, mockResponse, mockAccessControl);
+
+    vm.startPrank(caller);
+    // Propose response
     mockAccessControl.user = proposer;
     oracle.proposeResponse(mockRequest, mockResponse, mockAccessControl);
 
@@ -38,13 +63,16 @@ contract Integration_ResponseProposal is IntegrationBase {
   function test_proposeResponse_finalizedRequest(uint256 _timestamp) public {
     _timestamp = bound(_timestamp, _expectedDeadline + _baseDisputeWindow, type(uint128).max);
 
+    // Propose response
     mockAccessControl.user = proposer;
     oracle.proposeResponse(mockRequest, mockResponse, mockAccessControl);
 
+    // Finalize request
     vm.warp(_timestamp);
     mockAccessControl.user = finalizer;
     oracle.finalize(mockRequest, mockResponse, mockAccessControl);
 
+    // Revert if the request is already finalized
     mockAccessControl.user = proposer;
     mockResponse.response = abi.encode(_timestamp);
     vm.expectRevert(abi.encodeWithSelector(IOracle.Oracle_AlreadyFinalized.selector, _requestId));

solidity/test/mocks/contracts/MockAtomicArbitrator.sol

@@ -18,7 +18,7 @@ contract MockAtomicArbitrator {
   ) external returns (bytes memory _result) {
     _result = new bytes(0);
     answer = IOracle.DisputeStatus.Won;
-    // oracle.resolveDispute(_request, _response, _dispute);
+    oracle.resolveDispute(_request, _response, _dispute);
   }
 
   function getAnswer(bytes32 /* _dispute */ ) external view returns (IOracle.DisputeStatus _answer) {