feat: address comments

defi-wonderland · Nov 7, 2024 · 14f565e · 14f565e
1 parent 5a0a725
commit 14f565e
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 15 deletions.
diff --git a/solidity/contracts/CommonAccessController.sol b/solidity/contracts/CommonAccessController.sol
@@ -6,22 +6,20 @@ import {IAccessModule} from '../interfaces/modules/accessControl/IAccessModule.s
 
 abstract contract CommonAccessController is IAccessController {
   /**
-   * @notice Modifier to check if the caller has access to the user
-   * @dev The return value of this function must always be `true`,
-   *       otherwise it would revert.
+   * @notice Check whether the caller is authorized for the given parameters.
    * @param _accessModule The access module
    * @param _typehash The typehash
-   * @param _params The params passed to the typehash
+   * @param _typehashParams The params passed to the typehash
    * @param _accessControl The access control struct
    */
   function _hasAccess(
     address _accessModule,
     bytes32 _typehash,
-    bytes memory _params,
+    bytes memory _typehashParams,
     AccessControl memory _accessControl
-  ) internal returns (bool _granted) {
+  ) internal {
     // todo: if _accessModule == address(0) we should skip this check.
-    _granted = msg.sender == _accessControl.user
+    bool _granted = msg.sender == _accessControl.user
       || (
         _accessModule != address(0)
           && IAccessModule(_accessModule).hasAccess(
@@ -30,7 +28,7 @@ abstract contract CommonAccessController is IAccessController {
                 sender: msg.sender,
                 accessControl: _accessControl,
                 typehash: _typehash,
-                params: _params
+                typehashParams: _typehashParams
               })
             )
           )

diff --git a/solidity/contracts/Oracle.sol b/solidity/contracts/Oracle.sol
@@ -353,7 +353,10 @@ contract Oracle is IOracle, OracleAccessController, OracleTypehash {
     IDisputeModule(_request.disputeModule).finalizeRequest(_request, _response, _accessControl.user);
     IResponseModule(_request.responseModule).finalizeRequest(_request, _response, _accessControl.user);
     IRequestModule(_request.requestModule).finalizeRequest(_request, _response, _accessControl.user);
-    IAccessModule(_request.accessModule).finalizeRequest(_request, _response, _accessControl.user);
+
+    if (_request.accessModule != address(0)) {
+      IAccessModule(_request.accessModule).finalizeRequest(_request, _response, _accessControl.user);
+    }
 
     emit OracleRequestFinalized(_requestId, _responseId);
   }

diff --git a/solidity/contracts/OracleAccessController.sol b/solidity/contracts/OracleAccessController.sol
@@ -14,7 +14,6 @@ abstract contract OracleAccessController is IOracleAccessController, CommonAcces
     bytes memory _params,
     AccessControl memory _accessControl
   ) {
-    // we do not care about `_hasAccess` return value, check function's @notice
     _hasAccess(_accessModule, _typehash, _params, _accessControl);
     _;
   }

diff --git a/solidity/interfaces/modules/accessControl/IAccessModule.sol b/solidity/interfaces/modules/accessControl/IAccessModule.sol
@@ -25,14 +25,14 @@ interface IAccessModule is IModule {
     address sender;
     IAccessController.AccessControl accessControl;
     bytes32 typehash;
-    bytes params;
+    bytes typehashParams;
   }
 
   /*///////////////////////////////////////////////////////////////
                               LOGIC
   //////////////////////////////////////////////////////////////*/
   /**
-   * @notice Checks if the caller has access to the user
+   * @notice Check whether the caller is authorized for the given parameters.
    * @param _data The data for access control validation
    * @return _hasAccess True if the caller has access to the user
    */

diff --git a/solidity/test/mocks/contracts/MockAccessModule.sol b/solidity/test/mocks/contracts/MockAccessModule.sol
@@ -19,7 +19,7 @@ contract MockAccessModule is Module, IMockAccessModule {
 
   function moduleName() external view returns (string memory _moduleName) {}
 
-  function hasAccess(bytes memory _data) external view override returns (bool _hasAccess) {
+  function hasAccess(bytes memory _data) external view returns (bool _hasAccess) {
     IAccessModule.AccessControlParameters memory _accessControlData = decodeAccesControlData(_data);
     _hasAccess = callerHasAccess[_accessControlData.sender];
   }

diff --git a/solidity/test/unit/AccessController.t.sol b/solidity/test/unit/AccessController.t.sol
@@ -61,7 +61,7 @@ contract AccessController_Unit_HasAccess is BaseTest {
       sender: caller,
       accessControl: _accessControl,
       typehash: _typehash,
-      params: _params
+      typehashParams: _params
     });
 
     // Expect the hasAccess function to not be called
@@ -101,7 +101,7 @@ contract AccessController_Unit_HasAccess is BaseTest {
       sender: caller,
       accessControl: _accessControl,
       typehash: _typehash,
-      params: _params
+      typehashParams: _params
     });
 
     // Expect the hasAccess function to be called