diff --git a/solidity/contracts/access/OracleAccessController.sol b/solidity/contracts/access/OracleAccessController.sol index cdca1b6..7c97da0 100644 --- a/solidity/contracts/access/OracleAccessController.sol +++ b/solidity/contracts/access/OracleAccessController.sol @@ -28,12 +28,27 @@ abstract contract OracleAccessController is IOracleAccessController, CommonAcces } /// @inheritdoc IOracleAccessController - function setAccessModule(address _accessModule, bool _approved) external { + function approveAccessModule(address _accessModule) external { + _updateAccessModuleApprovalStatus(_accessModule, true); + } + + /// @inheritdoc IOracleAccessController + function revokeAccessModule(address _accessModule) external { + _updateAccessModuleApprovalStatus(_accessModule, false); + } + + /** + * @notice Sets the approval of an access module address. + * + * @param _accessModule The address of the access module + * @param _approved True to approve, false otherwise. + */ + function _updateAccessModuleApprovalStatus(address _accessModule, bool _approved) internal { if (isAccessModuleApproved[msg.sender][_accessModule] == _approved) { - revert OracleAccessController_AccessModuleAlreadySet(); + revert OracleAccessController_AccessModuleApprovalStatusAlreadySet(); } isAccessModuleApproved[msg.sender][_accessModule] = _approved; - emit AccessModuleSet(msg.sender, _accessModule, _approved); + emit AccessModuleApprovalStatusUpdated(msg.sender, _accessModule, _approved); } } diff --git a/solidity/interfaces/access/IOracleAccessController.sol b/solidity/interfaces/access/IOracleAccessController.sol index d44df90..38348e6 100644 --- a/solidity/interfaces/access/IOracleAccessController.sol +++ b/solidity/interfaces/access/IOracleAccessController.sol @@ -18,7 +18,7 @@ interface IOracleAccessController is IAccessController { * @param _accessModule The address of the access module * @param _approved True if approved, false otherwise */ - event AccessModuleSet(address indexed _user, address indexed _accessModule, bool _approved); + event AccessModuleApprovalStatusUpdated(address indexed _user, address indexed _accessModule, bool _approved); /*/////////////////////////////////////////////////////////////// ERRORS @@ -30,9 +30,9 @@ interface IOracleAccessController is IAccessController { error OracleAccessController_AccessModuleNotApproved(); /** - * @notice Thrown when the access module is already set + * @notice Thrown when the access module approval status is already set */ - error OracleAccessController_AccessModuleAlreadySet(); + error OracleAccessController_AccessModuleApprovalStatusAlreadySet(); /*/////////////////////////////////////////////////////////////// VARIABLES @@ -52,10 +52,16 @@ interface IOracleAccessController is IAccessController { //////////////////////////////////////////////////////////////*/ /** - * @notice Sets the approval of an access module address. + * @notice Approves an access module address * - * @param _accessModule The address of the access module - * @param _approved True to approve, false otherwise. + * @param _accessModule The address of the access module to approve + */ + function approveAccessModule(address _accessModule) external; + + /** + * @notice Revokes approval of an access module address + * + * @param _accessModule The address of the access module to revoke */ - function setAccessModule(address _accessModule, bool _approved) external; + function revokeAccessModule(address _accessModule) external; } diff --git a/solidity/test/integration/IntegrationBase.sol b/solidity/test/integration/IntegrationBase.sol index 701ded5..f9cd419 100644 --- a/solidity/test/integration/IntegrationBase.sol +++ b/solidity/test/integration/IntegrationBase.sol @@ -171,7 +171,7 @@ contract IntegrationBase is TestConstants, Helpers { address[] memory _allowedCallers = new address[](1); _allowedCallers[0] = _caller; _accessModule.setHasAccess(_allowedCallers); - oracle.setAccessModule(address(_accessModule), true); + oracle.approveAccessModule(address(_accessModule)); vm.stopPrank(); } } diff --git a/solidity/test/integration/ResponseProposal.t.sol b/solidity/test/integration/ResponseProposal.t.sol index 77f37fb..f35a70e 100644 --- a/solidity/test/integration/ResponseProposal.t.sol +++ b/solidity/test/integration/ResponseProposal.t.sol @@ -24,15 +24,17 @@ contract Integration_ResponseProposal is IntegrationBase { address _eoaAccessModule = makeAddr('eoaAccessModule'); vm.prank(requester); - oracle.setAccessModule(_eoaAccessModule, true); + oracle.approveAccessModule(_eoaAccessModule); // Revert access module already set vm.expectRevert( - abi.encodeWithSelector(IOracleAccessController.OracleAccessController_AccessModuleAlreadySet.selector) + abi.encodeWithSelector( + IOracleAccessController.OracleAccessController_AccessModuleApprovalStatusAlreadySet.selector + ) ); vm.prank(requester); - oracle.setAccessModule(_eoaAccessModule, true); + oracle.approveAccessModule(_eoaAccessModule); vm.startPrank(caller); diff --git a/solidity/test/unit/Oracle.t.sol b/solidity/test/unit/Oracle.t.sol index 6034398..ad68db0 100644 --- a/solidity/test/unit/Oracle.t.sol +++ b/solidity/test/unit/Oracle.t.sol @@ -74,7 +74,7 @@ contract MockOracle is Oracle { _responseIds[_requestId] = abi.encodePacked(_responseIds[_requestId], _responseId); } - function mock_setAccessModuleApproved(address _user, address _accessModule, bool _approved) external { + function mock_setAccessModuleApprovalStatus(address _user, address _accessModule, bool _approved) external { isAccessModuleApproved[_user][_accessModule] = _approved; } } @@ -105,7 +105,7 @@ contract BaseTest is Test, Helpers { event DisputeEscalated(address indexed _caller, bytes32 indexed _disputeId, IOracle.Dispute _dispute); event DisputeStatusUpdated(bytes32 indexed _disputeId, IOracle.Dispute _dispute, IOracle.DisputeStatus _status); event DisputeResolved(bytes32 indexed _disputeId, IOracle.Dispute _dispute); - event AccessModuleSet(address indexed _user, address indexed _accessModule, bool _approved); + event AccessModuleApprovalStatusUpdated(address indexed _user, address indexed _accessModule, bool _approved); address public anyone = makeAddr('anyone'); @@ -140,7 +140,7 @@ contract Oracle_Unit_CreateRequest is BaseTest { modifier happyPath() { mockAccessControl.user = requester; vm.startPrank(requester); - oracle.mock_setAccessModuleApproved(requester, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(requester, address(accessModule), true); _; } /** @@ -282,7 +282,7 @@ contract Oracle_Unit_CreateRequests is BaseTest { IAccessController.AccessControl[] memory _accessControls = new IAccessController.AccessControl[](_requestsAmount); vm.startPrank(requester); - oracle.mock_setAccessModuleApproved(requester, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(requester, address(accessModule), true); // Generate requests batch for (uint256 _i = 0; _i < _requestsAmount; _i++) { @@ -366,7 +366,7 @@ contract Oracle_Unit_CreateRequests is BaseTest { } vm.startPrank(requester); - oracle.mock_setAccessModuleApproved(requester, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(requester, address(accessModule), true); oracle.createRequests(_requests, _ipfsHashes, _accessControls); uint256 _newNonce = oracle.totalRequestCount(); @@ -455,7 +455,7 @@ contract Oracle_Unit_ProposeResponse is BaseTest { modifier happyPath() { mockAccessControl.user = proposer; vm.startPrank(proposer); - oracle.mock_setAccessModuleApproved(proposer, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(proposer, address(accessModule), true); _; } /** @@ -553,7 +553,7 @@ contract Oracle_Unit_ProposeResponse is BaseTest { oracle.mock_setRequestCreatedAt(_getId(mockRequest), block.timestamp); - oracle.mock_setAccessModuleApproved(_caller, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(_caller, address(accessModule), true); mockAccessControl.user = _caller; // Check: revert? @@ -614,7 +614,7 @@ contract Oracle_Unit_DisputeResponse is BaseTest { modifier happyPath() { mockAccessControl.user = disputer; vm.startPrank(disputer); - oracle.mock_setAccessModuleApproved(disputer, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(disputer, address(accessModule), true); _; } @@ -709,7 +709,7 @@ contract Oracle_Unit_DisputeResponse is BaseTest { function test_disputeResponse_revertIfWrongDisputer(address _caller) public { vm.assume(_caller != disputer); - oracle.mock_setAccessModuleApproved(_caller, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(_caller, address(accessModule), true); mockAccessControl.user = _caller; // Check: revert? @@ -811,7 +811,7 @@ contract Oracle_Unit_UpdateDisputeStatus is BaseTest { // Mock the dispute oracle.mock_setDisputeOf(_responseId, _disputeId); oracle.mock_setDisputeCreatedAt(_disputeId, block.timestamp); - oracle.mock_setAccessModuleApproved(proposer, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(proposer, address(accessModule), true); vm.mockCall(address(accessModule), abi.encodeWithSelector(IAccessModule.hasAccess.selector), abi.encode(true)); @@ -830,7 +830,7 @@ contract Oracle_Unit_UpdateDisputeStatus is BaseTest { bytes32 _disputeId = _getId(mockDispute); oracle.mock_setDisputeCreatedAt(_disputeId, 0); - oracle.mock_setAccessModuleApproved(address(resolutionModule), address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(address(resolutionModule), address(accessModule), true); // Check: revert? vm.expectRevert(abi.encodeWithSelector(IOracle.Oracle_InvalidDispute.selector)); @@ -844,7 +844,7 @@ contract Oracle_Unit_UpdateDisputeStatus is BaseTest { contract Oracle_Unit_ResolveDispute is BaseTest { modifier happyPath() { mockAccessControl.user = address(resolutionModule); - oracle.mock_setAccessModuleApproved(address(resolutionModule), address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(address(resolutionModule), address(accessModule), true); vm.startPrank(address(resolutionModule)); _; } @@ -1022,7 +1022,7 @@ contract Oracle_Unit_Finalize is BaseTest { modifier happyPath() { mockAccessControl.user = address(requester); - oracle.mock_setAccessModuleApproved(mockAccessControl.user, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(mockAccessControl.user, address(accessModule), true); vm.startPrank(address(requester)); _; } @@ -1040,7 +1040,7 @@ contract Oracle_Unit_Finalize is BaseTest { bytes32 _responseId = _getId(mockResponse); mockAccessControl.user = _caller; - oracle.mock_setAccessModuleApproved(mockAccessControl.user, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(mockAccessControl.user, address(accessModule), true); oracle.mock_addResponseId(_requestId, _responseId); oracle.mock_setRequestCreatedAt(_requestId, block.timestamp); @@ -1183,7 +1183,7 @@ contract Oracle_Unit_Finalize is BaseTest { oracle.mock_setRequestCreatedAt(_requestId, block.timestamp); mockResponse.requestId = bytes32(0); mockAccessControl.user = _caller; - oracle.mock_setAccessModuleApproved(mockAccessControl.user, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(mockAccessControl.user, address(accessModule), true); // Create mock request and store it bytes memory _calldata = abi.encodeCall(IModule.finalizeRequest, (mockRequest, mockResponse, _caller)); @@ -1227,7 +1227,7 @@ contract Oracle_Unit_Finalize is BaseTest { oracle.mock_setRequestCreatedAt(_requestId, block.timestamp); mockAccessControl.user = _caller; - oracle.mock_setAccessModuleApproved(mockAccessControl.user, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(mockAccessControl.user, address(accessModule), true); IOracle.DisputeStatus _disputeStatus = IOracle.DisputeStatus(_status); @@ -1268,7 +1268,7 @@ contract Oracle_Unit_Finalize is BaseTest { oracle.mock_setRequestCreatedAt(_getId(mockRequest), block.timestamp); mockAccessControl.user = _caller; - oracle.mock_setAccessModuleApproved(mockAccessControl.user, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(mockAccessControl.user, address(accessModule), true); // Test: finalize a finalized request vm.expectRevert(abi.encodeWithSelector(IOracle.Oracle_AlreadyFinalized.selector, _requestId)); @@ -1304,7 +1304,7 @@ contract Oracle_Unit_EscalateDispute is BaseTest { modifier happyPath(address _caller) { mockAccessControl.user = _caller; vm.startPrank(_caller); - oracle.mock_setAccessModuleApproved(_caller, address(accessModule), true); + oracle.mock_setAccessModuleApprovalStatus(_caller, address(accessModule), true); _; } /** @@ -1427,18 +1427,18 @@ contract Oracle_Unit_EscalateDispute is BaseTest { } contract Oracle_Unit_SetAccessModule is BaseTest { - function test_revertIfAccessModuleAlreadySet(address _accessModule, bool _approved) public { - vm.assume(_approved == true); - + function test_revertIfAccessModuleApprovalIsAlreadySet_approved(address _accessModule) public { // Set the access module - oracle.mock_setAccessModuleApproved(address(this), _accessModule, _approved); + oracle.mock_setAccessModuleApprovalStatus(address(this), _accessModule, true); vm.expectRevert( - abi.encodeWithSelector(IOracleAccessController.OracleAccessController_AccessModuleAlreadySet.selector) + abi.encodeWithSelector( + IOracleAccessController.OracleAccessController_AccessModuleApprovalStatusAlreadySet.selector + ) ); // Test: set the access module - oracle.setAccessModule(address(_accessModule), _approved); + oracle.approveAccessModule(address(_accessModule)); } function test_setAccessModuleTrue(address _accessModule, bool _approved) public { @@ -1448,10 +1448,10 @@ contract Oracle_Unit_SetAccessModule is BaseTest { // Check: emits AccessModuleSet event? _expectEmit(address(oracle)); - emit AccessModuleSet(anyone, address(_accessModule), _approved); + emit AccessModuleApprovalStatusUpdated(anyone, address(_accessModule), _approved); // Test: set the access module - oracle.setAccessModule(address(_accessModule), _approved); + oracle.approveAccessModule(address(_accessModule)); // Check: correct access module set? assertEq(oracle.isAccessModuleApproved(anyone, address(_accessModule)), _approved); diff --git a/solidity/test/unit/OracleAccessController.t.sol b/solidity/test/unit/OracleAccessController.t.sol index 528e82f..88e4529 100644 --- a/solidity/test/unit/OracleAccessController.t.sol +++ b/solidity/test/unit/OracleAccessController.t.sol @@ -17,7 +17,7 @@ contract MockOracleAccessController is OracleAccessController { AccessControl memory _accessControl ) public hasAccess(_accessModule, _typehash, _params, _accessControl) {} - function setAccessModuleForTest(address _user, address _accessModule, bool _approved) public { + function setAccessModuleApprovalForTest(address _user, address _accessModule, bool _approved) public { isAccessModuleApproved[_user][_accessModule] = _approved; } } @@ -32,7 +32,7 @@ contract BaseTest is Test, Helpers { address public caller; // Events - event AccessModuleSet(address indexed user, address indexed accessModule, bool approved); + event AccessModuleApprovalStatusUpdated(address indexed user, address indexed accessModule, bool approved); /** * @notice Deploy the access controller @@ -54,7 +54,7 @@ contract OracleAccessController_Unit_HasAccess is BaseTest { vm.assume(_accessModule != address(0) && _accessModule != caller); vm.assume(_params.length < 32); - oracleAccessController.setAccessModuleForTest(_accessControl.user, _accessModule, true); + oracleAccessController.setAccessModuleApprovalForTest(_accessControl.user, _accessModule, true); vm.startPrank(caller); _; @@ -80,7 +80,7 @@ contract OracleAccessController_Unit_HasAccess is BaseTest { IAccessController.AccessControl memory _accessControl ) public happyPath(_accessModule, _typehash, _params, _accessControl) { // Ensure the access module is not approved - oracleAccessController.setAccessModuleForTest(_accessControl.user, _accessModule, false); + oracleAccessController.setAccessModuleApprovalForTest(_accessControl.user, _accessModule, false); // Expect the revert vm.expectRevert( @@ -148,7 +148,7 @@ contract OracleAccessController_Unit_HasAccess is BaseTest { bytes memory _params, IAccessController.AccessControl memory _accessControl ) public happyPath(_accessModule, _typehash, _params, _accessControl) { - oracleAccessController.setAccessModuleForTest(caller, _accessModule, true); + oracleAccessController.setAccessModuleApprovalForTest(caller, _accessModule, true); _accessControl.user = caller; // We expect the hasAccess function to not be called vm.expectCall( @@ -171,27 +171,58 @@ contract OracleAccessController_Unit_HasAccess is BaseTest { } contract OracleAccessController_Unit_SetAccessModule is BaseTest { - function test_revertIfAccessModuleAlreadySet(address _accessModule, bool _approved) public { + function test_revertIfAccessModuleAlreadyApprove(address _accessModule) public { // Ensure the access module is already set - oracleAccessController.setAccessModuleForTest(caller, _accessModule, _approved); + oracleAccessController.setAccessModuleApprovalForTest(caller, _accessModule, true); // Expect the revert vm.expectRevert( - abi.encodeWithSelector(IOracleAccessController.OracleAccessController_AccessModuleAlreadySet.selector) + abi.encodeWithSelector( + IOracleAccessController.OracleAccessController_AccessModuleApprovalStatusAlreadySet.selector + ) ); // Call the setAccessModule function vm.prank(caller); - oracleAccessController.setAccessModule(_accessModule, _approved); + oracleAccessController.approveAccessModule(_accessModule); } - function test_setAccessModule(address _accessModule) public { + function test_revertIfAccessModuleAlreadyRevoked(address _accessModule) public { + // Ensure the access module is already set + oracleAccessController.setAccessModuleApprovalForTest(caller, _accessModule, false); + + // Expect the revert + vm.expectRevert( + abi.encodeWithSelector( + IOracleAccessController.OracleAccessController_AccessModuleApprovalStatusAlreadySet.selector + ) + ); + + // Call the setAccessModule function + vm.prank(caller); + oracleAccessController.revokeAccessModule(_accessModule); + } + + function test_approveAccessModule(address _accessModule) public { + // Expect the event to be emitted + vm.expectEmit(); + emit AccessModuleApprovalStatusUpdated(caller, _accessModule, true); + + // Call the setAccessModule functi; + vm.prank(caller); + oracleAccessController.approveAccessModule(_accessModule); + } + + function test_revokeAccessModule(address _accessModule) public { + // Ensure the access module is already set + oracleAccessController.setAccessModuleApprovalForTest(caller, _accessModule, true); + // Expect the event to be emitted vm.expectEmit(); - emit AccessModuleSet(caller, _accessModule, true); + emit AccessModuleApprovalStatusUpdated(caller, _accessModule, false); // Call the setAccessModule functi; vm.prank(caller); - oracleAccessController.setAccessModule(_accessModule, true); + oracleAccessController.revokeAccessModule(_accessModule); } }