From 179d9ed06904e56909d172f2b1b3ce037cb4fa1f Mon Sep 17 00:00:00 2001 From: rapiddenis <41779817+rapidddenis@users.noreply.github.com> Date: Thu, 22 Aug 2024 12:09:19 +0000 Subject: [PATCH 1/7] Component lock by owner is independent from services, add missing auth config --- contracts/instance/IInstance.sol | 1 + contracts/instance/Instance.sol | 12 ++- contracts/instance/InstanceAdmin.sol | 16 ++-- contracts/registry/ServiceAuthorizationV3.sol | 88 ++++++++++++++++++- contracts/shared/Component.sol | 9 +- contracts/shared/ComponentService.sol | 21 ++--- contracts/shared/IComponentService.sol | 5 +- 7 files changed, 116 insertions(+), 36 deletions(-) diff --git a/contracts/instance/IInstance.sol b/contracts/instance/IInstance.sol index b7a90e631..30dba5c15 100644 --- a/contracts/instance/IInstance.sol +++ b/contracts/instance/IInstance.sol @@ -48,6 +48,7 @@ interface IInstance is function createTarget(address target, string memory name) external; function setTargetFunctionRole(string memory targetName, bytes4[] calldata selectors, RoleId roleId) external; function setLocked(address target, bool locked) external; + function setLockedFromService(address target, bool locked) external; function setStakingLockingPeriod(Seconds stakeLockingPeriod) external; function setStakingRewardRate(UFixed rewardRate) external; diff --git a/contracts/instance/Instance.sol b/contracts/instance/Instance.sol index dd1d0f9de..7381b2804 100644 --- a/contracts/instance/Instance.sol +++ b/contracts/instance/Instance.sol @@ -164,7 +164,7 @@ contract Instance is } //--- Targets ------------------------------------------------------------// - + // TODO consider restricted() function createTarget(address target, string memory name) external onlyOwner() @@ -189,7 +189,15 @@ contract Instance is external onlyOwner() { - _componentService.setLockedFromInstance(target, locked); + _instanceAdmin.setTargetLocked(target, locked); + } + + function setLockedFromService(address target, bool locked) + external + restricted() + { + // TODO service can not lock any of instance contracts + _instanceAdmin.setTargetLocked(target, locked); } //--- ITransferInterceptor ----------------------------------------------// diff --git a/contracts/instance/InstanceAdmin.sol b/contracts/instance/InstanceAdmin.sol index facae43bd..8b73f6779 100644 --- a/contracts/instance/InstanceAdmin.sol +++ b/contracts/instance/InstanceAdmin.sol @@ -29,6 +29,7 @@ contract InstanceAdmin is uint64 public constant CUSTOM_ROLE_ID_MIN = 10000; // MUST be even error ErrorInstanceAdminCallerNotInstanceOwner(address caller); + error ErrorInstanceAdminCallerNotInstance(address caller); error ErrorInstanceAdminInstanceAlreadyLocked(); error ErrorInstanceAdminNotRegistered(address target); @@ -57,6 +58,13 @@ contract InstanceAdmin is _; } + modifier onlyInstance() { + if(msg.sender != address(_instance)) { + revert ErrorInstanceAdminCallerNotInstance(msg.sender); + } + _; + } + /// @dev Only used for master instance admin. /// Contracts created via constructor come with disabled initializers. constructor( @@ -129,6 +137,8 @@ contract InstanceAdmin is external restricted() { + // TODO check componentInfo exists + // TODO deploy token handler here // checks _checkTargetIsReadyForAuthorization(address(component)); @@ -139,10 +149,6 @@ contract InstanceAdmin is IAuthorization authorization = component.getAuthorization(); _createRoles(authorization); - // TODO cleanup - // FunctionInfo[] memory functions = new FunctionInfo[](2); - // functions[0] = toFunction(TokenHandler.pullToken.selector, "pullToken"); - // functions[1] = toFunction(TokenHandler.pushToken.selector, "pushToken"); // // FIXME: make this a bit nicer and work with IAuthorization. Use a specific role, not public - access to TokenHandler must be restricted // _authorizeTargetFunctions( @@ -210,7 +216,7 @@ contract InstanceAdmin is function setTargetLocked(address target, bool locked) external - restricted() + onlyInstance() { _setTargetClosed(target, locked); } diff --git a/contracts/registry/ServiceAuthorizationV3.sol b/contracts/registry/ServiceAuthorizationV3.sol index f4a8707a7..f0a927d15 100644 --- a/contracts/registry/ServiceAuthorizationV3.sol +++ b/contracts/registry/ServiceAuthorizationV3.sol @@ -7,10 +7,14 @@ import { import {IAccess} from "../authorization/IAccess.sol"; import {IAccountingService} from "../accounting/IAccountingService.sol"; +import {IApplicationService} from "../product/IApplicationService.sol"; import {IBundleService} from "../pool/IBundleService.sol"; import {IClaimService} from "../product/IClaimService.sol"; +import {IComponentService} from "../shared/IComponentService.sol"; import {IDistributionService} from "../distribution/IDistributionService.sol"; import {IInstanceService} from "../instance/IInstanceService.sol"; +import {IOracleService} from "../oracle/IOracleService.sol"; +import {IPolicyService} from "../product/IPolicyService.sol"; import {IPoolService} from "../pool/IPoolService.sol"; import {IStakingService} from "../staking/IStakingService.sol"; import {IRegistryService} from "./IRegistryService.sol"; @@ -62,6 +66,9 @@ contract ServiceAuthorizationV3 _setupDistributionServiceAuthorization(); _setupPoolServiceAuthorization(); _setupBundleServiceAuthorization(); + _setupOracleServiceAuthorization(); + _setupApplicationServiceAuthorization(); + _setupPolicyServiceAuthorization(); } @@ -143,9 +150,6 @@ contract ServiceAuthorizationV3 _authorize(functions, IAccountingService.increaseBundleBalance.selector, "increaseBundleBalance"); _authorize(functions, IAccountingService.decreaseBundleBalance.selector, "decreaseBundleBalance"); - functions = _authorizeForService(ACCOUNTING(), POOL()); - _authorize(functions, IAccountingService.decreaseBundleBalanceForPool.selector, "decreaseBundleBalanceForPool"); - functions = _authorizeForService(ACCOUNTING(), COMPONENT()); _authorize(functions, IAccountingService.decreaseComponentFees.selector, "decreaseComponentFees"); @@ -171,6 +175,20 @@ contract ServiceAuthorizationV3 function _setupComponentServiceAuthorization() internal { + IAccess.FunctionInfo[] storage functions; + + functions = _authorizeForService(COMPONENT(), ALL()); + _authorize(functions, IComponentService.registerComponent.selector, "registerComponent"); + _authorize(functions, IComponentService.approveTokenHandler.selector, "approveTokenHandler"); + _authorize(functions, IComponentService.approveStakingTokenHandler.selector, "approveStakingTokenHandler"); + _authorize(functions, IComponentService.setWallet.selector, "setWallet"); + _authorize(functions, IComponentService.setComponentLocked.selector, "setComponentLocked"); + _authorize(functions, IComponentService.withdrawFees.selector, "withdrawFees"); + _authorize(functions, IComponentService.registerProduct.selector, "registerProduct"); + _authorize(functions, IComponentService.setProductFees.selector, "setProductFees"); + _authorize(functions, IComponentService.setDistributionFees.selector, "setDistributionFees"); + _authorize(functions, IComponentService.setPoolFees.selector, "setPoolFees"); + } /// @dev Distribution service function authorization. @@ -191,6 +209,14 @@ contract ServiceAuthorizationV3 IAccess.FunctionInfo[] storage functions; functions = _authorizeForService(CLAIM(), ALL()); _authorize(functions, IClaimService.submit.selector, "submit"); + _authorize(functions, IClaimService.confirm.selector, "confirm"); + _authorize(functions, IClaimService.decline.selector, "decline"); + _authorize(functions, IClaimService.revoke.selector, "revoke"); + _authorize(functions, IClaimService.close.selector, "close"); + _authorize(functions, IClaimService.createPayoutForBeneficiary.selector, "createPayoutForBeneficiary"); + _authorize(functions, IClaimService.createPayout.selector, "createPayout"); + _authorize(functions, IClaimService.processPayout.selector, "processPayout"); + _authorize(functions, IClaimService.cancelPayout.selector, "cancelPayout"); } /// @dev Distribution service function authorization. @@ -201,6 +227,13 @@ contract ServiceAuthorizationV3 functions = _authorizeForService(DISTRIBUTION(), POLICY()); _authorize(functions, IDistributionService.processSale.selector, "processSale"); _authorize(functions, IDistributionService.processReferral.selector, "processReferral"); + + functions = _authorizeForService(DISTRIBUTION(), ALL()); + _authorize(functions, IDistributionService.createDistributorType.selector, "createDistributorType"); + _authorize(functions, IDistributionService.createDistributor.selector, "createDistributor"); + _authorize(functions, IDistributionService.changeDistributorType.selector, "changeDistributorType"); + _authorize(functions, IDistributionService.createReferral.selector, "createReferral"); + _authorize(functions, IDistributionService.withdrawCommission.selector, "withdrawCommission"); } @@ -219,7 +252,13 @@ contract ServiceAuthorizationV3 _authorize(functions, IPoolService.processPayout.selector, "processPayout"); functions = _authorizeForService(POOL(), ALL()); - _authorize(functions, IPoolService.withdrawBundleFees.selector, "withdrawBundleFees"); + _authorize(functions, IPoolService.setMaxBalanceAmount.selector, "setMaxBalanceAmount"); + _authorize(functions, IPoolService.closeBundle.selector, "closeBundle"); + _authorize(functions, IPoolService.processFundedClaim.selector, "processFundedClaim"); + _authorize(functions, IPoolService.stake.selector, "stake"); + _authorize(functions, IPoolService.unstake.selector, "unstake"); + _authorize(functions, IPoolService.fundPoolWallet.selector, "fundPoolWallet"); + _authorize(functions, IPoolService.defundPoolWallet.selector, "defundPoolWallet"); } @@ -242,6 +281,47 @@ contract ServiceAuthorizationV3 _authorize(functions, IBundleService.lock.selector, "lock"); _authorize(functions, IBundleService.unlock.selector, "unlock"); _authorize(functions, IBundleService.setFee.selector, "setFee"); + _authorize(functions, IBundleService.withdrawBundleFees.selector, "withdrawBundleFees"); + } + + function _setupOracleServiceAuthorization() + internal + { + IAccess.FunctionInfo[] storage functions; + + functions = _authorizeForService(ORACLE(), ALL()); + _authorize(functions, IOracleService.request.selector, "request"); + _authorize(functions, IOracleService.respond.selector, "respond"); + _authorize(functions, IOracleService.resend.selector, "resend"); + _authorize(functions, IOracleService.cancel.selector, "cancel"); + } + + function _setupApplicationServiceAuthorization() + internal + { + IAccess.FunctionInfo[] storage functions; + + functions = _authorizeForService(APPLICATION(), ALL()); + _authorize(functions, IApplicationService.create.selector, "create"); + _authorize(functions, IApplicationService.renew.selector, "renew"); + _authorize(functions, IApplicationService.adjust.selector, "adjust"); + _authorize(functions, IApplicationService.revoke.selector, "revoke"); + } + + function _setupPolicyServiceAuthorization() + internal + { + IAccess.FunctionInfo[] storage functions; + + functions = _authorizeForService(POLICY(), ALL()); + _authorize(functions, IPolicyService.decline.selector, "decline"); + _authorize(functions, IPolicyService.createPolicy.selector, "createPolicy"); + _authorize(functions, IPolicyService.collectPremium.selector, "collectPremium"); + _authorize(functions, IPolicyService.activate.selector, "activate"); + _authorize(functions, IPolicyService.expire.selector, "expire"); + _authorize(functions, IPolicyService.expireFromService.selector, "expireFromService"); + _authorize(functions, IPolicyService.close.selector, "close"); + } } diff --git a/contracts/shared/Component.sol b/contracts/shared/Component.sol index 3d1686fe5..c5b2f8ca0 100644 --- a/contracts/shared/Component.sol +++ b/contracts/shared/Component.sol @@ -4,14 +4,13 @@ pragma solidity ^0.8.20; import {IAccessManaged} from "@openzeppelin/contracts/access/manager/IAccessManaged.sol"; import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; +import {Amount} from "../type/Amount.sol"; +import {ContractLib} from "./ContractLib.sol"; import {IComponent} from "./IComponent.sol"; import {IComponents} from "../instance/module/IComponents.sol"; import {IComponentService} from "./IComponentService.sol"; import {IRegistry} from "../registry/IRegistry.sol"; import {IRelease} from "../registry/IRelease.sol"; - -import {Amount, AmountLib} from "../type/Amount.sol"; -import {ContractLib} from "./ContractLib.sol"; import {NftId} from "../type/NftId.sol"; import {ObjectType, COMPONENT, STAKING} from "../type/ObjectType.sol"; import {Registerable} from "../shared/Registerable.sol"; @@ -111,7 +110,7 @@ abstract contract Component is /// override internal function _nftTransferFrom to implement custom behaviour function nftTransferFrom(address from, address to, uint256 tokenId, address operator) external - onlyChainNft() + onlyChainNft { _nftTransferFrom(from, to, tokenId, operator); } @@ -202,7 +201,7 @@ abstract contract Component is internal virtual { - _getComponentStorage()._componentService.setLockedFromComponent(address(this), locked); + _getComponentStorage()._componentService.setComponentLocked(address(this), locked); } diff --git a/contracts/shared/ComponentService.sol b/contracts/shared/ComponentService.sol index 2d5251fd3..d3f575a96 100644 --- a/contracts/shared/ComponentService.sol +++ b/contracts/shared/ComponentService.sol @@ -144,25 +144,13 @@ contract ComponentService is } /// @inheritdoc IComponentService - function setLockedFromInstance(address componentAddress, bool locked) + function setComponentLocked(address componentAddress, bool locked) external virtual - onlyInstance() + restricted() { - address instanceAddress = msg.sender; - // NftId instanceNftId = getRegistry().getNftIdForAddress(msg.sender); - IInstance instance = IInstance(instanceAddress); - _setLocked(instance.getInstanceAdmin(), componentAddress, locked); - } - - /// @inheritdoc IComponentService - function setLockedFromComponent(address componentAddress, bool locked) - external - virtual - onlyComponent(msg.sender) - { - (, IInstance instance) = _getAndVerifyComponent(COMPONENT(), false); - _setLocked(instance.getInstanceAdmin(), componentAddress, locked); + (IInstance instance) = _getAndVerifyActiveComponent(COMPONENT()); + instance.setLockedFromService(componentAddress, locked); } /// @inheritdoc IComponentService @@ -198,6 +186,7 @@ contract ComponentService is componentNftId, withdrawnAmount); + // transfer amount to component owner address componentOwner = getRegistry().ownerOf(componentNftId); TokenHandler tokenHandler = instanceReader.getTokenHandler(componentNftId); emit LogComponentServiceComponentFeesWithdrawn( diff --git a/contracts/shared/IComponentService.sol b/contracts/shared/IComponentService.sol index 85f215c14..ecea46459 100644 --- a/contracts/shared/IComponentService.sol +++ b/contracts/shared/IComponentService.sol @@ -70,11 +70,8 @@ interface IComponentService is /// @dev Sets the components associated wallet address function setWallet(address newWallet) external; - /// @dev Locks/Unlocks the given component - call from instanceService - function setLockedFromInstance(address componentAddress, bool locked) external; - /// @dev Locks/Unlocks the given component - call from component - function setLockedFromComponent(address componentAddress, bool locked) external; + function setComponentLocked(address componentAddress, bool locked) external; /// @dev Withdraw fees from the distribution component. Only component owner is allowed to withdraw fees. /// @param withdrawAmount the amount to withdraw From fef903ff55c2f2456286bc2c252bcdda5f290f61 Mon Sep 17 00:00:00 2001 From: rapiddenis <41779817+rapidddenis@users.noreply.github.com> Date: Thu, 22 Aug 2024 12:14:38 +0000 Subject: [PATCH 2/7] fix --- contracts/registry/ServiceAuthorizationV3.sol | 5 ++--- contracts/shared/ComponentService.sol | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/contracts/registry/ServiceAuthorizationV3.sol b/contracts/registry/ServiceAuthorizationV3.sol index f0a927d15..b0b32b50c 100644 --- a/contracts/registry/ServiceAuthorizationV3.sol +++ b/contracts/registry/ServiceAuthorizationV3.sol @@ -180,7 +180,6 @@ contract ServiceAuthorizationV3 functions = _authorizeForService(COMPONENT(), ALL()); _authorize(functions, IComponentService.registerComponent.selector, "registerComponent"); _authorize(functions, IComponentService.approveTokenHandler.selector, "approveTokenHandler"); - _authorize(functions, IComponentService.approveStakingTokenHandler.selector, "approveStakingTokenHandler"); _authorize(functions, IComponentService.setWallet.selector, "setWallet"); _authorize(functions, IComponentService.setComponentLocked.selector, "setComponentLocked"); _authorize(functions, IComponentService.withdrawFees.selector, "withdrawFees"); @@ -259,6 +258,7 @@ contract ServiceAuthorizationV3 _authorize(functions, IPoolService.unstake.selector, "unstake"); _authorize(functions, IPoolService.fundPoolWallet.selector, "fundPoolWallet"); _authorize(functions, IPoolService.defundPoolWallet.selector, "defundPoolWallet"); + _authorize(functions, IPoolService.withdrawBundleFees.selector, "withdrawBundleFees"); } @@ -281,7 +281,6 @@ contract ServiceAuthorizationV3 _authorize(functions, IBundleService.lock.selector, "lock"); _authorize(functions, IBundleService.unlock.selector, "unlock"); _authorize(functions, IBundleService.setFee.selector, "setFee"); - _authorize(functions, IBundleService.withdrawBundleFees.selector, "withdrawBundleFees"); } function _setupOracleServiceAuthorization() @@ -319,7 +318,7 @@ contract ServiceAuthorizationV3 _authorize(functions, IPolicyService.collectPremium.selector, "collectPremium"); _authorize(functions, IPolicyService.activate.selector, "activate"); _authorize(functions, IPolicyService.expire.selector, "expire"); - _authorize(functions, IPolicyService.expireFromService.selector, "expireFromService"); + _authorize(functions, IPolicyService.expirePolicy.selector, "expirePolicy"); _authorize(functions, IPolicyService.close.selector, "close"); } diff --git a/contracts/shared/ComponentService.sol b/contracts/shared/ComponentService.sol index d3f575a96..4819a370a 100644 --- a/contracts/shared/ComponentService.sol +++ b/contracts/shared/ComponentService.sol @@ -149,7 +149,7 @@ contract ComponentService is virtual restricted() { - (IInstance instance) = _getAndVerifyActiveComponent(COMPONENT()); + (,IInstance instance) = _getAndVerifyActiveComponent(COMPONENT()); instance.setLockedFromService(componentAddress, locked); } From bb17bbfe45c676130d332a407c64bc186401deab Mon Sep 17 00:00:00 2001 From: rapiddenis <41779817+rapidddenis@users.noreply.github.com> Date: Thu, 22 Aug 2024 12:57:42 +0000 Subject: [PATCH 3/7] add setLockedFromService tests --- .../instance/InstanceAuthorizationV3.sol | 5 +- contracts/shared/ComponentService.sol | 3 +- test/instance/Instance.t.sol | 108 ++++++++++++++++++ 3 files changed, 114 insertions(+), 2 deletions(-) diff --git a/contracts/instance/InstanceAuthorizationV3.sol b/contracts/instance/InstanceAuthorizationV3.sol index 6fce34962..52cfb468c 100644 --- a/contracts/instance/InstanceAuthorizationV3.sol +++ b/contracts/instance/InstanceAuthorizationV3.sol @@ -110,6 +110,10 @@ contract InstanceAuthorizationV3 // authorize instance service role functions = _authorizeForTarget(INSTANCE_TARGET_NAME, getServiceRole(INSTANCE())); _authorize(functions, Instance.setInstanceReader.selector, "setInstanceReader"); + + // authorize component service role + functions = _authorizeForTarget(INSTANCE_TARGET_NAME, getServiceRole(COMPONENT())); + _authorize(functions, Instance.setLockedFromService.selector, "setLockedFromService"); } @@ -125,7 +129,6 @@ contract InstanceAuthorizationV3 // authorize component service role functions = _authorizeForTarget(INSTANCE_ADMIN_TARGET_NAME, getServiceRole(COMPONENT())); _authorize(functions, InstanceAdmin.initializeComponentAuthorization.selector, "initializeComponentAuthoriz"); - _authorize(functions, InstanceAdmin.setTargetLocked.selector, "setTargetLocked"); } diff --git a/contracts/shared/ComponentService.sol b/contracts/shared/ComponentService.sol index 4819a370a..096f2cd91 100644 --- a/contracts/shared/ComponentService.sol +++ b/contracts/shared/ComponentService.sol @@ -149,7 +149,8 @@ contract ComponentService is virtual restricted() { - (,IInstance instance) = _getAndVerifyActiveComponent(COMPONENT()); + // TODO inactive component can lock/unlock other components? + (, IInstance instance) = _getAndVerifyComponent(COMPONENT(), false); instance.setLockedFromService(componentAddress, locked); } diff --git a/test/instance/Instance.t.sol b/test/instance/Instance.t.sol index 540c3f1ef..f6f40f97c 100644 --- a/test/instance/Instance.t.sol +++ b/test/instance/Instance.t.sol @@ -12,6 +12,7 @@ import {InstanceAdmin} from "../../contracts/instance/InstanceAdmin.sol"; import {InstanceReader} from "../../contracts/instance/InstanceReader.sol"; import {NftId} from "../../contracts/type/NftId.sol"; import {UFixed, UFixedLib} from "../../contracts/type/UFixed.sol"; +import {INftOwnable} from "../../contracts/shared/INftOwnable.sol"; contract TestInstance is GifTest { @@ -19,6 +20,20 @@ contract TestInstance is GifTest { super.setUp(); _prepareProduct(); // also deploys and registers distribution } + + function test_Instance_setLocked_invalidCaller() public { + // GIVEN + Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); + Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); + + vm.startPrank(address(this)); + + // THEN + vm.expectRevert(abi.encodeWithSelector(INftOwnable.ErrorNftOwnableNotOwner.selector, address(this))); + + // WHEN + instance.setLocked(address(distribution), true); + } function test_Instance_setLocked_lock() public { // GIVEN @@ -97,4 +112,97 @@ contract TestInstance is GifTest { instance.setLocked(address(distribution), false); } + + + function test_Instance_setLockedFromService_invalidCaller() public { + // GIVEN + Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); + Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); + + vm.startPrank(address(this)); + + // THEN + vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, address(this))); + + // WHEN + instance.setLockedFromService(address(distribution), true); + } + + function test_Instance_setLockedFromService_lock() public { + // GIVEN + Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); + Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); + + vm.startPrank(address(componentService)); + instance.setLockedFromService(address(distribution), true); + vm.stopPrank(); + + vm.startPrank(distributionOwner); + + // THEN + vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, address(distributionOwner))); + + // WHEN + distribution.setFees(newMinDistributionOwnerFee, newDistributionFee); + } + + function test_Instance_setLockedFromService_unlock() public { + // GIVEN + Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); + Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); + + vm.startPrank(address(componentService)); + instance.setLockedFromService(address(distribution), true); + instance.setLockedFromService(address(distribution), false); + vm.stopPrank(); + + vm.startPrank(distributionOwner); + + // THEN - WHEN - make sure no revert + distribution.setFees(newMinDistributionOwnerFee, newDistributionFee); + } + + function test_Instance_setLockedFromService_invalidTarget() public { + // GIVEN + Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); + Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); + + vm.startPrank(address(componentService)); + + // THEN + vm.expectRevert(abi.encodeWithSelector(IAccessAdmin.ErrorTargetUnknown.selector, address(this))); + + // WHEN + instance.setLockedFromService(address(this), true); + } + + function test_Instance_setLockedFromService_alreadyLocked() public { + // GIVEN + Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); + Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); + + vm.startPrank(address(componentService)); + instance.setLockedFromService(address(distribution), true); + + // THEN + vm.expectRevert(abi.encodeWithSelector(IAccessAdmin.ErrorTargetAlreadyLocked.selector, address(distribution), true)); + + // WHEN + instance.setLockedFromService(address(distribution), true); + } + + function test_Instance_setLockedFromService_alreadyUnlocked() public { + // GIVEN + Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); + Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); + + vm.startPrank(address(componentService)); + + // THEN + vm.expectRevert(abi.encodeWithSelector(IAccessAdmin.ErrorTargetAlreadyLocked.selector, address(distribution), false)); + + // WHEN + instance.setLockedFromService(address(distribution), false); + } + } From ef1a082e8b88a50372095d814e0fbb376fdf9987 Mon Sep 17 00:00:00 2001 From: Matthias Zimmermann Date: Sun, 25 Aug 2024 23:47:56 +0000 Subject: [PATCH 4/7] refactor registry authorization --- contracts/authorization/AccessAdmin.sol | 278 +++++++++++++----- .../authorization/AccessManagerCloneable.sol | 68 +++-- contracts/authorization/Authorization.sol | 92 ++++-- contracts/authorization/IAccessAdmin.sol | 88 ++++-- contracts/authorization/IAuthorization.sol | 6 + .../BasicDistributionAuthorization.sol | 2 +- contracts/instance/IInstance.sol | 48 ++- contracts/instance/IInstanceService.sol | 15 +- contracts/instance/Instance.sol | 78 +++-- contracts/instance/InstanceAdmin.sol | 232 ++++++++------- .../instance/InstanceAuthorizationV3.sol | 67 +++-- contracts/instance/InstanceService.sol | 181 ++++++++---- contracts/oracle/BasicOracleAuthorization.sol | 2 +- contracts/pool/BasicPoolAuthorization.sol | 2 +- .../product/BasicProductAuthorization.sol | 2 +- contracts/registry/RegistryAdmin.sol | 197 ++++++++++--- contracts/registry/RegistryAuthorization.sol | 266 +++++++++++++++++ contracts/registry/ReleaseAdmin.sol | 41 +-- contracts/registry/ReleaseRegistry.sol | 24 +- contracts/registry/ServiceAuthorizationV3.sol | 5 +- contracts/shared/Component.sol | 5 +- contracts/shared/ComponentService.sol | 41 +-- contracts/shared/ContractLib.sol | 10 + contracts/shared/IComponentService.sol | 7 +- contracts/shared/InitializableERC165.sol | 10 +- contracts/shared/NftOwnable.sol | 2 +- contracts/shared/Registerable.sol | 5 +- contracts/shared/TokenHandler.sol | 9 +- contracts/staking/IStaking.sol | 4 + contracts/type/ObjectType.sol | 19 +- contracts/type/Version.sol | 39 +++ scripts/libs/registry.ts | 1 + test/authorization/AccessAdmin.t.sol | 55 ++-- .../authorization/AccessAdminManageMock.t.sol | 2 +- .../AccessManagerCloneable.t.sol | 202 ++----------- .../AccessManagerCloneableEx.t.sol | 203 +++++++++++++ test/authorization/InstanceAdmin.t.sol | 56 ---- test/authorization/RegistryAdminEx.sol | 14 +- test/base/GifDeployer.sol | 36 +-- test/base/GifTest.sol | 44 ++- test/instance/Instance.t.sol | 221 +++++++------- test/instance/InstanceAuthorization.t.sol | 38 +++ test/instance/InstanceReader.t.sol | 106 ++++++- test/instance/MasterInstance.t.sol | 29 ++ test/instance/Test.t.sol | 100 ------- test/instance/TestInstanceService.t.sol | 100 ------- test/staking/Staking.t.sol | 2 + 47 files changed, 1935 insertions(+), 1119 deletions(-) create mode 100644 contracts/registry/RegistryAuthorization.sol create mode 100644 test/authorization/AccessManagerCloneableEx.t.sol delete mode 100644 test/authorization/InstanceAdmin.t.sol create mode 100644 test/instance/InstanceAuthorization.t.sol create mode 100644 test/instance/MasterInstance.t.sol delete mode 100644 test/instance/Test.t.sol delete mode 100644 test/instance/TestInstanceService.t.sol diff --git a/contracts/authorization/AccessAdmin.sol b/contracts/authorization/AccessAdmin.sol index 705dcdab6..116951408 100644 --- a/contracts/authorization/AccessAdmin.sol +++ b/contracts/authorization/AccessAdmin.sol @@ -5,10 +5,14 @@ import {AccessManagedUpgradeable} from "@openzeppelin/contracts-upgradeable/acce import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol"; -import {AccessManagerCloneable} from "./AccessManagerCloneable.sol"; -import {ContractLib} from "../shared/ContractLib.sol"; import {IAccessAdmin} from "./IAccessAdmin.sol"; +import {IAuthorization} from "./IAuthorization.sol"; import {IRegistry} from "../registry/IRegistry.sol"; + +import {AccessManagerCloneable} from "./AccessManagerCloneable.sol"; +import {ContractLib} from "../shared/ContractLib.sol"; +import {NftId, NftIdLib} from "../type/NftId.sol"; +import {ObjectType} from "../type/ObjectType.sol"; import {RoleId, RoleIdLib, ADMIN_ROLE, PUBLIC_ROLE} from "../type/RoleId.sol"; import {Selector, SelectorLib, SelectorSetLib} from "../type/Selector.sol"; import {Str, StrLib} from "../type/String.sol"; @@ -34,13 +38,23 @@ contract AccessAdmin is string public constant ADMIN_ROLE_NAME = "AdminRole"; string public constant PUBLIC_ROLE_NAME = "PublicRole"; - /// @dev the OpenZeppelin access manager driving the access admin contract + /// @dev admin name used for logging only + string internal _adminName; + + /// @dev the access manager driving the access admin contract + /// hold link to registry and release version AccessManagerCloneable internal _authority; + /// @dev the authorization contract used for initial access control + IAuthorization internal _authorization; + /// @dev stores the deployer address and allows to create initializers /// that are restricted to the deployer address. address internal _deployer; + /// @dev the linked NFT ID + NftId internal _linkedNftId; + /// @dev store role info per role id mapping(RoleId roleId => RoleInfo info) internal _roleInfo; @@ -79,7 +93,7 @@ contract AccessAdmin is } if (msg.sender != _deployer) { - revert ErrorNotDeployer(); + revert ErrorAccessAdminNotDeployer(); } _; } @@ -88,14 +102,14 @@ contract AccessAdmin is _checkRoleExists(roleId, false); if (!hasAdminRole(msg.sender, roleId)) { - revert ErrorNotAdminOfRole(_roleInfo[roleId].adminRoleId); + revert ErrorAccessAdminNotAdminOfRole(_roleInfo[roleId].adminRoleId); } _; } modifier onlyRoleMember(RoleId roleId) { if (!hasRole(msg.sender, roleId)) { - revert ErrorNotRoleOwner(roleId); + revert ErrorAccessAdminNotRoleOwner(roleId); } _; } @@ -124,34 +138,147 @@ contract AccessAdmin is /// @dev Initializes this admin with the provided accessManager (and authorization specification). /// Internally initializes access manager with this admin and creates basic role setup. function initialize( - AccessManagerCloneable authority + address authority, + string memory adminName ) public initializer() { - __AccessAdmin_init(authority); + __AccessAdmin_init(authority, adminName); } function __AccessAdmin_init( - AccessManagerCloneable authority + address authority, + string memory adminName ) internal onlyInitializing() - onlyDeployer() // initializes deployer if not initialized yet + onlyDeployer() { - authority.initialize(address(this)); + // checks + // check authority is contract + if (!ContractLib.isContract(authority)) { + revert ErrorAccessAdminAuthorityNotContract(authority); + } + + // check name not empty + if (bytes(adminName).length == 0) { + revert ErrorAccessAdminAccessManagerEmptyName(); + } + + _authority = AccessManagerCloneable(authority); + _authority.initialize(address(this)); + + // delayed additional check for authority after its initialization + if (!ContractLib.isAuthority(authority)) { + revert ErrorAccessAdminAccessManagerNotAccessManager(authority); + } + // effects // set and initialize this access manager contract as // the admin (ADMIN_ROLE) of the provided authority - __AccessManaged_init(address(authority)); - _authority = authority; + __AccessManaged_init(authority); + + // set name for logging + _adminName = adminName; + + // set initial linked NFT ID to zero + _linkedNftId = NftIdLib.zero(); // create admin and public roles _initializeAdminAndPublicRoles(); + } + + + /// @dev check if target exists and reverts if it doesn't + function _checkTarget( + address target + ) + internal + view + { + // check not yet created + if (!targetExists(target)) { + revert ErrorAccessAdminTargetNotCreated(target); + } + } - // additional use case specific initialization - _initializeCustom(); + function _checkIsRegistered( + address registry, + address target, + ObjectType expectedType + ) + internal + view + { + ObjectType tagetType = IRegistry(registry).getObjectInfo(target).objectType; + if (tagetType.eqz()) { + revert ErrorAccessAdminNotRegistered(target); + } + + if (tagetType != expectedType) { + revert ErrorAccessAdminTargetTypeMismatch(target, expectedType, tagetType); + } + } + + /// @dev check if target exists and and has expected type, reverts if it doesn't + function _checkTargetWithType( + address target, + ObjectType expectedType + ) + internal + view + { + // check target exists + _checkTarget(target); + + // check target type + ObjectType tagetType = _authority.getRegistry().getObjectInfo(target).objectType; + if (tagetType != expectedType) { + revert ErrorAccessAdminTargetTypeMismatch(target, expectedType, tagetType); + } + + // target release is checked during component/instance registration + } + + + function _checkAuthorization( + address authorization, + ObjectType expectedDomain, + VersionPart expectedRelease, + bool checkAlreadyInitialized + ) + internal + view + { + // checks + // check not yet initialized + if (checkAlreadyInitialized && address(_authorization) != address(0)) { + revert ErrorAccessAdminAlreadyInitialized(address(_authorization)); + } + + // check contract type matches + if (!ContractLib.supportsInterface(authorization, type(IAuthorization).interfaceId)) { + revert ErrorAccessAdminNotAuthorization(authorization); + } + + // check domain matches + ObjectType domain = IAuthorization(authorization).getDomain(); + if (domain != expectedDomain) { + revert ErrorAccessAdminDomainMismatch(authorization, expectedDomain, domain); + } + + // check release matches + VersionPart authorizationRelease = IAuthorization(authorization).getRelease(); + if (authorizationRelease != expectedRelease) { + revert ErrorAccessAdminReleaseMismatch(authorization, getRelease(), authorizationRelease); + } + } + + + function getRelease() public view virtual returns (VersionPart release) { + return _authority.getRelease(); } @@ -160,11 +287,29 @@ contract AccessAdmin is } - function getRelease() public view returns (VersionPart release) { - return _authority.getRelease(); + function getLinkedNftId() external view returns (NftId linkedNftId) { + return _linkedNftId; + } + + + function getLinkedOwner() external view returns (address linkedOwner) { + return getRegistry().ownerOf(_linkedNftId); } + + function isLocked() public view returns (bool locked) { + return _authority.isLocked(); + } + + function _linkToNftOwnable(address registerable) internal { + if (!getRegistry().isRegistered(registerable)) { + revert ErrorAccessAdminNotRegistered(registerable); + } + + _linkedNftId = getRegistry().getNftIdForAddress(registerable); + } + function _initializeAdminAndPublicRoles() internal virtual @@ -194,13 +339,6 @@ contract AccessAdmin is name: PUBLIC_ROLE_NAME})); } - - function _initializeCustom() - internal - virtual - onlyInitializing() - { } - //--- view functions for roles ------------------------------------------// function roles() external view returns (uint256 numberOfRoles) { @@ -333,6 +471,17 @@ contract AccessAdmin is //--- internal/private functions -------------------------------------------------// + function _createTargetWithRole( + address target, + string memory targetName, + RoleId targetRoleId + ) + internal + { + _createTarget(target, targetName, true, false); + _grantRoleToAccount(targetRoleId, target); + } + function _authorizeTargetFunctions( address target, RoleId roleId, @@ -341,7 +490,7 @@ contract AccessAdmin is internal { if (roleId == getAdminRole()) { - revert ErrorAuthorizeForAdminRoleInvalid(target); + revert ErrorAccessAdminAuthorizeForAdminRoleInvalid(target); } ( @@ -430,13 +579,15 @@ contract AccessAdmin is functionSelectors, RoleId.unwrap(roleId)); + // log function grantings for (uint256 i = 0; i < functionSelectors.length; i++) { emit LogAccessAdminFunctionGranted( + _adminName, target, - // _getAccountName(target), - // string(abi.encodePacked("", functionSelectors[i])), - functionNames[i], - _getRoleName(roleId)); + string(abi.encodePacked( + functionNames[i], + "(): ", + _getRoleName(roleId)))); } } @@ -448,7 +599,7 @@ contract AccessAdmin is { // check max role members will not be exceeded if (_roleMembers[roleId].length() >= _roleInfo[roleId].maxMemberCount) { - revert ErrorRoleMembersLimitReached(roleId, _roleInfo[roleId].maxMemberCount); + revert ErrorAccessAdminRoleMembersLimitReached(roleId, _roleInfo[roleId].maxMemberCount); } // check account is contract for contract role @@ -456,7 +607,7 @@ contract AccessAdmin is _roleInfo[roleId].roleType == RoleType.Contract && !ContractLib.isContract(account) // will fail in account's constructor ) { - revert ErrorRoleMemberNotContract(roleId, account); + revert ErrorAccessAdminRoleMemberNotContract(roleId, account); } // TODO check account already have roleId @@ -466,7 +617,7 @@ contract AccessAdmin is account, 0); - emit LogAccessAdminRoleGranted(account, _getRoleName(roleId)); + emit LogAccessAdminRoleGranted(_adminName, account, _getRoleName(roleId)); } /// @dev revoke the specified role from the provided account @@ -477,7 +628,7 @@ contract AccessAdmin is // check role removal is permitted if (_roleInfo[roleId].roleType == RoleType.Contract) { - revert ErrorRoleMemberRemovalDisabled(roleId, account); + revert ErrorAccessAdminRoleMemberRemovalDisabled(roleId, account); } // TODO check account have roleId? @@ -486,7 +637,7 @@ contract AccessAdmin is RoleId.unwrap(roleId), account); - emit LogAccessAdminRoleRevoked(account, _roleInfo[roleId].name.toString()); + emit LogAccessAdminRoleRevoked(_adminName, account, _roleInfo[roleId].name.toString()); } @@ -500,24 +651,24 @@ contract AccessAdmin is { // check role does not yet exist if(roleExists(roleId)) { - revert ErrorRoleAlreadyCreated( + revert ErrorAccessAdminRoleAlreadyCreated( roleId, _roleInfo[roleId].name.toString()); } // check admin role exists if(!roleExists(info.adminRoleId)) { - revert ErrorRoleAdminNotExisting(info.adminRoleId); + revert ErrorAccessAdminRoleAdminNotExisting(info.adminRoleId); } // check role name is not empty if(info.name.length() == 0) { - revert ErrorRoleNameEmpty(roleId); + revert ErrorAccessAdminRoleNameEmpty(roleId); } // check role name is not used for another role if(_roleForName[info.name].exists) { - revert ErrorRoleNameAlreadyExists( + revert ErrorAccessAdminRoleNameAlreadyExists( roleId, info.name.toString(), _roleForName[info.name].roleId); @@ -545,7 +696,7 @@ contract AccessAdmin is // add role to list of roles _roleIds.push(roleId); - emit LogAccessAdminRoleCreated(roleId, info.roleType, info.adminRoleId, info.name.toString()); + emit LogAccessAdminRoleCreated(_adminName, roleId, info.roleType, info.adminRoleId, info.name.toString()); } @@ -559,7 +710,7 @@ contract AccessAdmin is { // check target does not yet exist if(targetExists(target)) { - revert ErrorTargetAlreadyCreated( + revert ErrorAccessAdminTargetAlreadyCreated( target, _targetInfo[target].name.toString()); } @@ -567,12 +718,12 @@ contract AccessAdmin is // check target name is not empty Str name = StrLib.toStr(targetName); if(name.length() == 0) { - revert ErrorTargetNameEmpty(target); + revert ErrorAccessAdminTargetNameEmpty(target); } // check target name is not used for another target if( _targetForName[name] != address(0)) { - revert ErrorTargetNameAlreadyExists( + revert ErrorAccessAdminTargetNameAlreadyExists( target, targetName, _targetForName[name]); @@ -580,14 +731,14 @@ contract AccessAdmin is // check target is an access managed contract if (!_isAccessManaged(target)) { - revert ErrorTargetNotAccessManaged(target); + revert ErrorAccessAdminTargetNotAccessManaged(target); } // check target shares authority with this contract if (checkAuthority) { address targetAuthority = AccessManagedUpgradeable(target).authority(); if (targetAuthority != authority()) { - revert ErrorTargetAuthorityMismatch(authority(), targetAuthority); + revert ErrorAccessAdminTargetAuthorityMismatch(authority(), targetAuthority); } } @@ -604,7 +755,15 @@ contract AccessAdmin is // add role to list of roles _targets.push(target); - emit LogAccessAdminTargetCreated(target, targetName); + emit LogAccessAdminTargetCreated(_adminName, target, targetName); + } + + + function _setTargetLocked(address target, bool locked) + internal + onlyExistingTarget(target) + { + _authority.setTargetClosed(target, locked); } @@ -625,19 +784,6 @@ contract AccessAdmin is } - function _setTargetClosed(address target, bool locked) - internal - { - _checkTarget(target); - - // target locked/unlocked already - if(_authority.isTargetClosed(target) == locked) { - revert ErrorTargetAlreadyLocked(target, locked); - } - - _authority.setTargetClosed(target, locked); - } - function _getAccountName(address account) internal view returns (string memory) { if (targetExists(account)) { return _targetInfo[account].name.toString(); @@ -662,28 +808,24 @@ contract AccessAdmin is view { if (!roleExists(roleId)) { - revert ErrorRoleUnknown(roleId); + revert ErrorAccessAdminRoleUnknown(roleId); } uint64 roleIdInt = RoleId.unwrap(roleId); if (roleIdInt == _authority.ADMIN_ROLE()) { - revert ErrorRoleIsLocked(roleId); + revert ErrorAccessAdminRoleIsLocked(roleId); } // check if role is disabled if (onlyActiveRole && _roleInfo[roleId].pausedAt <= TimestampLib.blockTimestamp()) { - revert ErrorRoleIsPaused(roleId); + revert ErrorAccessAdminRoleIsPaused(roleId); } } - /// @dev check if target exists and reverts if it doesn't - function _checkTarget(address target) - internal - view - { - if (!targetExists(target)) { - revert ErrorTargetUnknown(target); + function _checkRegistry(address registry) internal view { + if (!ContractLib.isRegistry(registry)) { + revert ErrorAccessAdminNotRegistry(registry); } } } \ No newline at end of file diff --git a/contracts/authorization/AccessManagerCloneable.sol b/contracts/authorization/AccessManagerCloneable.sol index 67a43d2a6..d3c6a3c49 100644 --- a/contracts/authorization/AccessManagerCloneable.sol +++ b/contracts/authorization/AccessManagerCloneable.sol @@ -28,6 +28,7 @@ contract AccessManagerCloneable is VersionPart private _release; bool private _isLocked; + modifier onlyAdminRole() { (bool isMember, ) = hasRole(ADMIN_ROLE, msg.sender); if(!isMember) { @@ -36,12 +37,14 @@ contract AccessManagerCloneable is _; } + function initialize(address admin) - external + public initializer() { + __ERC165_init(); __AccessManager_init(admin); - _initializeERC165(); + _registerInterface(type(IAccessManager).interfaceId); } @@ -56,22 +59,27 @@ contract AccessManagerCloneable is onlyAdminRole reinitializer(uint64(release.toInt())) { - _completeSetup(registry, release, true); + _checkAndSetRegistry(registry); + _checkAndSetRelease(release); } - /// @dev Completes the setup of the access manager. - /// Links the access manager to the registry and sets the release version. - function completeSetup( - address registry, - VersionPart release, - bool verifyRelease - ) - public - onlyAdminRole - reinitializer(uint64(release.toInt())) - { - _completeSetup(registry, release, verifyRelease); - } + // /// @dev Completes the setup of the access manager. + // /// Links the access manager to the registry and sets the release version. + // function completeSetup( + // address registry, + // VersionPart release, + // bool verifyRelease + // ) + // public + // onlyAdminRole + // reinitializer(uint64(release.toInt())) + // { + // _checkAndSetRegistry(registry); + + // if (verifyRelease) { + // _checkAndSetRelease(release); + // } + // } /// @dev Returns true if the caller is authorized to call the target with the given selector and the manager lock is not set to locked. /// Feturn values as in OpenZeppelin AccessManager. @@ -89,12 +97,12 @@ contract AccessManagerCloneable is uint32 delay ) { - (immediate, delay) = super.canCall(caller, target, selector); - // locking of all contracts under control of this access manager - if (isLocked()) { + if (_isLocked) { revert ErrorAccessManagerTargetAdminLocked(target); } + + (immediate, delay) = super.canCall(caller, target, selector); } @@ -126,11 +134,18 @@ contract AccessManagerCloneable is } - function _completeSetup( - address registry, - VersionPart release, - bool verifyRelease - ) + function _checkAndSetRelease(VersionPart release) + internal + { + if (!release.isValidRelease()) { + revert ErrorAccessManagerInvalidRelease(release); + } + + _release = release; + } + + + function _checkAndSetRegistry(address registry) internal { // checks @@ -138,12 +153,7 @@ contract AccessManagerCloneable is revert ErrorAccessManagerRegistryAlreadySet(address(getRegistry()) ); } - if (verifyRelease && !release.isValidRelease()) { - revert ErrorAccessManagerInvalidRelease(release); - } - // effects __RegistryLinked_init(registry); - _release = release; } } \ No newline at end of file diff --git a/contracts/authorization/Authorization.sol b/contracts/authorization/Authorization.sol index ab45f7788..e154ce4a8 100644 --- a/contracts/authorization/Authorization.sol +++ b/contracts/authorization/Authorization.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.20; import {IAccess} from "./IAccess.sol"; import {IAuthorization} from "./IAuthorization.sol"; + +import {InitializableERC165} from "../shared/InitializableERC165.sol"; import {ObjectType, ObjectTypeLib, PRODUCT, ORACLE, DISTRIBUTION, POOL} from "../type/ObjectType.sol"; import {RoleId, RoleIdLib, ADMIN_ROLE} from "../type/RoleId.sol"; import {SelectorLib} from "../type/Selector.sol"; @@ -10,20 +12,23 @@ import {Str, StrLib} from "../type/String.sol"; import {TimestampLib} from "../type/Timestamp.sol"; import {VersionPart, VersionPartLib} from "../type/Version.sol"; -contract Authorization - is IAuthorization +contract Authorization is + InitializableERC165, + IAuthorization { uint256 public constant GIF_RELEASE = 3; string public constant ROLE_NAME_SUFFIX = "Role"; string public constant SERVICE_ROLE_NAME_SUFFIX = "ServiceRole"; + uint64 internal _nextGifContractRoleId; ObjectType[] internal _serviceDomains; mapping(ObjectType domain => Str target) internal _serviceTarget; string internal _mainTargetName = "Component"; string internal _tokenHandlerName = "ComponentTH"; + ObjectType _domain; Str internal _mainTarget; Str internal _tokenHandlerTarget; Str[] internal _targets; @@ -40,7 +45,9 @@ contract Authorization constructor( string memory mainTargetName, - ObjectType targetDomain + ObjectType targetDomain, + bool isComponent, + bool includeTokenHandler ) { // checks @@ -48,45 +55,52 @@ contract Authorization revert ErrorAuthorizationMainTargetNameEmpty(); } - if (targetDomain.eqz()) { + if (targetDomain == ObjectTypeLib.zero()) { revert ErrorAuthorizationTargetDomainZero(); } // effects - // setup main target, main role id and main role info + _initializeERC165(); + + _domain = targetDomain; _mainTargetName = mainTargetName; - _tokenHandlerName = string(abi.encodePacked(mainTargetName, "TH")); + _mainTarget = StrLib.toStr(mainTargetName); + _nextGifContractRoleId = 10; - RoleId mainRoleId = RoleIdLib.toComponentRoleId(targetDomain, 0); - string memory mainRolName = _toTargetRoleName(mainTargetName); + if (isComponent) { + if (targetDomain.eqz()) { + revert ErrorAuthorizationTargetDomainZero(); + } - _addTargetWithRole( - _mainTargetName, - mainRoleId, - mainRolName); + RoleId mainRoleId = RoleIdLib.toComponentRoleId(targetDomain, 0); + string memory mainRolName = _toTargetRoleName(_mainTargetName); - _mainTarget = StrLib.toStr(mainTargetName); - _targetRole[_mainTarget] = mainRoleId; - - // add token handler target for components - if (targetDomain == PRODUCT() - || targetDomain == DISTRIBUTION() - || targetDomain == ORACLE() - || targetDomain == POOL() - ) { - _addTarget(_tokenHandlerName); + _addTargetWithRole( + _mainTargetName, + mainRoleId, + mainRolName); + } else { + _addGifContractTarget(_mainTargetName); } - - // setup token handler target - _tokenHandlerTarget = StrLib.toStr(_tokenHandlerName); - // setup use case specific parts _setupServiceTargets(); _setupRoles(); // not including main target role _setupTargets(); // not including main target (and token handler target) - - _setupTokenHandlerAuthorizations(); _setupTargetAuthorizations(); // not including token handler target + + // setup component token handler + if (includeTokenHandler) { + _tokenHandlerName = string(abi.encodePacked(mainTargetName, "TH")); + _tokenHandlerTarget = StrLib.toStr(_tokenHandlerName); + _addTarget(_tokenHandlerName); + _setupTokenHandlerAuthorizations(); + } + + _registerInterfaceNotInitializing(type(IAuthorization).interfaceId); + } + + function getDomain() external view returns(ObjectType targetDomain) { + return _domain; } function getServiceDomains() external view returns(ObjectType[] memory serviceDomains) { @@ -147,7 +161,7 @@ contract Authorization return target == _mainTarget || _targetExists[target]; } - function getTargetRole(Str target) external view returns(RoleId roleId) { + function getTargetRole(Str target) public view returns(RoleId roleId) { return _targetRole[target]; } @@ -183,6 +197,20 @@ contract Authorization /// Overwrite this function for use case specific authorizations. function _setupTargetAuthorizations() internal virtual {} + function _addGifContractTarget(string memory contractName) internal { + + RoleId contractRoleId = RoleIdLib.toRoleId(_nextGifContractRoleId++); + string memory contractRoleName = string( + abi.encodePacked( + contractName, + ROLE_NAME_SUFFIX)); + + _addTargetWithRole( + contractName, + contractRoleId, + contractRoleName); + } + /// @dev Add the service target role for the specified service domain function _addServiceTargetWithRole(ObjectType serviceDomain) internal { // add service domain @@ -192,7 +220,7 @@ contract Authorization string memory serviceTargetName = ObjectTypeLib.toVersionedName( ObjectTypeLib.toName(serviceDomain), "Service", - getRelease().toInt()); + getRelease()); _serviceTarget[serviceDomain] = StrLib.toStr(serviceTargetName); @@ -200,7 +228,7 @@ contract Authorization string memory serviceRoleName = ObjectTypeLib.toVersionedName( ObjectTypeLib.toName(serviceDomain), "ServiceRole", - getRelease().toInt()); + getRelease()); _addTargetWithRole( serviceTargetName, @@ -235,7 +263,7 @@ contract Authorization ObjectTypeLib.toVersionedName( ObjectTypeLib.toName(serviceDomain), SERVICE_ROLE_NAME_SUFFIX, - getRelease().toInt())); + getRelease())); } diff --git a/contracts/authorization/IAccessAdmin.sol b/contracts/authorization/IAccessAdmin.sol index f26cb47d7..6bb16ed16 100644 --- a/contracts/authorization/IAccessAdmin.sol +++ b/contracts/authorization/IAccessAdmin.sol @@ -5,6 +5,10 @@ import {IAccessManaged} from "@openzeppelin/contracts/access/manager/IAccessMana import {IAccess} from "./IAccess.sol"; import {IRegistryLinked} from "../shared/IRegistryLinked.sol"; +import {IRelease} from "../registry/IRelease.sol"; + +import {NftId} from "../type/NftId.sol"; +import {ObjectType} from "../type/ObjectType.sol"; import {RoleId} from "../type/RoleId.sol"; import {Selector} from "../type/Selector.sol"; import {Str} from "../type/String.sol"; @@ -13,59 +17,80 @@ import {VersionPart} from "../type/Version.sol"; interface IAccessAdmin is IAccessManaged, IAccess, - IRegistryLinked + IRegistryLinked, + IRelease { // roles, targets and functions - event LogAccessAdminRoleCreated(RoleId roleId, RoleType roleType, RoleId roleAdminId, string name); - event LogAccessAdminTargetCreated(address target, string name); + event LogAccessAdminRoleCreated(string admin, RoleId roleId, RoleType roleType, RoleId roleAdminId, string name); + event LogAccessAdminTargetCreated(string admin, address target, string name); - event LogAccessAdminRoleGranted(address account, string roleName); - event LogAccessAdminRoleRevoked(address account, string roleName); - event LogAccessAdminFunctionGranted(address target, string functionName, string roleName); + event LogAccessAdminRoleGranted(string admin, address account, string roleName); + event LogAccessAdminRoleRevoked(string admin, address account, string roleName); + event LogAccessAdminFunctionGranted(string admin, address target, string func); // only deployer modifier - error ErrorNotDeployer(); + error ErrorAccessAdminNotDeployer(); // only role admin modifier - error ErrorNotAdminOfRole(RoleId adminRoleId); + error ErrorAccessAdminNotAdminOfRole(RoleId adminRoleId); // only role owner modifier - error ErrorNotRoleOwner(RoleId roleId); + error ErrorAccessAdminNotRoleOwner(RoleId roleId); + + // initialization + error ErrorAccessAdminNotRegistry(address registry); + error ErrorAccessAdminAuthorityNotContract(address authority); + error ErrorAccessAdminAccessManagerNotAccessManager(address authority); + error ErrorAccessAdminAccessManagerEmptyName(); + + // check target + error ErrorAccessAdminTargetNotCreated(address target); + error ErrorAccessAdminTargetNotRegistered(address target); + error ErrorAccessAdminTargetTypeMismatch(address target, ObjectType expectedType, ObjectType actualType); + + // check authorization + error ErrorAccessAdminAlreadyInitialized(address authorization); + error ErrorAccessAdminNotAuthorization(address authorization); + error ErrorAccessAdminDomainMismatch(address authorization, ObjectType expectedDomain, ObjectType actualDomain); + error ErrorAccessAdminReleaseMismatch(address authorization, VersionPart expectedRelease, VersionPart actualRelease); + + // link to nft + error ErrorAccessAdminNotRegistered(address registerable); // initialize authority - error ErrorAdminRoleMissing(); + error ErrorAccessAdminAdminRoleMissing(); // create role - error ErrorRoleAlreadyCreated(RoleId roleId, string name); - error ErrorRoleAdminNotExisting(RoleId adminRoleId); - error ErrorRoleNameEmpty(RoleId roleId); - error ErrorRoleNameAlreadyExists(RoleId roleId, string name, RoleId existingRoleId); + error ErrorAccessAdminRoleAlreadyCreated(RoleId roleId, string name); + error ErrorAccessAdminRoleAdminNotExisting(RoleId adminRoleId); + error ErrorAccessAdminRoleNameEmpty(RoleId roleId); + error ErrorAccessAdminRoleNameAlreadyExists(RoleId roleId, string name, RoleId existingRoleId); // grant/revoke/renounce role - error ErrorRoleUnknown(RoleId roleId); - error ErrorRoleIsLocked(RoleId roleId); - error ErrorRoleIsPaused(RoleId roleId); - error ErrorRoleMembersLimitReached(RoleId roleId, uint256 memberCountLimit); - error ErrorRoleMemberNotContract(RoleId roleId, address notContract); - error ErrorRoleMemberRemovalDisabled(RoleId roleId, address expectedMember); + error ErrorAccessAdminRoleUnknown(RoleId roleId); + error ErrorAccessAdminRoleIsLocked(RoleId roleId); + error ErrorAccessAdminRoleIsPaused(RoleId roleId); + error ErrorAccessAdminRoleMembersLimitReached(RoleId roleId, uint256 memberCountLimit); + error ErrorAccessAdminRoleMemberNotContract(RoleId roleId, address notContract); + error ErrorAccessAdminRoleMemberRemovalDisabled(RoleId roleId, address expectedMember); // create target - error ErrorTargetAlreadyCreated(address target, string name); - error ErrorTargetNameEmpty(address target); - error ErrorTargetNameAlreadyExists(address target, string name, address existingTarget); - error ErrorTargetNotAccessManaged(address target); - error ErrorTargetAuthorityMismatch(address expectedAuthority, address actualAuthority); + error ErrorAccessAdminTargetAlreadyCreated(address target, string name); + error ErrorAccessAdminTargetNameEmpty(address target); + error ErrorAccessAdminTargetNameAlreadyExists(address target, string name, address existingTarget); + error ErrorAccessAdminTargetNotAccessManaged(address target); + error ErrorAccessAdminTargetAuthorityMismatch(address expectedAuthority, address actualAuthority); // lock target - error ErrorTagetNotLockable(); - error ErrorTargetAlreadyLocked(address target, bool isLocked); + error ErrorAccessAdminTagetNotLockable(); + error ErrorAccessAdminTargetAlreadyLocked(address target, bool isLocked); // authorize target functions - error ErrorAuthorizeForAdminRoleInvalid(address target); + error ErrorAccessAdminAuthorizeForAdminRoleInvalid(address target); // check target - error ErrorTargetUnknown(address target); + error ErrorAccessAdminTargetUnknown(address target); /// @dev Set the disabled status of the speicified role. /// Role disabling only prevents the role from being granted to new accounts. @@ -110,7 +135,10 @@ interface IAccessAdmin is //--- view functions ----------------------------------------------------// - function getRelease() external view returns (VersionPart release); + function getLinkedNftId() external view returns (NftId linkedNftId); + function getLinkedOwner() external view returns (address linkedOwner); + + function isLocked() external view returns (bool locked); function roles() external view returns (uint256 numberOfRoles); function getRoleId(uint256 idx) external view returns (RoleId roleId); diff --git a/contracts/authorization/IAuthorization.sol b/contracts/authorization/IAuthorization.sol index ba27951e2..4551f18b7 100644 --- a/contracts/authorization/IAuthorization.sol +++ b/contracts/authorization/IAuthorization.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.20; +import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; + import {IAccess} from "../authorization/IAccess.sol"; import {ObjectType} from "../type/ObjectType.sol"; import {RoleId} from "../type/RoleId.sol"; @@ -8,12 +10,16 @@ import {Str} from "../type/String.sol"; import {VersionPart} from "../type/Version.sol"; interface IAuthorization is + IERC165, IAccess { error ErrorAuthorizationMainTargetNameEmpty(); error ErrorAuthorizationTargetDomainZero(); + /// @dev Returns the main domain of the authorization. + function getDomain() external view returns(ObjectType targetDomain); + /// @dev Returns the list of service targets. function getServiceDomains() external view returns(ObjectType[] memory serviceDomains); diff --git a/contracts/distribution/BasicDistributionAuthorization.sol b/contracts/distribution/BasicDistributionAuthorization.sol index 861f969c7..08e2f81d7 100644 --- a/contracts/distribution/BasicDistributionAuthorization.sol +++ b/contracts/distribution/BasicDistributionAuthorization.sol @@ -17,7 +17,7 @@ contract BasicDistributionAuthorization { constructor(string memory distributionlName) - Authorization(distributionlName, DISTRIBUTION()) + Authorization(distributionlName, DISTRIBUTION(), true, true) {} function _setupServiceTargets() diff --git a/contracts/instance/IInstance.sol b/contracts/instance/IInstance.sol index 30dba5c15..8382aa21e 100644 --- a/contracts/instance/IInstance.sol +++ b/contracts/instance/IInstance.sol @@ -38,25 +38,57 @@ interface IInstance is uint64 requestsCount; } + ///--- instance ---------------------------------------------------------// + + /// @dev Locks/unlocks the complete instance, including all its components. + function setInstanceLocked(bool locked) external; + + /// @dev Upgrades the instance reader to the specified target. + function upgradeInstanceReader() external; + + /// @dev Sets the instance reader for the instance. + /// Permissioned: only the instance service may call this function. + function setInstanceReader(InstanceReader instanceReader) external; + + ///--- staking ----------------------------------------------------------// + + /// @dev Sets the duration for locking new stakes on this instance.. + function setStakingLockingPeriod(Seconds stakeLockingPeriod) external; + + /// @dev Sets the staking reward rate [apr] for this instance. + function setStakingRewardRate(UFixed rewardRate) external; + + /// @dev Refills the staking reward reserves for the specified target. + function refillStakingRewardReserves(Amount dipAmount) external; + + /// @dev Defunds the staking reward reserves for the specified target. + function withdrawStakingRewardReserves(Amount dipAmount) external returns (Amount newBalance); + + ///--- product/component ------------------------------------------------// + + /// @dev Locks/unlocks the specified target. + function setTargetLocked(address target, bool locked) external; + /// @dev Register a product with the instance. function registerProduct(address product) external returns (NftId productNftId); + ///--- authz ------------------------------------------------------------// + function createRole(string memory roleName, string memory adminName) external returns (RoleId roleId, RoleId admin); function grantRole(RoleId roleId, address account) external; function revokeRole(RoleId roleId, address account) external; function createTarget(address target, string memory name) external; function setTargetFunctionRole(string memory targetName, bytes4[] calldata selectors, RoleId roleId) external; - function setLocked(address target, bool locked) external; - function setLockedFromService(address target, bool locked) external; - function setStakingLockingPeriod(Seconds stakeLockingPeriod) external; - function setStakingRewardRate(UFixed rewardRate) external; - function refillStakingRewardReserves(Amount dipAmount) external; - /// @dev Defunds the staking reward reserves for the specified target. - /// Permissioned: only the target owner may call this function. - function withdrawStakingRewardReserves(Amount dipAmount) external returns (Amount newBalance); + //--- getters -----------------------------------------------------------// + + /// @dev returns the overall locking state of the instance (including all components) + function isInstanceLocked() external view returns (bool isLocked); + + /// @dev returns the locking state of the specified target + function isTargetLocked(address target) external view returns (bool isLocked); // get products function products() external view returns (uint256 productCount); diff --git a/contracts/instance/IInstanceService.sol b/contracts/instance/IInstanceService.sol index 06513f8fe..9f927e275 100644 --- a/contracts/instance/IInstanceService.sol +++ b/contracts/instance/IInstanceService.sol @@ -15,7 +15,7 @@ interface IInstanceService is IService { // onlyInstance error ErrorInstanceServiceNotRegistered(address instance); error ErrorInstanceServiceNotInstance(address instance, ObjectType objectType); - error ErrorInstanceServiceInstanceVersionMismatch(address instance, VersionPart instanceVersion); + error ErrorInstanceServiceInstanceVersionMismatch(NftId instanceNftId, VersionPart expectedRelease, VersionPart instanceRelease); error ErrorInstanceServiceComponentNotInstanceLinked(address component); @@ -53,6 +53,17 @@ interface IInstanceService is IService { event LogInstanceCloned(NftId instanceNftId, address instance); + /// @dev Locks the complete instance, including all its components. + function setInstanceLocked(bool locked) external; + + /// @dev Locks/unlocks the specified target constrolled by the corresponding instance admin. + function setTargetLocked(address target, bool locked) external; + + /// @dev Creates a new instance. + /// The caller becomes the owner of the new instance. + /// Creation of a new instance is achieved by this service through the creation and registration + /// of a new clone of the master instance and then setting up the initial wiring and authorization + /// of the necessary components. function createInstance() external returns ( @@ -60,7 +71,7 @@ interface IInstanceService is IService { NftId instanceNftId ); - function upgradeInstanceReader(NftId instanceNftId) external; + function upgradeInstanceReader() external; function upgradeMasterInstanceReader(address instanceReaderAddress) external; function setStakingLockingPeriod(Seconds stakeLockingPeriod) external; diff --git a/contracts/instance/Instance.sol b/contracts/instance/Instance.sol index 7381b2804..3c434ce3e 100644 --- a/contracts/instance/Instance.sol +++ b/contracts/instance/Instance.sol @@ -1,22 +1,24 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.20; +import {IInstance} from "./IInstance.sol"; +import {IComponentService} from "../shared/IComponentService.sol"; +import {IInstanceService} from "./IInstanceService.sol"; +import {IRegistry} from "../registry/IRegistry.sol"; + import {Amount} from "../type/Amount.sol"; import {BundleSet} from "./BundleSet.sol"; import {RiskSet} from "./RiskSet.sol"; import {COMPONENT, INSTANCE} from "../type/ObjectType.sol"; -import {IInstance} from "./IInstance.sol"; -import {IComponentService} from "../shared/IComponentService.sol"; -import {IInstanceService} from "./IInstanceService.sol"; import {InstanceReader} from "./InstanceReader.sol"; import {InstanceAdmin} from "./InstanceAdmin.sol"; import {InstanceStore} from "./InstanceStore.sol"; -import {IRegistry} from "../registry/IRegistry.sol"; import {NftId} from "../type/NftId.sol"; import {Registerable} from "../shared/Registerable.sol"; import {RoleId} from "../type/RoleId.sol"; import {Seconds} from "../type/Seconds.sol"; import {UFixed} from "../type/UFixed.sol"; +import {VersionPart} from "../type/Version.sol"; contract Instance is IInstance, @@ -48,6 +50,7 @@ contract Instance is RiskSet riskSet, InstanceReader instanceReader, IRegistry registry, + VersionPart release, address initialOwner ) external @@ -84,20 +87,40 @@ contract Instance is _componentService = IComponentService( getRegistry().getServiceAddress( COMPONENT(), - getRelease())); + release)); _instanceService = IInstanceService( getRegistry().getServiceAddress( INSTANCE(), - getRelease())); + release)); _registerInterface(type(IInstance).interfaceId); } + + function setInstanceLocked(bool locked) + external + // not restricted(): need to be able to unlock a locked instance + onlyOwner() + { + _instanceService.setInstanceLocked(locked); + } + + + function upgradeInstanceReader() + external + restricted() + onlyOwner() + { + _instanceService.upgradeInstanceReader(); + } + + //--- ProductRegistration ----------------------------------------------// function registerProduct(address product) external + restricted() onlyOwner() returns (NftId productNftId) { @@ -109,6 +132,7 @@ contract Instance is function setStakingLockingPeriod(Seconds stakeLockingPeriod) external + restricted() onlyOwner() { _instanceService.setStakingLockingPeriod(stakeLockingPeriod); @@ -116,6 +140,7 @@ contract Instance is function setStakingRewardRate(UFixed rewardRate) external + restricted() onlyOwner() { _instanceService.setStakingRewardRate(rewardRate); @@ -123,6 +148,7 @@ contract Instance is function refillStakingRewardReserves(Amount dipAmount) external + restricted() onlyOwner() { address instanceOwner = msg.sender; @@ -131,6 +157,7 @@ contract Instance is function withdrawStakingRewardReserves(Amount dipAmount) external + restricted() onlyOwner() returns (Amount newBalance) { @@ -141,6 +168,7 @@ contract Instance is function createRole(string memory roleName, string memory adminName) external + restricted() onlyOwner() returns (RoleId roleId, RoleId admin) { @@ -150,6 +178,7 @@ contract Instance is function grantRole(RoleId roleId, address account) external + restricted() onlyOwner() { _instanceAdmin.grantRole(roleId, account); @@ -157,6 +186,7 @@ contract Instance is function revokeRole(RoleId roleId, address account) external + restricted() onlyOwner() { // TODO refactor @@ -164,9 +194,19 @@ contract Instance is } //--- Targets ------------------------------------------------------------// - // TODO consider restricted() + + + function setTargetLocked(address target, bool locked) + external + // not restricted(): instance owner may need to be able to unlock targets on an locked instance + onlyOwner() + { + _instanceService.setTargetLocked(target, locked); + } + function createTarget(address target, string memory name) external + restricted() onlyOwner() { // TODO refactor @@ -179,27 +219,13 @@ contract Instance is RoleId roleId ) external + restricted() onlyOwner() { // TODO refactor // _instanceAdmin.setTargetFunctionRoleByInstance(targetName, selectors, roleId); } - function setLocked(address target, bool locked) - external - onlyOwner() - { - _instanceAdmin.setTargetLocked(target, locked); - } - - function setLockedFromService(address target, bool locked) - external - restricted() - { - // TODO service can not lock any of instance contracts - _instanceAdmin.setTargetLocked(target, locked); - } - //--- ITransferInterceptor ----------------------------------------------// function nftTransferFrom(address from, address to, uint256 tokenId, address operator) external onlyChainNft { @@ -223,6 +249,14 @@ contract Instance is //--- external view functions -------------------------------------------// + function isInstanceLocked() external view returns (bool) { + return _instanceAdmin.isLocked(); + } + + function isTargetLocked(address target) external view returns (bool) { + return _instanceAdmin.isTargetLocked(target); + } + function products() external view returns (uint256 productCount) { return _products.length; } diff --git a/contracts/instance/InstanceAdmin.sol b/contracts/instance/InstanceAdmin.sol index 8b73f6779..41c4e4681 100644 --- a/contracts/instance/InstanceAdmin.sol +++ b/contracts/instance/InstanceAdmin.sol @@ -10,7 +10,7 @@ import {IInstance} from "./IInstance.sol"; import {AccessAdmin} from "../authorization/AccessAdmin.sol"; import {AccessManagerCloneable} from "../authorization/AccessManagerCloneable.sol"; import {NftId} from "../type/NftId.sol"; -import {ObjectType} from "../type/ObjectType.sol"; +import {ObjectType, INSTANCE} from "../type/ObjectType.sol"; import {RoleId, RoleIdLib} from "../type/RoleId.sol"; import {Str, StrLib} from "../type/String.sol"; import {TokenHandler} from "../shared/TokenHandler.sol"; @@ -21,19 +21,19 @@ contract InstanceAdmin is AccessAdmin { string public constant INSTANCE_TARGET_NAME = "Instance"; - string public constant INSTANCE_STORE_TARGET_NAME = "InstanceStore"; string public constant INSTANCE_ADMIN_TARGET_NAME = "InstanceAdmin"; + string public constant INSTANCE_STORE_TARGET_NAME = "InstanceStore"; string public constant BUNDLE_SET_TARGET_NAME = "BundleSet"; - string public constant RISK_SET_TAGET_NAME = "RiskSet"; + string public constant RISK_SET_TARGET_NAME = "RiskSet"; uint64 public constant CUSTOM_ROLE_ID_MIN = 10000; // MUST be even - error ErrorInstanceAdminCallerNotInstanceOwner(address caller); - error ErrorInstanceAdminCallerNotInstance(address caller); - error ErrorInstanceAdminInstanceAlreadyLocked(); - error ErrorInstanceAdminNotRegistered(address target); + error ErrorInstanceAdminNotInstanceService(address caller); + + error ErrorInstanceAdminNotRegistered(address instance); + error ErrorInstanceAdminNotInstance(address instance); + error ErrorInstanceAdminAlreadyAuthorized(address instance); - error ErrorInstanceAdminAlreadyAuthorized(address target); error ErrorInstanceAdminNotComponentRole(RoleId roleId); error ErrorInstanceAdminRoleAlreadyExists(RoleId roleId); error ErrorInstanceAdminRoleTypeNotContract(RoleId roleId, IAccess.RoleType roleType); @@ -43,136 +43,171 @@ contract InstanceAdmin is IInstance internal _instance; IRegistry internal _registry; + VersionPart internal _release; uint64 internal _customIdNext; mapping(address target => RoleId roleId) internal _targetRoleId; uint64 internal _components; - IAuthorization internal _instanceAuthorization; - - - modifier onlyInstanceOwner() { - if(msg.sender != _registry.ownerOf(address(_instance))) { - revert ErrorInstanceAdminCallerNotInstanceOwner(msg.sender); + modifier onlyInstanceService() { + if (msg.sender != _registry.getServiceAddress(INSTANCE(), getRelease())) { + revert ErrorInstanceAdminNotInstanceService(msg.sender); } _; } - modifier onlyInstance() { - if(msg.sender != address(_instance)) { - revert ErrorInstanceAdminCallerNotInstance(msg.sender); - } - _; - } +// TODO cleanup logs +event LogDebug3(string key, string value); /// @dev Only used for master instance admin. - /// Contracts created via constructor come with disabled initializers. - constructor( - address instanceAuthorization - ) { - initialize(new AccessManagerCloneable()); - - _instanceAuthorization = IAuthorization(instanceAuthorization); - - _disableInitializers(); + constructor(address accessManager) { +emit LogDebug3("2a", ""); + initialize( + accessManager, + "MasterInstanceAdmin"); +emit LogDebug3("2b", ""); } + // TODO cleanup + // function initialize( + // AccessManagerCloneable clonedAccessManager, + // IRegistry registry, + // VersionPart release + // ) + // public + // initializer() + // { + // // checks - function initialize( - AccessManagerCloneable clonedAccessManager, - IRegistry registry, - VersionPart release - ) - external - initializer() - { - __AccessAdmin_init(clonedAccessManager); + // // effects + // __AccessAdmin_init( + // address(clonedAccessManager), + // "InstanceAdmin"); - clonedAccessManager.completeSetup( - address(registry), - release); + // clonedAccessManager.completeSetup( + // address(registry), + // release); + + // _registry = registry; + // } - _registry = registry; - } /// @dev Completes the initialization of this instance admin using the provided instance, registry and version. /// Important: Initialization of instance admin is only complete after calling this function. /// Important: The instance MUST be registered and all instance supporting contracts must be wired to this instance. function completeSetup( + address registry, address instance, - address authorization + address authorization, + VersionPart release ) external - reinitializer(uint64(getRelease().toInt())) + reinitializer(uint64(release.toInt())) onlyDeployer() { + // checks + _checkRegistry(registry); + _checkIsRegistered(registry, instance, INSTANCE()); + + AccessManagerCloneable( + authority()).completeSetup( + registry, + release); + + _checkAuthorization(authorization, INSTANCE(), release, true); + + // effects + _registry = IRegistry(registry); + _release = release; + + _instance = IInstance(instance); + _authorization = IAuthorization(authorization); _components = 0; _customIdNext = CUSTOM_ROLE_ID_MIN; - _instance = IInstance(instance); - _instanceAuthorization = IAuthorization(authorization); - _checkTargetIsReadyForAuthorization(instance); - - // check matching releases - if (_instanceAuthorization.getRelease() != getRelease()) { - revert ErrorInstanceAdminReleaseMismatch(); - } + // link nft ownability to instance + _linkToNftOwnable(instance); +emit LogDebug3("3d", ""); // create instance role and target _setupInstance(instance); +emit LogDebug3("3e", ""); // add instance authorization - _createRoles(_instanceAuthorization); - + _createRoles(_authorization); +emit LogDebug3("3f", ""); + // _createTargets(_authorization); +emit LogDebug3("3g", ""); _setupInstanceHelperTargetsWithRoles(); - _createTargetAuthorizations(_instanceAuthorization); +emit LogDebug3("3h", ""); + _createTargetAuthorizations(_authorization); +emit LogDebug3("3i", ""); + } + + + function _createTargets(IAuthorization authorization) + internal + { + _createTargetWithRole(address(this), INSTANCE_ADMIN_TARGET_NAME, authorization.getTargetRole(StrLib.toStr(INSTANCE_ADMIN_TARGET_NAME))); + _createTargetWithRole(address(_instance.getInstanceStore()), INSTANCE_STORE_TARGET_NAME, authorization.getTargetRole(StrLib.toStr(INSTANCE_STORE_TARGET_NAME))); + _createTargetWithRole(address(_instance.getBundleSet()), BUNDLE_SET_TARGET_NAME, authorization.getTargetRole(StrLib.toStr(BUNDLE_SET_TARGET_NAME))); + _createTargetWithRole(address(_instance.getRiskSet()), RISK_SET_TARGET_NAME, authorization.getTargetRole(StrLib.toStr(RISK_SET_TARGET_NAME))); } /// @dev Initializes the authorization for the specified component. /// Important: The component MUST be registered. function initializeComponentAuthorization( - IInstanceLinkedComponent component + address componentAddress, + ObjectType expectedType ) external restricted() { - // TODO check componentInfo exists - // TODO deploy token handler here // checks - _checkTargetIsReadyForAuthorization(address(component)); + _checkIsRegistered(address(getRegistry()), componentAddress, expectedType); + IInstanceLinkedComponent component = IInstanceLinkedComponent(componentAddress); + IAuthorization authorization = component.getAuthorization(); + _checkAuthorization(address(authorization), expectedType, getRelease(), false); + + // effects // setup target and role for component (including token handler) _setupComponentAndTokenHandler(component); - // create other roles - IAuthorization authorization = component.getAuthorization(); + // create other roles and function authorizations _createRoles(authorization); - - - // // FIXME: make this a bit nicer and work with IAuthorization. Use a specific role, not public - access to TokenHandler must be restricted - // _authorizeTargetFunctions( - // address(component.getTokenHandler()), - // getPublicRole(), - // functions); - _createTargetAuthorizations(authorization); } + function getRelease() + public + view + override + returns (VersionPart release) + { + return _release; + } + + event LogDebug(string key, string value); + // create instance role and target function _setupInstance(address instance) internal { + + emit LogDebug("main target", _authorization.getMainTarget().toString()); + // create instance role - RoleId instanceRoleId = _instanceAuthorization.getTargetRole( - _instanceAuthorization.getMainTarget()); + RoleId instanceRoleId = _authorization.getTargetRole( + _authorization.getMainTarget()); _createRole( instanceRoleId, - _instanceAuthorization.getRoleInfo(instanceRoleId)); + _authorization.getRoleInfo(instanceRoleId)); // create instance target _createTarget( instance, - _instanceAuthorization.getMainTargetName(), + _authorization.getMainTargetName(), true, // checkAuthority false); // custom @@ -204,21 +239,28 @@ contract InstanceAdmin is function setInstanceLocked(bool locked) external - onlyInstanceOwner() + // not restricted(): need to operate on locked instances to unlock instance + onlyInstanceService() { AccessManagerCloneable accessManager = AccessManagerCloneable(authority()); - - if(accessManager.isLocked() == locked) { - revert ErrorInstanceAdminInstanceAlreadyLocked(); - } accessManager.setLocked(locked); } + function setTargetLocked(address target, bool locked) external - onlyInstance() + // not restricted(): might need to operate on targets while instance is locked + onlyInstanceService() + { + _setTargetLocked(target, locked); + } + + + function setComponentLocked(address target, bool locked) + external + restricted() { - _setTargetClosed(target, locked); + _setTargetLocked(target, locked); } @@ -229,7 +271,7 @@ contract InstanceAdmin is view returns (IAuthorization instanceAuthorizaion) { - return _instanceAuthorization; + return _authorization; } // ------------------- Internal functions ------------------- // @@ -323,20 +365,6 @@ contract InstanceAdmin is } - function _checkTargetIsReadyForAuthorization(address target) - internal - view - { - if (!_registry.isRegistered(target)) { - revert ErrorInstanceAdminNotRegistered(target); - } - - if (targetExists(target)) { - revert ErrorInstanceAdminAlreadyAuthorized(target); - } - } - - function _createRoles(IAuthorization authorization) internal { @@ -403,7 +431,7 @@ contract InstanceAdmin is { // check that target name is defined in authorization specification Str name = StrLib.toStr(targetName); - if (!_instanceAuthorization.targetExists(name)) { + if (!_authorization.targetExists(name)) { revert ErrorInstanceAdminExpectedTargetMissing(targetName); } @@ -415,7 +443,7 @@ contract InstanceAdmin is false); // assign target role if defined - RoleId targetRoleId = _instanceAuthorization.getTargetRole(name); + RoleId targetRoleId = _authorization.getTargetRole(name); if (targetRoleId != RoleIdLib.zero()) { _grantRoleToAccount(targetRoleId, target); } @@ -430,11 +458,11 @@ contract InstanceAdmin is _checkAndCreateTargetWithRole(address(_instance.getInstanceStore()), INSTANCE_STORE_TARGET_NAME); _checkAndCreateTargetWithRole(address(_instance.getInstanceAdmin()), INSTANCE_ADMIN_TARGET_NAME); _checkAndCreateTargetWithRole(address(_instance.getBundleSet()), BUNDLE_SET_TARGET_NAME); - _checkAndCreateTargetWithRole(address(_instance.getRiskSet()), RISK_SET_TAGET_NAME); + _checkAndCreateTargetWithRole(address(_instance.getRiskSet()), RISK_SET_TARGET_NAME); // create targets for services that need to access the module targets - ObjectType[] memory serviceDomains = _instanceAuthorization.getServiceDomains(); - VersionPart release = _instanceAuthorization.getRelease(); + ObjectType[] memory serviceDomains = _authorization.getServiceDomains(); + VersionPart release = _authorization.getRelease(); ObjectType serviceDomain; for (uint256 i = 0; i < serviceDomains.length; i++) { @@ -442,7 +470,7 @@ contract InstanceAdmin is _checkAndCreateTargetWithRole( _registry.getServiceAddress(serviceDomain, release), - _instanceAuthorization.getServiceTarget(serviceDomain).toString()); + _authorization.getServiceTarget(serviceDomain).toString()); } } } diff --git a/contracts/instance/InstanceAuthorizationV3.sol b/contracts/instance/InstanceAuthorizationV3.sol index 52cfb468c..c999c7722 100644 --- a/contracts/instance/InstanceAuthorizationV3.sol +++ b/contracts/instance/InstanceAuthorizationV3.sol @@ -1,17 +1,16 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.20; -import { - ACCOUNTING, ORACLE, POOL, INSTANCE, COMPONENT, DISTRIBUTION, APPLICATION, POLICY, CLAIM, BUNDLE, RISK -} from "../../contracts/type/ObjectType.sol"; +import {IAccess} from "../authorization/IAccess.sol"; +import {Authorization} from "../authorization/Authorization.sol"; +import {ACCOUNTING, ORACLE, POOL, INSTANCE, COMPONENT, DISTRIBUTION, APPLICATION, POLICY, CLAIM, BUNDLE, RISK} from "../../contracts/type/ObjectType.sol"; import {BundleSet} from "../instance/BundleSet.sol"; -import {RiskSet} from "../instance/RiskSet.sol"; -import {IAccess} from "../authorization/IAccess.sol"; import {Instance} from "../instance/Instance.sol"; import {InstanceAdmin} from "../instance/InstanceAdmin.sol"; import {InstanceStore} from "../instance/InstanceStore.sol"; -import {Authorization} from "../authorization/Authorization.sol"; +import {PUBLIC_ROLE} from "../type/RoleId.sol"; +import {RiskSet} from "../instance/RiskSet.sol"; contract InstanceAuthorizationV3 @@ -27,19 +26,10 @@ contract InstanceAuthorizationV3 string public constant RISK_SET_TARGET_NAME = "RiskSet"; constructor() - Authorization(INSTANCE_TARGET_NAME, INSTANCE()) + Authorization(INSTANCE_TARGET_NAME, INSTANCE(), false, false) { } - function _setupTargets() - internal - override - { - // instance supporting targets - _addTarget(INSTANCE_STORE_TARGET_NAME); - _addTarget(INSTANCE_ADMIN_TARGET_NAME); - _addTarget(BUNDLE_SET_TARGET_NAME); - _addTarget(RISK_SET_TARGET_NAME); - + function _setupServiceTargets() internal virtual override { // service targets relevant to instance _addServiceTargetWithRole(INSTANCE()); _addServiceTargetWithRole(ACCOUNTING()); @@ -54,6 +44,19 @@ contract InstanceAuthorizationV3 _addServiceTargetWithRole(CLAIM()); } + function _setupTargets() + internal + override + { + // instance supporting targets + // _addGifContractTarget(INSTANCE_ADMIN_TARGET_NAME); + _addTarget(INSTANCE_ADMIN_TARGET_NAME); + _addTarget(INSTANCE_STORE_TARGET_NAME); + _addTarget(BUNDLE_SET_TARGET_NAME); + _addTarget(RISK_SET_TARGET_NAME); + + } + function _setupTargetAuthorizations() internal @@ -107,13 +110,27 @@ contract InstanceAuthorizationV3 { IAccess.FunctionInfo[] storage functions; + // authorize instance service role + functions = _authorizeForTarget(INSTANCE_TARGET_NAME, PUBLIC_ROLE()); + _authorize(functions, Instance.registerProduct.selector, "registerProduct"); + _authorize(functions, Instance.upgradeInstanceReader.selector, "upgradeInstanceReader"); + + // staking + _authorize(functions, Instance.setStakingLockingPeriod.selector, "setStakingLockingPeriod"); + _authorize(functions, Instance.setStakingRewardRate.selector, "setStakingRewardRate"); + _authorize(functions, Instance.refillStakingRewardReserves.selector, "refillStakingRewardReserves"); + _authorize(functions, Instance.withdrawStakingRewardReserves.selector, "withdrawStakingRewardReserves"); + + // custom authz + _authorize(functions, Instance.createRole.selector, "createRole"); + _authorize(functions, Instance.grantRole.selector, "grantRole"); + _authorize(functions, Instance.revokeRole.selector, "revokeRole"); + _authorize(functions, Instance.createTarget.selector, "createTarget"); + _authorize(functions, Instance.setTargetFunctionRole.selector, "setTargetFunctionRole"); + // authorize instance service role functions = _authorizeForTarget(INSTANCE_TARGET_NAME, getServiceRole(INSTANCE())); _authorize(functions, Instance.setInstanceReader.selector, "setInstanceReader"); - - // authorize component service role - functions = _authorizeForTarget(INSTANCE_TARGET_NAME, getServiceRole(COMPONENT())); - _authorize(functions, Instance.setLockedFromService.selector, "setLockedFromService"); } @@ -122,13 +139,15 @@ contract InstanceAuthorizationV3 { IAccess.FunctionInfo[] storage functions; - // authorize instance role - functions = _authorizeForTarget(INSTANCE_ADMIN_TARGET_NAME, getComponentRole(INSTANCE())); - _authorize(functions, InstanceAdmin.grantRole.selector, "grantRole"); + // authorize component service role + functions = _authorizeForTarget(INSTANCE_ADMIN_TARGET_NAME, getServiceRole(INSTANCE())); + _authorize(functions, InstanceAdmin.setInstanceLocked.selector, "setInstanceLocked"); + _authorize(functions, InstanceAdmin.setTargetLocked.selector, "setTargetLocked"); // authorize component service role functions = _authorizeForTarget(INSTANCE_ADMIN_TARGET_NAME, getServiceRole(COMPONENT())); _authorize(functions, InstanceAdmin.initializeComponentAuthorization.selector, "initializeComponentAuthoriz"); + _authorize(functions, InstanceAdmin.setComponentLocked.selector, "setComponentLocked"); } diff --git a/contracts/instance/InstanceService.sol b/contracts/instance/InstanceService.sol index e824e9a47..e48bc531c 100644 --- a/contracts/instance/InstanceService.sol +++ b/contracts/instance/InstanceService.sol @@ -4,31 +4,28 @@ pragma solidity ^0.8.20; import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; import {IAuthorization} from "../authorization/IAuthorization.sol"; -import {AccessManagerCloneable} from "../authorization/AccessManagerCloneable.sol"; -import {Amount} from "../type/Amount.sol"; -import {BundleSet} from "./BundleSet.sol"; -import {RiskSet} from "./RiskSet.sol"; -import {ChainNft} from "../registry/ChainNft.sol"; -import {NftId} from "../type/NftId.sol"; -import {RoleId} from "../type/RoleId.sol"; -import {UFixed} from "../type/UFixed.sol"; -import {ObjectType, INSTANCE, COMPONENT, INSTANCE, REGISTRY, STAKING} from "../type/ObjectType.sol"; - -import {Service} from "../shared/Service.sol"; - +import {IComponentService} from "../shared/IComponentService.sol"; +import {IInstance} from "./IInstance.sol"; +import {IInstanceService} from "./IInstanceService.sol"; import {IRegistry} from "../registry/IRegistry.sol"; import {IRegistryService} from "../registry/IRegistryService.sol"; import {IStakingService} from "../staking/IStakingService.sol"; -import {TargetManagerLib} from "../staking/TargetManagerLib.sol"; -import {IComponentService} from "../shared/IComponentService.sol"; +import {AccessManagerCloneable} from "../authorization/AccessManagerCloneable.sol"; +import {Amount} from "../type/Amount.sol"; +import {BundleSet} from "./BundleSet.sol"; import {Instance} from "./Instance.sol"; -import {IInstance} from "./IInstance.sol"; import {InstanceAdmin} from "./InstanceAdmin.sol"; -import {IInstanceService} from "./IInstanceService.sol"; import {InstanceReader} from "./InstanceReader.sol"; import {InstanceStore} from "./InstanceStore.sol"; +import {NftId} from "../type/NftId.sol"; +import {ObjectType, INSTANCE, COMPONENT, INSTANCE, REGISTRY, STAKING} from "../type/ObjectType.sol"; +import {RiskSet} from "./RiskSet.sol"; +import {RoleId} from "../type/RoleId.sol"; import {Seconds} from "../type/Seconds.sol"; +import {Service} from "../shared/Service.sol"; +import {TargetManagerLib} from "../staking/TargetManagerLib.sol"; +import {UFixed} from "../type/UFixed.sol"; import {VersionPart} from "../type/Version.sol"; @@ -53,23 +50,8 @@ contract InstanceService is address internal _masterInstanceStore; - modifier onlyInstance() { - address instanceAddress = msg.sender; - NftId instanceNftId = getRegistry().getNftIdForAddress(msg.sender); - if (instanceNftId.eqz()) { - revert ErrorInstanceServiceNotRegistered(instanceAddress); - } - - ObjectType objectType = getRegistry().getObjectInfo(instanceNftId).objectType; - if (objectType != INSTANCE()) { - revert ErrorInstanceServiceNotInstance(instanceAddress, objectType); - } - - VersionPart instanceVersion = IInstance(instanceAddress).getRelease(); - if (instanceVersion != getVersion().toMajorPart()) { - revert ErrorInstanceServiceInstanceVersionMismatch(instanceAddress, instanceVersion); - } - + modifier onlyInstance() { + _checkInstance(msg.sender, getRelease()); _; } @@ -90,8 +72,35 @@ contract InstanceService is _; } + /// @inheritdoc IInstanceService + function setInstanceLocked(bool locked) + external + virtual + restricted() + onlyInstance() + { + address instanceAddress = msg.sender; + IInstance(instanceAddress).getInstanceAdmin().setInstanceLocked(locked); + } + + + /// @dev Locks/unlocks the specified target constrolled by the corresponding instance admin. + function setTargetLocked(address target, bool locked) + external + virtual + restricted() + onlyInstance() + { + address instanceAddress = msg.sender; + IInstance(instanceAddress).getInstanceAdmin().setTargetLocked(target, locked); + } + +// TODO cleanup logs +event LogDebug4(string key, string value); + function createInstance() external + virtual restricted() returns ( IInstance instance, @@ -102,25 +111,38 @@ contract InstanceService is address instanceOwner = msg.sender; // create instance admin and instance - InstanceAdmin instanceAdmin = _createInstanceAdmin(); + InstanceAdmin instanceAdmin = _cloneNewInstanceAdmin(); instance = _createInstance(instanceAdmin, instanceOwner); +emit LogDebug4("3a", "0"); // register cloned instance with registry instanceNftId = _registryService.registerInstance( instance, instanceOwner).nftId; + // MUST be set after instance is set up and registered +emit LogDebug4("3a", "1"); + IAuthorization instanceAuthorization = InstanceAdmin(_masterInstanceAdmin).getInstanceAuthorization(); +emit LogDebug4("3a", "2"); + instanceAdmin.completeSetup( + address(getRegistry()), + address(instance), + address(instanceAuthorization), + getRelease()); +emit LogDebug4("3a", "3"); + + // hard checks for newly cloned instance + assert(address(instance.getRegistry()) == address(getRegistry())); + assert(instance.getRelease() == getRelease()); + +emit LogDebug4("3a", "4"); + // register cloned instance as staking target _stakingService.createInstanceTarget( instanceNftId, TargetManagerLib.getDefaultLockingPeriod(), TargetManagerLib.getDefaultRewardRate()); - // MUST be set after instance is set up and registered - IAuthorization instanceAuthorization = InstanceAdmin(_masterInstanceAdmin).getInstanceAuthorization(); - VersionPart release = AccessManagerCloneable(authority()).getRelease(); - instanceAdmin.completeSetup( - address(instance), - address(instanceAuthorization)); +emit LogDebug4("3a", "5"); emit LogInstanceCloned( instanceNftId, @@ -181,22 +203,23 @@ contract InstanceService is dipAmount); } - function upgradeInstanceReader(NftId instanceNftId) + + function upgradeInstanceReader() external - nonReentrant() restricted() - onlyInstanceOwner(instanceNftId) - onlyNftOfType(instanceNftId, INSTANCE()) + onlyInstance() { - IRegistry registry = getRegistry(); - IRegistry.ObjectInfo memory instanceInfo = registry.getObjectInfo(instanceNftId); - Instance instance = Instance(instanceInfo.objectAddress); + address instanceAddress = msg.sender; + IInstance instance = IInstance(msg.sender); - InstanceReader upgradedInstanceReaderClone = InstanceReader(Clones.clone(address(_masterInstanceReader))); - upgradedInstanceReaderClone.initializeWithInstance(address(instance)); + InstanceReader upgradedInstanceReaderClone = InstanceReader( + Clones.clone(address(_masterInstanceReader))); + + upgradedInstanceReaderClone.initializeWithInstance(instanceAddress); instance.setInstanceReader(upgradedInstanceReaderClone); } + function setAndRegisterMasterInstance(address instanceAddress) external onlyOwner() @@ -269,22 +292,23 @@ contract InstanceService is /// @dev create new cloned instance admin /// function used to setup a new instance - function _createInstanceAdmin() + function _cloneNewInstanceAdmin() internal virtual - returns (InstanceAdmin clonedInstanceAdmin) + returns (InstanceAdmin clonedAdmin) { - // start with setting up a new OZ access manager - // TODO consider _masterInstanceAdmin.authority() instead of _masterAccessManager + // clone instance specific access manager AccessManagerCloneable clonedAccessManager = AccessManagerCloneable( - Clones.clone(_masterAccessManager)); + Clones.clone( + InstanceAdmin(_masterInstanceAdmin).authority())); // set up the instance admin - clonedInstanceAdmin = InstanceAdmin(Clones.clone(_masterInstanceAdmin)); - clonedInstanceAdmin.initialize( - clonedAccessManager, - getRegistry(), - getRelease()); + clonedAdmin = InstanceAdmin( + Clones.clone(_masterInstanceAdmin)); + + clonedAdmin.initialize( + address(clonedAccessManager), + "InstanceAdmin"); } @@ -312,7 +336,16 @@ contract InstanceService is clonedRiskSet, clonedInstanceReader, getRegistry(), + getRelease(), instanceOwner); + + // TODO cleanup + // instanceAdmin.completeSetup( + // address(getRegistry()), + // address(clonedInstance), + // address(instanceAdmin.getInstanceAuthorization()), + // getRelease()); + return clonedInstance; } @@ -397,6 +430,36 @@ contract InstanceService is } + + function _checkInstance( + address instanceAddress, + VersionPart expectedRelease + ) + internal + virtual + view + { + IRegistry registry = getRegistry(); + + NftId instanceNftId = registry.getNftIdForAddress(instanceAddress); + if (instanceNftId.eqz()) { + revert ErrorInstanceServiceNotRegistered(instanceAddress); + } + + ObjectType objectType = registry.getObjectInfo(instanceNftId).objectType; + if (objectType != INSTANCE()) { + revert ErrorInstanceServiceNotInstance(instanceAddress, objectType); + } + + if (expectedRelease.gtz()) { + VersionPart instanceRelease = IInstance(instanceAddress).getRelease(); + if (instanceRelease != expectedRelease) { + revert ErrorInstanceServiceInstanceVersionMismatch(instanceNftId, expectedRelease, instanceRelease); + } + } + } + + // From IService function _getDomain() internal pure override returns(ObjectType) { return INSTANCE(); diff --git a/contracts/oracle/BasicOracleAuthorization.sol b/contracts/oracle/BasicOracleAuthorization.sol index 3b8f5cd62..423ff6936 100644 --- a/contracts/oracle/BasicOracleAuthorization.sol +++ b/contracts/oracle/BasicOracleAuthorization.sol @@ -15,7 +15,7 @@ contract BasicOracleAuthorization { constructor(string memory componentName) - Authorization(componentName, ORACLE()) + Authorization(componentName, ORACLE(), true, false) {} function _setupTargetAuthorizations() diff --git a/contracts/pool/BasicPoolAuthorization.sol b/contracts/pool/BasicPoolAuthorization.sol index a5cf12357..19497b892 100644 --- a/contracts/pool/BasicPoolAuthorization.sol +++ b/contracts/pool/BasicPoolAuthorization.sol @@ -18,7 +18,7 @@ contract BasicPoolAuthorization { constructor(string memory poolName) - Authorization(poolName, POOL()) + Authorization(poolName, POOL(), true, true) {} function _setupServiceTargets() diff --git a/contracts/product/BasicProductAuthorization.sol b/contracts/product/BasicProductAuthorization.sol index 63ec54f67..c810af56f 100644 --- a/contracts/product/BasicProductAuthorization.sol +++ b/contracts/product/BasicProductAuthorization.sol @@ -16,7 +16,7 @@ contract BasicProductAuthorization { constructor(string memory componentName) - Authorization(componentName, PRODUCT()) + Authorization(componentName, PRODUCT(), true, true) {} function _setupServiceTargets() diff --git a/contracts/registry/RegistryAdmin.sol b/contracts/registry/RegistryAdmin.sol index 39346c604..6928d0b72 100644 --- a/contracts/registry/RegistryAdmin.sol +++ b/contracts/registry/RegistryAdmin.sol @@ -1,18 +1,20 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.20; -import {AccessAdmin} from "../authorization/AccessAdmin.sol"; -import {AccessManagerCloneable} from "../authorization/AccessManagerCloneable.sol"; -import {IAccess} from "../authorization/IAccess.sol"; +import {IAuthorization} from "../authorization/IAuthorization.sol"; import {IComponent} from "../shared/IComponent.sol"; import {IRegistry} from "./IRegistry.sol"; import {IService} from "../shared/IService.sol"; -import {IServiceAuthorization} from "../authorization/IServiceAuthorization.sol"; import {IStaking} from "../staking/IStaking.sol"; -import {ObjectType, ObjectTypeLib, ALL, COMPONENT, REGISTRY, STAKING, POOL, RELEASE} from "../type/ObjectType.sol"; + +import {AccessAdmin} from "../authorization/AccessAdmin.sol"; +import {AccessManagerCloneable} from "../authorization/AccessManagerCloneable.sol"; +import {ContractLib} from "../shared/ContractLib.sol"; +import {ObjectType, REGISTRY, STAKING, POOL, RELEASE} from "../type/ObjectType.sol"; import {ReleaseRegistry} from "./ReleaseRegistry.sol"; import {RoleId, RoleIdLib, ADMIN_ROLE, GIF_MANAGER_ROLE, GIF_ADMIN_ROLE, PUBLIC_ROLE} from "../type/RoleId.sol"; import {Staking} from "../staking/Staking.sol"; +import {Str, StrLib} from "../type/String.sol"; import {StakingStore} from "../staking/StakingStore.sol"; import {TokenHandler} from "../shared/TokenHandler.sol"; import {TokenRegistry} from "./TokenRegistry.sol"; @@ -60,7 +62,8 @@ contract RegistryAdmin is string public constant TOKEN_REGISTRY_TARGET_NAME = "TokenRegistry"; string public constant TOKEN_HANDLER_TARGET_NAME = "TokenHandler"; - uint8 public constant MAX_NUM_RELEASES = 99; + // completeSetup + error ErrorRegistryAdminNotRegistry(address registry); address internal _registry; address private _releaseRegistry; @@ -68,12 +71,27 @@ contract RegistryAdmin is address private _staking; address private _stakingStore; + // function initialize() + // external + // { + // AccessManagerCloneable accessManager = new AccessManagerCloneable(); + + // initialize( + // address(accessManager), + // "RegistryAdmin"); + // } + constructor() { - initialize(new AccessManagerCloneable()); + initialize( + address(new AccessManagerCloneable()), + "RegistryAdmin"); } +event LogDebug2(string message, uint256 value); + function completeSetup( - IRegistry registry, + address registry, + address authorization, address gifAdmin, address gifManager ) @@ -82,37 +100,144 @@ contract RegistryAdmin is reinitializer(type(uint8).max) onlyDeployer() { - AccessManagerCloneable accessManager = AccessManagerCloneable(authority()); - accessManager.completeSetup( - address(registry), - VersionPartLib.toVersionPart(type(uint8).max), - false); - - _registry = address(registry); - _releaseRegistry = registry.getReleaseRegistryAddress(); - _tokenRegistry = registry.getTokenRegistryAddress(); - _staking = registry.getStakingAddress(); + // TODO cleanup logs + emit LogDebug2("a", 0); + + // checks + _checkRegistry(registry); + + VersionPart release = VersionPartLib.toVersionPart(3); + AccessManagerCloneable( + authority()).completeSetup( + registry, + release); + + _checkAuthorization(authorization, REGISTRY(), release, true); + + _registry = registry; + _authorization = IAuthorization(authorization); + + IRegistry registryContract = IRegistry(registry); + _releaseRegistry = registryContract.getReleaseRegistryAddress(); + _tokenRegistry = registryContract.getTokenRegistryAddress(); + _staking = registryContract.getStakingAddress(); _stakingStore = address( IStaking(_staking).getStakingStore()); - _createTargets(); + // link nft ownability to registry + _linkToNftOwnable(_registry); + + _setupRegistry(_registry); +emit LogDebug2("b", 0); - _setupGifAdminRole(gifAdmin); - _setupGifManagerRole(gifManager); + // setup authorization for registry and supporting contracts + _createRoles(_authorization); +emit LogDebug2("c", 0); + _grantRoleToAccount(GIF_ADMIN_ROLE(), gifAdmin); +emit LogDebug2("d", 0); + _grantRoleToAccount(GIF_MANAGER_ROLE(), gifManager); +emit LogDebug2("e", 0); - _setupRegistryRoles(); - _setupStakingRoles(); + _createTargets(_authorization); +emit LogDebug2("f", 0); + _createTargetAuthorizations(_authorization); +emit LogDebug2("g", 0); } - /*function transferAdmin(address to) - external - restricted // only with GIF_ADMIN_ROLE or nft owner + + // create registry role and target + function _setupRegistry(address registry) internal { + + // create registry role + RoleId roleId = _authorization.getTargetRole( + _authorization.getMainTarget()); + + _createRole( + roleId, + _authorization.getRoleInfo(roleId)); + + // create registry target + _createTarget( + registry, + _authorization.getMainTargetName(), + true, // checkAuthority + false); // custom + + // assign registry role to registry + _grantRoleToAccount( + roleId, + registry); + } + + + function _createRoles(IAuthorization authorization) + internal { - _accessManager.revoke(GIF_ADMIN_ROLE, ); - _accesssManager.grant(GIF_ADMIN_ROLE, to, 0); - }*/ + RoleId[] memory roles = authorization.getRoles(); + RoleId mainTargetRoleId = authorization.getTargetRole( + authorization.getMainTarget()); + + RoleId roleId; + RoleInfo memory roleInfo; - function grantServiceRoleForAllVersions(IService service, ObjectType domain) + for(uint256 i = 0; i < roles.length; i++) { + + roleId = roles[i]; + + // skip main target role, create role if not exists + if (roleId != mainTargetRoleId && !roleExists(roleId)) { + _createRole( + roleId, + authorization.getRoleInfo(roleId)); + } + } + } + + + function _createTargets(IAuthorization authorization) + internal + { + // registry + _createTargetWithRole(address(this), REGISTRY_ADMIN_TARGET_NAME, authorization.getTargetRole(StrLib.toStr(REGISTRY_ADMIN_TARGET_NAME))); + _createTargetWithRole(_releaseRegistry, RELEASE_REGISTRY_TARGET_NAME, authorization.getTargetRole(StrLib.toStr(RELEASE_REGISTRY_TARGET_NAME))); + _createTargetWithRole(_tokenRegistry, TOKEN_REGISTRY_TARGET_NAME, authorization.getTargetRole(StrLib.toStr(TOKEN_REGISTRY_TARGET_NAME))); + + // staking + _createTargetWithRole(_staking, STAKING_TARGET_NAME, authorization.getTargetRole(StrLib.toStr(STAKING_TARGET_NAME))); + _createTarget(_stakingStore, STAKING_STORE_TARGET_NAME, true, false); + _createTarget(address(IComponent(_staking).getTokenHandler()), STAKING_TH_TARGET_NAME, true, false); + } + + + function _createTargetAuthorizations(IAuthorization authorization) + internal + { + Str[] memory targets = authorization.getTargets(); + Str target; + + for(uint256 i = 0; i < targets.length; i++) { + target = targets[i]; + RoleId[] memory authorizedRoles = authorization.getAuthorizedRoles(target); + RoleId authorizedRole; + + for(uint256 j = 0; j < authorizedRoles.length; j++) { + authorizedRole = authorizedRoles[j]; + + _authorizeTargetFunctions( + getTargetForName(target), + authorizedRole, + authorization.getAuthorizedFunctions( + target, + authorizedRole)); + } + } + } + + + function grantServiceRoleForAllVersions( + IService service, + ObjectType domain + ) external restricted() { @@ -133,7 +258,7 @@ contract RegistryAdmin is //--- private initialization functions -------------------------------------------// - function _createTargets() + function _createTargets(address authorization) private onlyInitializing() { @@ -245,7 +370,7 @@ contract RegistryAdmin is toRole({ adminRoleId: ADMIN_ROLE(), roleType: RoleType.Contract, - maxMemberCount: MAX_NUM_RELEASES, + maxMemberCount: _getMaxRelases(), name: REGISTRY_SERVICE_ROLE_NAME})); // grant permissions to the registry service role for registry contract @@ -304,7 +429,7 @@ contract RegistryAdmin is toRole({ adminRoleId: ADMIN_ROLE(), roleType: RoleType.Contract, - maxMemberCount: MAX_NUM_RELEASES, + maxMemberCount: _getMaxRelases(), name: STAKING_SERVICE_ROLE_NAME})); // grant permissions to the staking service role for staking contract @@ -337,7 +462,7 @@ contract RegistryAdmin is toRole({ adminRoleId: ADMIN_ROLE(), roleType: RoleType.Contract, - maxMemberCount: MAX_NUM_RELEASES, + maxMemberCount: _getMaxRelases(), name: POOL_SERVICE_ROLE_NAME})); // grant permissions to the pool service role for staking contract @@ -351,4 +476,8 @@ contract RegistryAdmin is functions[0] = toFunction(Staking.approveTokenHandler.selector, "approveTokenHandler"); _authorizeTargetFunctions(_staking, PUBLIC_ROLE(), functions); } + + function _getMaxRelases() internal pure returns (uint32) { + return uint32(VersionPartLib.releaseMax().toInt()); + } } \ No newline at end of file diff --git a/contracts/registry/RegistryAuthorization.sol b/contracts/registry/RegistryAuthorization.sol new file mode 100644 index 000000000..d68b01c34 --- /dev/null +++ b/contracts/registry/RegistryAuthorization.sol @@ -0,0 +1,266 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.20; + +import {IAccess} from "../authorization/IAccess.sol"; +import {IRegistry} from "../registry/IRegistry.sol"; +import {IStaking} from "../staking/IStaking.sol"; + +import {Authorization} from "../authorization/Authorization.sol"; +import {POOL, REGISTRY, STAKING} from "../../contracts/type/ObjectType.sol"; +import {PUBLIC_ROLE} from "../type/RoleId.sol"; +import {ReleaseRegistry} from "./ReleaseRegistry.sol"; +import {RegistryAdmin} from "./RegistryAdmin.sol"; +import {RoleIdLib, ADMIN_ROLE, GIF_ADMIN_ROLE, GIF_MANAGER_ROLE} from "../type/RoleId.sol"; +import {StakingStore} from "../staking/StakingStore.sol"; +import {TokenHandler} from "../shared/TokenHandler.sol"; +import {TokenRegistry} from "../registry/TokenRegistry.sol"; + + +contract RegistryAuthorization + is Authorization +{ + + /// @dev gif core roles + string public constant GIF_ADMIN_ROLE_NAME = "GifAdminRole"; + string public constant GIF_MANAGER_ROLE_NAME = "GifManagerRole"; + string public constant RELEASE_REGISTRY_ROLE_NAME = "ReleaseRegistryRole"; + string public constant STAKING_ROLE_NAME = "StakingRole"; + + /// @dev gif roles for external contracts + string public constant REGISTRY_SERVICE_ROLE_NAME = "RegistryServiceRole"; + string public constant COMPONENT_SERVICE_ROLE_NAME = "ComponentServiceRole"; + string public constant POOL_SERVICE_ROLE_NAME = "PoolServiceRole"; + string public constant STAKING_SERVICE_ROLE_NAME = "StakingServiceRole"; + + /// @dev gif core targets + string public constant REGISTRY_ADMIN_TARGET_NAME = "RegistryAdmin"; + string public constant REGISTRY_TARGET_NAME = "Registry"; + string public constant RELEASE_REGISTRY_TARGET_NAME = "ReleaseRegistry"; + string public constant TOKEN_REGISTRY_TARGET_NAME = "TokenRegistry"; + + string public constant STAKING_TARGET_NAME = "Staking"; + string public constant STAKING_TH_TARGET_NAME = "StakingTH"; + string public constant STAKING_STORE_TARGET_NAME = "StakingStore"; + + constructor() + Authorization(REGISTRY_TARGET_NAME, REGISTRY(), false, false) + { } + + /// @dev Sets up the GIF admin and manager roles. + function _setupRoles() internal override { + + // service roles (for all versions) + _addRole( + RoleIdLib.roleForTypeAndAllVersions(REGISTRY()), + _toRoleInfo({ + adminRoleId: ADMIN_ROLE(), + roleType: RoleType.Gif, + maxMemberCount: 999, // max member count = max + name: REGISTRY_SERVICE_ROLE_NAME})); + + _addRole( + RoleIdLib.roleForTypeAndAllVersions(STAKING()), + _toRoleInfo({ + adminRoleId: ADMIN_ROLE(), + roleType: RoleType.Gif, + maxMemberCount: 999, // max member count = max + name: STAKING_SERVICE_ROLE_NAME})); + + _addRole( + RoleIdLib.roleForTypeAndAllVersions(POOL()), + _toRoleInfo({ + adminRoleId: ADMIN_ROLE(), + roleType: RoleType.Gif, + maxMemberCount: 999, // max member count = max + name: POOL_SERVICE_ROLE_NAME})); + + // gif admin role + _addRole( + GIF_ADMIN_ROLE(), + _toRoleInfo({ + adminRoleId: ADMIN_ROLE(), + roleType: RoleType.Gif, + maxMemberCount: 2, // TODO decide on max member count + name: GIF_ADMIN_ROLE_NAME})); + + // gif manager role + _addRole( + GIF_MANAGER_ROLE(), + _toRoleInfo({ + adminRoleId: ADMIN_ROLE(), + roleType: RoleType.Gif, + maxMemberCount: 1, // TODO decide on max member count + name: GIF_MANAGER_ROLE_NAME})); + + } + + /// @dev Sets up the relevant (non-service) targets for the registry. + function _setupTargets() internal override { + _addGifContractTarget(REGISTRY_ADMIN_TARGET_NAME); + _addGifContractTarget(RELEASE_REGISTRY_TARGET_NAME); + _addGifContractTarget(TOKEN_REGISTRY_TARGET_NAME); + + _addGifContractTarget(STAKING_TARGET_NAME); + _addGifContractTarget(STAKING_TH_TARGET_NAME); + _addTarget(STAKING_STORE_TARGET_NAME); + } + + + function _setupTargetAuthorizations() + internal + override + { + // registry + _setupRegistryAuthorization(); + _setupRegistryAdminAuthorization(); + _setupReleaseRegistryAuthorization(); + _setupTokenRegistryAuthorization(); + + // staking + _setupStakingAuthorization(); + _setupStakingThAuthorization(); + _setupStakingStoreAuthorization(); + } + + event LogAccessAdminDebug(string message, string custom, uint256 value); + + function _setupRegistryAuthorization() internal { + IAccess.FunctionInfo[] storage functions; + + // gif admin role + functions = _authorizeForTarget(REGISTRY_TARGET_NAME, GIF_ADMIN_ROLE()); + _authorize(functions, IRegistry.registerRegistry.selector, "registerRegistry"); + + // registry service role + functions = _authorizeForTarget( + REGISTRY_TARGET_NAME, + RoleIdLib.roleForTypeAndAllVersions(REGISTRY())); + + _authorize(functions, IRegistry.register.selector, "register"); + _authorize(functions, IRegistry.registerWithCustomType.selector, "registerWithCustomType"); + + // release registry role + functions = _authorizeForTarget( + REGISTRY_TARGET_NAME, + getTargetRole(getTarget(RELEASE_REGISTRY_TARGET_NAME))); + + _authorize(functions, IRegistry.registerService.selector, "registerService"); + } + + + function _setupRegistryAdminAuthorization() internal { + IAccess.FunctionInfo[] storage functions; + + // release registry role + functions = _authorizeForTarget( + REGISTRY_ADMIN_TARGET_NAME, + getTargetRole(getTarget(RELEASE_REGISTRY_TARGET_NAME))); + + _authorize(functions, RegistryAdmin.grantServiceRoleForAllVersions.selector, "grantServiceRoleForAllVersions"); + } + + + function _setupReleaseRegistryAuthorization() internal { + IAccess.FunctionInfo[] storage functions; + + functions = _authorizeForTarget(RELEASE_REGISTRY_TARGET_NAME, GIF_ADMIN_ROLE()); + _authorize(functions, ReleaseRegistry.createNextRelease.selector, "createNextRelease"); + _authorize(functions, ReleaseRegistry.activateNextRelease.selector, "activateNextRelease"); + _authorize(functions, ReleaseRegistry.setActive.selector, "setActive"); + + functions = _authorizeForTarget(RELEASE_REGISTRY_TARGET_NAME, GIF_MANAGER_ROLE()); + _authorize(functions, ReleaseRegistry.prepareNextRelease.selector, "prepareNextRelease"); + _authorize(functions, ReleaseRegistry.registerService.selector, "registerService"); + } + + + function _setupTokenRegistryAuthorization() internal { + IAccess.FunctionInfo[] storage functions; + + // gif manager role + functions = _authorizeForTarget(TOKEN_REGISTRY_TARGET_NAME, GIF_MANAGER_ROLE()); + _authorize(functions, TokenRegistry.registerToken.selector, "registerToken"); + _authorize(functions, TokenRegistry.registerRemoteToken.selector, "registerRemoteToken"); + _authorize(functions, TokenRegistry.setActive.selector, "setActive"); + _authorize(functions, TokenRegistry.setActiveForVersion.selector, "setActiveForVersion"); + // TODO find a better way (only needed for testing) + _authorize(functions, TokenRegistry.setActiveWithVersionCheck.selector, "setActiveWithVersionCheck"); + } + + + function _setupStakingAuthorization() internal { + IAccess.FunctionInfo[] storage functions; + + // staking public role + functions = _authorizeForTarget( + STAKING_TARGET_NAME, + PUBLIC_ROLE()); + + _authorize(functions, IStaking.approveTokenHandler.selector, "approveTokenHandler"); + + // staking service role + functions = _authorizeForTarget( + STAKING_TARGET_NAME, + RoleIdLib.roleForTypeAndAllVersions(STAKING())); + + _authorize(functions, IStaking.registerTarget.selector, "registerTarget"); + _authorize(functions, IStaking.setLockingPeriod.selector, "setLockingPeriod"); + _authorize(functions, IStaking.setRewardRate.selector, "setRewardRate"); + _authorize(functions, IStaking.refillRewardReserves.selector, "refillRewardReserves"); + _authorize(functions, IStaking.withdrawRewardReserves.selector, "withdrawRewardReserves"); + _authorize(functions, IStaking.createStake.selector, "createStake"); + _authorize(functions, IStaking.stake.selector, "stake"); + _authorize(functions, IStaking.unstake.selector, "unstake"); + _authorize(functions, IStaking.restake.selector, "restake"); + _authorize(functions, IStaking.updateRewards.selector, "updateRewards"); + _authorize(functions, IStaking.claimRewards.selector, "claimRewards"); + + // pool service role + functions = _authorizeForTarget( + STAKING_TARGET_NAME, + RoleIdLib.roleForTypeAndAllVersions(POOL())); + + _authorize(functions, IStaking.increaseTotalValueLocked.selector, "increaseTotalValueLocked"); + _authorize(functions, IStaking.decreaseTotalValueLocked.selector, "decreaseTotalValueLocked"); + } + + + function _setupStakingThAuthorization() internal { + IAccess.FunctionInfo[] storage functions; + + // staking service role + functions = _authorizeForTarget( + STAKING_TH_TARGET_NAME, + RoleIdLib.roleForTypeAndAllVersions(STAKING())); + + _authorize(functions, TokenHandler.approve.selector, "approve"); + _authorize(functions, TokenHandler.pullToken.selector, "pullToken"); + _authorize(functions, TokenHandler.pushToken.selector, "pushToken"); + } + + + function _setupStakingStoreAuthorization() internal { + IAccess.FunctionInfo[] storage functions; + + // release registry role + functions = _authorizeForTarget( + STAKING_STORE_TARGET_NAME, + getTargetRole(getTarget(STAKING_TARGET_NAME))); + + _authorize(functions, StakingStore.setStakingRate.selector, "setStakingRate"); + _authorize(functions, StakingStore.createTarget.selector, "createTarget"); + _authorize(functions, StakingStore.updateTarget.selector, "updateTarget"); + _authorize(functions, StakingStore.increaseReserves.selector, "increaseReserves"); + _authorize(functions, StakingStore.decreaseReserves.selector, "decreaseReserves"); + _authorize(functions, StakingStore.increaseTotalValueLocked.selector, "increaseTotalValueLocked"); + _authorize(functions, StakingStore.decreaseTotalValueLocked.selector, "decreaseTotalValueLocked"); + _authorize(functions, StakingStore.create.selector, "create"); + _authorize(functions, StakingStore.update.selector, "update"); + _authorize(functions, StakingStore.increaseStake.selector, "increaseStake"); + _authorize(functions, StakingStore.restakeRewards.selector, "restakeRewards"); + _authorize(functions, StakingStore.updateRewards.selector, "updateRewards"); + _authorize(functions, StakingStore.claimUpTo.selector, "claimUpTo"); + _authorize(functions, StakingStore.unstakeUpTo.selector, "unstakeUpTo"); + } +} + diff --git a/contracts/registry/ReleaseAdmin.sol b/contracts/registry/ReleaseAdmin.sol index 4b1b2f4ca..0a3f5be18 100644 --- a/contracts/registry/ReleaseAdmin.sol +++ b/contracts/registry/ReleaseAdmin.sol @@ -38,13 +38,14 @@ contract ReleaseAdmin is _; } - /// @dev Only used for master release admin. - /// Contracts created via constructor come with disabled initializers. - constructor() { - initialize(new AccessManagerCloneable()); - _disableInitializers(); + // @dev Only used for master release admin + constructor(address accessManager) { + initialize( + accessManager, + "MasterReleaseAdmin"); } + function completeSetup( address registry, address releaseRegistry, @@ -53,11 +54,18 @@ contract ReleaseAdmin is external reinitializer(uint64(release.toInt())) { + + // checks + _checkRegistry(registry); + AccessManagerCloneable( authority()).completeSetup( registry, release); + // link nft ownability to registry + _linkToNftOwnable(registry); + _setupReleaseRegistry(releaseRegistry); } @@ -68,13 +76,13 @@ contract ReleaseAdmin is IServiceAuthorization serviceAuthorization, IService service, ObjectType serviceDomain, - VersionPart releaseVersion + VersionPart release ) external restricted() { - _createServiceTargetAndRole(service, serviceDomain, releaseVersion); - _authorizeServiceFunctions(serviceAuthorization, service, serviceDomain, releaseVersion); + _createServiceTargetAndRole(service, serviceDomain, release); + _authorizeServiceFunctions(serviceAuthorization, service, serviceDomain, release); } /// @dev Locks/unlocks all release targets. @@ -109,7 +117,7 @@ contract ReleaseAdmin is revert ErrorReleaseAdminNotService(address(service)); } - _setTargetClosed(address(service), locked); + _setTargetLocked(address(service), locked); emit LogReleaseAdminServiceLockChanged(service.getRelease(), address(service), locked); } @@ -126,16 +134,15 @@ contract ReleaseAdmin is function _createServiceTargetAndRole( IService service, ObjectType serviceDomain, - VersionPart releaseVersion + VersionPart release ) private { string memory baseName = ObjectTypeLib.toName(serviceDomain); - uint256 versionInt = releaseVersion.toInt(); // create service target string memory serviceTargetName = ObjectTypeLib.toVersionedName( - baseName, "Service", versionInt); + baseName, "Service", release); _createTarget( address(service), @@ -146,7 +153,7 @@ contract ReleaseAdmin is // create service role RoleId serviceRoleId = RoleIdLib.roleForTypeAndVersion( serviceDomain, - releaseVersion); + release); if(!roleExists(serviceRoleId)) { _createRole( @@ -158,7 +165,7 @@ contract ReleaseAdmin is name: ObjectTypeLib.toVersionedName( baseName, "ServiceRole", - versionInt)})); + release)})); } _grantRoleToAccount( @@ -171,7 +178,7 @@ contract ReleaseAdmin is IServiceAuthorization serviceAuthorization, IService service, ObjectType serviceDomain, - VersionPart releaseVersion + VersionPart release ) private { @@ -189,7 +196,7 @@ contract ReleaseAdmin is } else { authorizedRoleId = RoleIdLib.roleForTypeAndVersion( authorizedDomain, - releaseVersion); + release); } if(!roleExists(authorizedRoleId)) { @@ -203,7 +210,7 @@ contract ReleaseAdmin is name: ObjectTypeLib.toVersionedName( ObjectTypeLib.toName(authorizedDomain), "Role", - releaseVersion.toInt())})); + release)})); } // get authorized functions for authorized domain diff --git a/contracts/registry/ReleaseRegistry.sol b/contracts/registry/ReleaseRegistry.sol index 02499ef6d..874dd7eff 100644 --- a/contracts/registry/ReleaseRegistry.sol +++ b/contracts/registry/ReleaseRegistry.sol @@ -99,7 +99,8 @@ contract ReleaseRegistry is _registry = registry; _admin = RegistryAdmin(_registry.getRegistryAdminAddress()); - _masterReleaseAdmin = new ReleaseAdmin(); + _masterReleaseAdmin = new ReleaseAdmin( + address(new AccessManagerCloneable())); _next = VersionPartLib.toVersionPart(INITIAL_GIF_VERSION - 1); } @@ -373,19 +374,24 @@ contract ReleaseRegistry is private returns (ReleaseAdmin clonedAdmin) { + // clone release specific access manager AccessManagerCloneable clonedAccessManager = AccessManagerCloneable( Clones.clone( - _masterReleaseAdmin.authority() - ) - ); + _masterReleaseAdmin.authority())); + // clone and setup release specific release admin clonedAdmin = ReleaseAdmin( - Clones.clone( - address(_masterReleaseAdmin) - ) - ); + Clones.clone(address(_masterReleaseAdmin))); + + string memory releaseAdminName = string( + abi.encodePacked( + "ReleaseAdmin_v", + release.toString())); + + clonedAdmin.initialize( + address(clonedAccessManager), + releaseAdminName); - clonedAdmin.initialize(clonedAccessManager); clonedAdmin.completeSetup( address(_registry), address(this), // release registry (this contract) diff --git a/contracts/registry/ServiceAuthorizationV3.sol b/contracts/registry/ServiceAuthorizationV3.sol index b0b32b50c..32bc7f410 100644 --- a/contracts/registry/ServiceAuthorizationV3.sol +++ b/contracts/registry/ServiceAuthorizationV3.sol @@ -130,6 +130,9 @@ contract ServiceAuthorizationV3 { IAccess.FunctionInfo[] storage functions; functions = _authorizeForService(INSTANCE(), ALL()); + _authorize(functions, IInstanceService.setInstanceLocked.selector, "setInstanceLocked"); + _authorize(functions, IInstanceService.setTargetLocked.selector, "setTargetLocked"); + _authorize(functions, IInstanceService.createInstance.selector, "createInstance"); _authorize(functions, IInstanceService.upgradeInstanceReader.selector, "upgradeInstanceReader"); _authorize(functions, IInstanceService.upgradeMasterInstanceReader.selector, "upgradeMasterInstanceReader"); @@ -181,7 +184,7 @@ contract ServiceAuthorizationV3 _authorize(functions, IComponentService.registerComponent.selector, "registerComponent"); _authorize(functions, IComponentService.approveTokenHandler.selector, "approveTokenHandler"); _authorize(functions, IComponentService.setWallet.selector, "setWallet"); - _authorize(functions, IComponentService.setComponentLocked.selector, "setComponentLocked"); + _authorize(functions, IComponentService.setLocked.selector, "setLocked"); _authorize(functions, IComponentService.withdrawFees.selector, "withdrawFees"); _authorize(functions, IComponentService.registerProduct.selector, "registerProduct"); _authorize(functions, IComponentService.setProductFees.selector, "setProductFees"); diff --git a/contracts/shared/Component.sol b/contracts/shared/Component.sol index c5b2f8ca0..44dbae347 100644 --- a/contracts/shared/Component.sol +++ b/contracts/shared/Component.sol @@ -186,7 +186,8 @@ abstract contract Component is { } - /// @dev depending on the source of the component information this function needs to be overwritten. + /// @dev Sets the components wallet to the specified address. + /// Depending on the source of the component information this function needs to be overwritten. /// eg for instance linked components that externally store this information with the instance store contract function _setWallet( address newWallet @@ -201,7 +202,7 @@ abstract contract Component is internal virtual { - _getComponentStorage()._componentService.setComponentLocked(address(this), locked); + _getComponentStorage()._componentService.setLocked(locked); } diff --git a/contracts/shared/ComponentService.sol b/contracts/shared/ComponentService.sol index 096f2cd91..39b26f21c 100644 --- a/contracts/shared/ComponentService.sol +++ b/contracts/shared/ComponentService.sol @@ -144,20 +144,24 @@ contract ComponentService is } /// @inheritdoc IComponentService - function setComponentLocked(address componentAddress, bool locked) + function setLocked(bool locked) external virtual restricted() { - // TODO inactive component can lock/unlock other components? (, IInstance instance) = _getAndVerifyComponent(COMPONENT(), false); - instance.setLockedFromService(componentAddress, locked); + + address component = msg.sender; + instance.getInstanceAdmin().setComponentLocked( + component, + locked); } /// @inheritdoc IComponentService function withdrawFees(Amount amount) external virtual + restricted() returns (Amount withdrawnAmount) { // checks @@ -492,26 +496,29 @@ contract ComponentService is } // deploy and wire token handler - IComponents.ComponentInfo memory componentInfo = component.getInitialComponentInfo(); - IERC20Metadata token = componentInfo.token; - componentInfo.tokenHandler = TokenHandlerDeployerLib.deployTokenHandler( - address(getRegistry()), - address(component), // initially, component is its own wallet - address(token), - address(instanceAdmin.authority())); - - // register component with instance - instanceStore.createComponent( - componentNftId, - componentInfo); + address token; + { + IComponents.ComponentInfo memory componentInfo = component.getInitialComponentInfo(); + token = address(componentInfo.token); + componentInfo.tokenHandler = TokenHandlerDeployerLib.deployTokenHandler( + address(getRegistry()), + componentAddress, // initially, component is its own wallet + token, + address(instanceAdmin.authority())); + + // register component with instance + instanceStore.createComponent( + componentNftId, + componentInfo); + } // link component contract to nft id component.linkToRegisteredNftId(); // authorize - instanceAdmin.initializeComponentAuthorization(component); + instanceAdmin.initializeComponentAuthorization(componentAddress, requiredType); - emit LogComponentServiceRegistered(instanceNftId, componentNftId, requiredType, address(component), address(token), initialOwner); + emit LogComponentServiceRegistered(instanceNftId, componentNftId, requiredType, componentAddress, token, initialOwner); } diff --git a/contracts/shared/ContractLib.sol b/contracts/shared/ContractLib.sol index 169036350..51dac1cc6 100644 --- a/contracts/shared/ContractLib.sol +++ b/contracts/shared/ContractLib.sol @@ -170,6 +170,16 @@ library ContractLib { } + function isRegistered(address registry, address caller, ObjectType expectedType) public view returns (bool) { + NftId nftId = IRegistry(registry).getNftIdForAddress(caller); + if (nftId.eqz()) { + return false; + } + + return IRegistry(registry).getObjectInfo(nftId).objectType == expectedType; + } + + function isRegistry(address registry) public view returns (bool) { if (!isContract(registry)) { return false; diff --git a/contracts/shared/IComponentService.sol b/contracts/shared/IComponentService.sol index ecea46459..8417afa01 100644 --- a/contracts/shared/IComponentService.sol +++ b/contracts/shared/IComponentService.sol @@ -67,11 +67,12 @@ interface IComponentService is /// Reverts if the component's token handler wallet is not the token handler itself. function approveTokenHandler(IERC20Metadata token, Amount amount) external; - /// @dev Sets the components associated wallet address + /// @dev Sets the components associated wallet address. + /// To set the wallet to the token handler contract, use address(0) as the new wallet adress. function setWallet(address newWallet) external; - /// @dev Locks/Unlocks the given component - call from component - function setComponentLocked(address componentAddress, bool locked) external; + /// @dev Locks/Unlocks the calling component. + function setLocked(bool locked) external; /// @dev Withdraw fees from the distribution component. Only component owner is allowed to withdraw fees. /// @param withdrawAmount the amount to withdraw diff --git a/contracts/shared/InitializableERC165.sol b/contracts/shared/InitializableERC165.sol index 514ede5a9..11750040a 100644 --- a/contracts/shared/InitializableERC165.sol +++ b/contracts/shared/InitializableERC165.sol @@ -11,13 +11,21 @@ contract InitializableERC165 is mapping(bytes4 => bool) private _isSupported; // @dev initializes with support for ERC165 - function _initializeERC165() internal onlyInitializing() { + function __ERC165_init() internal onlyInitializing() { + _initializeERC165(); + } + + function _initializeERC165() internal { _isSupported[type(IERC165).interfaceId] = true; } // @dev register support for provided interfaceId // includes initialization for ERC165_ID if not yet done function _registerInterface(bytes4 interfaceId) internal onlyInitializing() { + _registerInterfaceNotInitializing(interfaceId); + } + + function _registerInterfaceNotInitializing(bytes4 interfaceId) internal{ _isSupported[interfaceId] = true; } diff --git a/contracts/shared/NftOwnable.sol b/contracts/shared/NftOwnable.sol index abd7c1967..4771ba719 100644 --- a/contracts/shared/NftOwnable.sol +++ b/contracts/shared/NftOwnable.sol @@ -57,8 +57,8 @@ contract NftOwnable is virtual onlyInitializing() { + __ERC165_init(); __RegistryLinked_init(registry); - _initializeERC165(); if(initialOwner == address(0)) { revert ErrorNftOwnableInitialOwnerZero(); diff --git a/contracts/shared/Registerable.sol b/contracts/shared/Registerable.sol index f2f043d6a..870c00471 100644 --- a/contracts/shared/Registerable.sol +++ b/contracts/shared/Registerable.sol @@ -26,7 +26,6 @@ abstract contract Registerable is struct RegisterableStorage { NftId _parentNftId; ObjectType _objectType; - VersionPart _release; bool _isInterceptor; bytes _data; } @@ -61,7 +60,6 @@ abstract contract Registerable is RegisterableStorage storage $ = _getRegisterableStorage(); $._parentNftId = parentNftId; $._objectType = objectType; - $._release = AccessManagerCloneable(authority).getRelease(); $._isInterceptor = isInterceptor; $._data = data; @@ -77,8 +75,7 @@ abstract contract Registerable is /// @inheritdoc IRelease function getRelease() public virtual view returns (VersionPart release) { - RegisterableStorage storage $ = _getRegisterableStorage(); - return $._release; + return AccessManagerCloneable(authority()).getRelease(); } diff --git a/contracts/shared/TokenHandler.sol b/contracts/shared/TokenHandler.sol index edc0df6a7..e4944c48c 100644 --- a/contracts/shared/TokenHandler.sol +++ b/contracts/shared/TokenHandler.sol @@ -270,11 +270,12 @@ contract TokenHandler is AccessManaged(authority) { } - /// @dev sets the wallet address for the component. - /// if the current wallet has tokens, these will be transferred. - /// if the new wallet address is externally owned, an approval from the + /// @dev Sets the wallet address for the component. + /// Seeting the new wallet address to address(0) will set the wallet to the tokenHandler contract itself. + /// If the current wallet has tokens, these will be transferred. + /// If the new wallet address is externally owned, an approval from the /// owner of the external wallet to the tokenhandler of the component that - /// covers the current component balance must exist + /// covers the current component balance must exist. function setWallet(address newWallet) external restricted() diff --git a/contracts/staking/IStaking.sol b/contracts/staking/IStaking.sol index 7696f22bd..067b1bcb9 100644 --- a/contracts/staking/IStaking.sol +++ b/contracts/staking/IStaking.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.20; +import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; + import {Amount} from "../type/Amount.sol"; import {IComponent} from "../shared/IComponent.sol"; import {IVersionable} from "../upgradeability/IVersionable.sol"; @@ -69,6 +71,8 @@ interface IStaking is function initializeTokenHandler() external; + function approveTokenHandler(IERC20Metadata token, Amount amount) external; + // staking rate management /// @dev sets the rate that converts 1 token of total value locked into the diff --git a/contracts/type/ObjectType.sol b/contracts/type/ObjectType.sol index c5fc67d0a..be528ea26 100644 --- a/contracts/type/ObjectType.sol +++ b/contracts/type/ObjectType.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.20; +import {VersionPart} from "./Version.sol"; + type ObjectType is uint8; // type bindings @@ -161,8 +163,6 @@ function neObjectType(ObjectType a, ObjectType b) pure returns (bool isSame) { // library functions that operate on user defined type library ObjectTypeLib { - error ErrorVersionTooBig(uint256 version); - function zero() public pure returns (ObjectType) { return ObjectType.wrap(0); } @@ -242,19 +242,15 @@ library ObjectTypeLib { function toVersionedName( string memory name, string memory suffix, - uint256 version + VersionPart release ) external pure returns (string memory versionedName) { - if (version > maxNumReleases()) { - revert ErrorVersionTooBig(version); - } - string memory versionName = "_v0"; - if (version >= 10) { + if (release.toInt() >= 10) { versionName = "_v"; } @@ -263,12 +259,7 @@ library ObjectTypeLib { name, suffix, versionName, - toString(version))); - } - - /// @dev returns the max number of releases (major versions) this gif setup can handle. - function maxNumReleases() public pure returns (uint8) { - return 99; + release.toString())); } /// @dev returns the provied int as a string diff --git a/contracts/type/Version.sol b/contracts/type/Version.sol index f964365ed..f6fef4901 100644 --- a/contracts/type/Version.sol +++ b/contracts/type/Version.sol @@ -8,7 +8,9 @@ using { versionPartEq as ==, versionPartNe as !=, VersionPartLib.eqz, + VersionPartLib.gtz, VersionPartLib.toInt, + VersionPartLib.toString, VersionPartLib.isValidRelease } for VersionPart global; @@ -18,6 +20,9 @@ function versionPartEq(VersionPart a, VersionPart b) pure returns(bool isSame) { function versionPartNe(VersionPart a, VersionPart b) pure returns(bool isSame) { return VersionPart.unwrap(a) != VersionPart.unwrap(b); } library VersionPartLib { + + error ErrorReleaseTooBig(VersionPart releaseMax, VersionPart release); + function releaseMin() public pure returns (VersionPart) { return toVersionPart(3); } function releaseMax() public pure returns (VersionPart) { return toVersionPart(99); } @@ -26,7 +31,41 @@ library VersionPartLib { return 3 <= releaseInt && releaseInt <= 99; } + function toString(VersionPart a) external pure returns (string memory) { + if (a > releaseMax()) { + revert ErrorReleaseTooBig(releaseMax(), a); + } + + uint256 value = VersionPart.unwrap(a); + if (value == 0) { + return "0"; + } + + uint256 temp = value; + uint256 digits = 0; + while (temp != 0) { + digits++; + temp /= 10; + } + + bytes memory buffer = new bytes(digits); + uint index = digits - 1; + + temp = value; + while (temp != 0) { + buffer[index] = bytes1(uint8(48 + temp % 10)); + temp /= 10; + + if (index > 0) { + index--; + } + } + + return string(buffer); + } + function eqz(VersionPart a) external pure returns(bool) { return VersionPart.unwrap(a) == 0; } + function gtz(VersionPart a) external pure returns(bool) { return VersionPart.unwrap(a) > 0; } function toInt(VersionPart a) external pure returns(uint256) { return VersionPart.unwrap(a); } function toVersionPart(uint256 a) public pure returns(VersionPart) { return VersionPart.wrap(uint8(a)); } } diff --git a/scripts/libs/registry.ts b/scripts/libs/registry.ts index cb885a686..e5f04f37e 100644 --- a/scripts/libs/registry.ts +++ b/scripts/libs/registry.ts @@ -87,6 +87,7 @@ export async function deployAndInitializeRegistry(owner: Signer, libraries: Libr { libraries: { ContractLib: libraries.contractLibAddress, + NftIdLib: libraries.nftIdLibAddress, RoleIdLib: libraries.roleIdLibAddress, SelectorLib: libraries.selectorLibAddress, SelectorSetLib: libraries.selectorSetLibAddress, diff --git a/test/authorization/AccessAdmin.t.sol b/test/authorization/AccessAdmin.t.sol index 0a5cb87fc..4eb6e17e5 100644 --- a/test/authorization/AccessAdmin.t.sol +++ b/test/authorization/AccessAdmin.t.sol @@ -31,7 +31,9 @@ contract AccessAdminForTesting is AccessAdmin { // constructor as in registry admin constructor() { - initialize(new AccessManagerCloneable()); + initialize( + address(new AccessManagerCloneable()), + "TestAdmin"); } function completeSetup( @@ -210,7 +212,7 @@ contract AccessAdminForTesting is AccessAdmin { virtual restricted() { - _setTargetClosed(target, locked); + _setTargetLocked(target, locked); // implizit logging: rely on OpenZeppelin log TargetClosed } @@ -220,6 +222,7 @@ contract AccessAdminForTesting is AccessAdmin { } } + contract AccessAdminBaseTest is Test { address public accessAdminDeployer = makeAddr("accessAdminDeployer"); @@ -339,7 +342,7 @@ contract AccessAdminTest is AccessAdminBaseTest { address accessAdminTarget = address(accessAdmin); vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorTargetAlreadyCreated.selector, + IAccessAdmin.ErrorAccessAdminTargetAlreadyCreated.selector, accessAdminTarget, targetName)); @@ -351,7 +354,7 @@ contract AccessAdminTest is AccessAdminBaseTest { address invalidTarget = makeAddr("invalidContract"); vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorTargetNotAccessManaged.selector, + IAccessAdmin.ErrorAccessAdminTargetNotAccessManaged.selector, invalidTarget)); vm.startPrank(accessAdminDeployer); @@ -361,7 +364,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // attempt to create target with empty name vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorTargetNameEmpty.selector, + IAccessAdmin.ErrorAccessAdminTargetNameEmpty.selector, address(this))); vm.startPrank(accessAdminDeployer); @@ -371,7 +374,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // attempt to create target with existing name vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorTargetNameAlreadyExists.selector, + IAccessAdmin.ErrorAccessAdminTargetNameAlreadyExists.selector, address(this), targetName, accessAdminTarget)); @@ -483,7 +486,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // WHEN + THEN - attempt to add 2nd role member vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleMembersLimitReached.selector, + IAccessAdmin.ErrorAccessAdminRoleMembersLimitReached.selector, newRoleId, maxOneRoleMember)); @@ -493,7 +496,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // WHEN + THEN - attempt to revoke role vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleMemberRemovalDisabled.selector, + IAccessAdmin.ErrorAccessAdminRoleMemberRemovalDisabled.selector, newRoleId, thisContract)); @@ -503,7 +506,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // WHEN + THEN - attempt to renounce role vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleMemberRemovalDisabled.selector, + IAccessAdmin.ErrorAccessAdminRoleMemberRemovalDisabled.selector, newRoleId, thisContract)); @@ -523,7 +526,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // attempt to recreate admin role vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleAlreadyCreated.selector, + IAccessAdmin.ErrorAccessAdminRoleAlreadyCreated.selector, adminRole, "AdminRole")); @@ -536,7 +539,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // attempt to recreate public role vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleAlreadyCreated.selector, + IAccessAdmin.ErrorAccessAdminRoleAlreadyCreated.selector, publicRole, "PublicRole")); @@ -549,7 +552,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // attempt to recreate manager role vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleAlreadyCreated.selector, + IAccessAdmin.ErrorAccessAdminRoleAlreadyCreated.selector, managerRole, "ManagerRole")); @@ -579,7 +582,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // WHEN + THEN - use existing role id vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleAlreadyCreated.selector, + IAccessAdmin.ErrorAccessAdminRoleAlreadyCreated.selector, newRoleId, newRoleName)); @@ -594,7 +597,7 @@ contract AccessAdminTest is AccessAdminBaseTest { RoleId otherRoleId = RoleIdLib.toRoleId(123); vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleNameAlreadyExists.selector, + IAccessAdmin.ErrorAccessAdminRoleNameAlreadyExists.selector, otherRoleId, newRoleName, newRoleId)); @@ -618,7 +621,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // WHEN + THEN vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleNameEmpty.selector, + IAccessAdmin.ErrorAccessAdminRoleNameEmpty.selector, newRoleId)); vm.startPrank(accessAdminDeployer); @@ -631,7 +634,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // WHEN + THEN vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleAdminNotExisting.selector, + IAccessAdmin.ErrorAccessAdminRoleAdminNotExisting.selector, missingAdminRoleId)); vm.startPrank(accessAdminDeployer); @@ -792,7 +795,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // WHEN + THEN vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorNotAdminOfRole.selector, + IAccessAdmin.ErrorAccessAdminNotAdminOfRole.selector, newAdminRoleId)); vm.startPrank(accessAdminDeployer); @@ -843,7 +846,7 @@ contract AccessAdminTest is AccessAdminBaseTest { vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorNotAdminOfRole.selector, + IAccessAdmin.ErrorAccessAdminNotAdminOfRole.selector, newAdminRoleId)); vm.startPrank(roleAdmin); @@ -963,7 +966,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // remove role account1 no longer has vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorNotRoleOwner.selector, + IAccessAdmin.ErrorAccessAdminNotRoleOwner.selector, newRoleId)); accessAdmin.renounceRole(newRoleId); @@ -983,7 +986,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // remove role account2 no longer has vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorNotRoleOwner.selector, + IAccessAdmin.ErrorAccessAdminNotRoleOwner.selector, newRoleId)); accessAdmin.renounceRole(newRoleId); @@ -1029,7 +1032,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // granting non existent role -> role unknown vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleUnknown.selector, + IAccessAdmin.ErrorAccessAdminRoleUnknown.selector, missingRoleId)); accessAdmin.grantRole(outsider, missingRoleId); @@ -1037,7 +1040,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // revoking non existent role -> role unknown vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleUnknown.selector, + IAccessAdmin.ErrorAccessAdminRoleUnknown.selector, missingRoleId)); accessAdmin.revokeRole(outsider, missingRoleId); @@ -1049,7 +1052,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // renouncing non existent role -> role unknown vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorNotRoleOwner.selector, + IAccessAdmin.ErrorAccessAdminNotRoleOwner.selector, missingRoleId)); accessAdmin.renounceRole(missingRoleId); @@ -1070,7 +1073,7 @@ contract AccessAdminTest is AccessAdminBaseTest { console.log("attempt to grant admin role"); vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleIsLocked.selector, + IAccessAdmin.ErrorAccessAdminRoleIsLocked.selector, adminRole)); accessAdmin.grantRole(outsider, adminRole); @@ -1079,7 +1082,7 @@ contract AccessAdminTest is AccessAdminBaseTest { console.log("attempt to revoke admin role"); vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleIsLocked.selector, + IAccessAdmin.ErrorAccessAdminRoleIsLocked.selector, adminRole)); accessAdmin.revokeRole(outsider, adminRole); @@ -1088,7 +1091,7 @@ contract AccessAdminTest is AccessAdminBaseTest { console.log("attempt to renounce admin role"); vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorRoleIsLocked.selector, + IAccessAdmin.ErrorAccessAdminRoleIsLocked.selector, adminRole)); accessAdmin.renounceRole(adminRole); diff --git a/test/authorization/AccessAdminManageMock.t.sol b/test/authorization/AccessAdminManageMock.t.sol index afa691d50..2071a1dd1 100644 --- a/test/authorization/AccessAdminManageMock.t.sol +++ b/test/authorization/AccessAdminManageMock.t.sol @@ -157,7 +157,7 @@ contract AccessAdminManageMockTest is AccessAdminBaseTest { vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorAuthorizeForAdminRoleInvalid.selector, + IAccessAdmin.ErrorAccessAdminAuthorizeForAdminRoleInvalid.selector, target)); vm.startPrank(accessAdminDeployer); diff --git a/test/authorization/AccessManagerCloneable.t.sol b/test/authorization/AccessManagerCloneable.t.sol index 5d5296324..aa4b6e906 100644 --- a/test/authorization/AccessManagerCloneable.t.sol +++ b/test/authorization/AccessManagerCloneable.t.sol @@ -4,202 +4,34 @@ pragma solidity ^0.8.20; import {IAccessManaged} from "../../lib/openzeppelin-contracts/contracts/access/manager/IAccessManaged.sol"; import {Test, console} from "../../lib/forge-std/src/Test.sol"; -import {VersionPart, VersionPartLib} from "../../contracts/type/Version.sol"; import {AccessManagerCloneable} from "../../contracts/authorization/AccessManagerCloneable.sol"; - +import {AccessManagedMock} from "../mock/AccessManagedMock.sol"; +import {ContractLib} from "../../contracts/shared/ContractLib.sol"; import {RegistryAdmin} from "../../contracts/registry/RegistryAdmin.sol"; import {Registry} from "../../contracts/registry/Registry.sol"; +import {VersionPart, VersionPartLib} from "../../contracts/type/Version.sol"; -import {AccessManagedMock} from "../mock/AccessManagedMock.sol"; +contract AccessManagerCloneableTesting is AccessManagerCloneable { + function setRelase(VersionPart release) public { + _checkAndSetRelease(release); + } +} contract AccessManagerCloneableTest is Test { - - address globalRegistry = makeAddr("globalRegistry"); - address admin = makeAddr("accessManagerAdmin"); - address accessManagedCaller = makeAddr("accessManagedCaller"); - address outsider = makeAddr("outsider"); - - - RegistryAdmin registryAdmin; - Registry registry; - AccessManagerCloneable accessManager; - AccessManagedMock accessManaged; - function setUp() public - { - registryAdmin = new RegistryAdmin(); - registry = new Registry(registryAdmin, globalRegistry); + address public admin = makeAddr("accessManagerAdmin"); + AccessManagerCloneableTesting public accessManager; - accessManager = new AccessManagerCloneable(); + function setUp() public virtual { + accessManager = new AccessManagerCloneableTesting(); accessManager.initialize(admin); - - VersionPart version = VersionPartLib.toVersionPart(3); - vm.prank(admin); - accessManager.completeSetup(address(registry), version); - - accessManaged = new AccessManagedMock(address(accessManager)); - - // set role 1 for accessManaged.incrementCounter1() - bytes4[] memory selector = new bytes4[](1); - selector[0] = AccessManagedMock.increaseCounter1.selector; - vm.prank(admin); - accessManager.setTargetFunctionRole(address(accessManaged), selector, 1); - - // grant role 1 to accessManagedCaller - vm.prank(admin); - accessManager.grantRole(1, accessManagedCaller, 0); + accessManager.setRelase(VersionPartLib.toVersionPart(3)); } - function test_accessManagerCloneableLockReleaseByUnauthorizedCaller() public { - assertFalse(accessManager.isLocked(), "Release should be unlocked"); - - vm.expectRevert(abi.encodeWithSelector(AccessManagerCloneable.ErrorAccessManagerCallerNotAdmin.selector, accessManagedCaller)); - vm.prank(accessManagedCaller); - accessManager.setLocked(true); - - - vm.expectRevert(abi.encodeWithSelector(AccessManagerCloneable.ErrorAccessManagerCallerNotAdmin.selector, outsider)); - vm.prank(outsider); - accessManager.setLocked(true); - } - - function test_accessManagerCloneableUnlockReleaseByUnauthorizedCaller() public { - assertFalse(accessManager.isLocked(), "Release should be unlocked"); - - vm.expectRevert(abi.encodeWithSelector(AccessManagerCloneable.ErrorAccessManagerCallerNotAdmin.selector, accessManagedCaller)); - vm.prank(accessManagedCaller); - accessManager.setLocked(false); - - vm.expectRevert(abi.encodeWithSelector(AccessManagerCloneable.ErrorAccessManagerCallerNotAdmin.selector, outsider)); - vm.prank(outsider); - accessManager.setLocked(false); - } - - function test_accessManagerCloneableLockReleaseWhenReleaseUnlockedHappyCase() public { - assertFalse(accessManager.isLocked(), "Release should be unlocked"); - - vm.prank(admin); - accessManager.setLocked(true); - - assertTrue(accessManager.isLocked(), "Release should be locked"); - - assertEq(accessManaged.counter1(), 0, "unexpected counter value (before)"); - - vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, accessManagedCaller)); - vm.prank(accessManagedCaller); - accessManaged.increaseCounter1(); - - assertEq(accessManaged.counter1(), 0, "unexpected counter value (after)"); - } - - function test_accessManagerCloneableLockReleaseWhenReleaseLockedHappyCase() public { - assertFalse(accessManager.isLocked(), "Release should be unlocked"); - - // first lock - vm.prank(admin); - accessManager.setLocked(true); - - assertTrue(accessManager.isLocked(), "Release should be locked"); - - // second lock - vm.prank(admin); - accessManager.setLocked(true); - - assertTrue(accessManager.isLocked(), "Release should be locked"); - - // call after second lock - vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, accessManagedCaller)); - vm.prank(accessManagedCaller); - accessManaged.increaseCounter1(); - } - - function test_accessManagerCloneableUnlockReleaseWhenReleaseLockedHappyCase() public { - assertFalse(accessManager.isLocked(), "Release should be unlocked"); - - // lock - vm.prank(admin); - accessManager.setLocked(true); - - assertTrue(accessManager.isLocked(), "Release should be locked"); - - // unlock - vm.prank(admin); - accessManager.setLocked(false); - - assertFalse(accessManager.isLocked(), "Release should be unlocked"); - - // call after unlock - vm.prank(accessManagedCaller); - accessManaged.increaseCounter1(); - assertEq(accessManaged.counter1(), 1); - } - - function test_accessManagerCloneableUnlockReleaseWhenReleaseUnlockedHappyCase() public { - assertFalse(accessManager.isLocked(), "Release should be unlocked"); - - // lock - vm.prank(admin); - accessManager.setLocked(true); - - // first unlock - vm.prank(admin); - accessManager.setLocked(false); - - // second unlock - vm.prank(admin); - accessManager.setLocked(false); - - assertFalse(accessManager.isLocked(), "Release should be unlocked"); - - // call after second unlock - vm.prank(accessManagedCaller); - accessManaged.increaseCounter1(); - assertEq(accessManaged.counter1(), 1); - } - - function test_accessManagerCloneableCallAccessManagedWhenReleaseUnlockedHappyCase() public { - assertFalse(accessManager.isLocked(), "Release should be unlocked"); - - vm.prank(accessManagedCaller); - accessManaged.increaseCounter1(); - assertEq(accessManaged.counter1(), 1); - } - - function test_accessManagerCloneableCallAccessManagedWhenReleaseLocked() public { - assertFalse(accessManager.isLocked(), "Release should be unlocked"); - - assertEq(accessManaged.counter1(), 0, "unexpected counter value (before)"); - - // call before lock - vm.prank(accessManagedCaller); - accessManaged.increaseCounter1(); - - assertEq(accessManaged.counter1(), 1, "unexpected counter value (after)"); - - // lock - vm.prank(admin); - accessManager.setLocked(true); - - // call after lock - vm.expectRevert( - abi.encodeWithSelector( - IAccessManaged.AccessManagedUnauthorized.selector, - accessManagedCaller)); - - vm.prank(accessManagedCaller); - accessManaged.increaseCounter1(); - } - - function test_accessManagerCloneableCallAccessManagedWhenReleaseUnlocked_byUnauthorizedCaller() public { - assertFalse(accessManager.isLocked(), "Release should be unlocked"); - - vm.prank(admin); - vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, admin)); - accessManaged.increaseCounter1(); - vm.prank(outsider); - vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, outsider)); - accessManaged.increaseCounter1(); + function test_addressManagerCloneableSetUp() public { + assertTrue(ContractLib.isAuthority(address(accessManager)), "unexpected authority"); + assertEq(accessManager.getRelease().toInt(), 3, "unexpected release"); } -} \ No newline at end of file +} diff --git a/test/authorization/AccessManagerCloneableEx.t.sol b/test/authorization/AccessManagerCloneableEx.t.sol new file mode 100644 index 000000000..e1fb3bcb5 --- /dev/null +++ b/test/authorization/AccessManagerCloneableEx.t.sol @@ -0,0 +1,203 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.20; + +import {IAccessManaged} from "../../lib/openzeppelin-contracts/contracts/access/manager/IAccessManaged.sol"; +import {Test, console} from "../../lib/forge-std/src/Test.sol"; + + +import {AccessManagerCloneable} from "../../contracts/authorization/AccessManagerCloneable.sol"; +import {AccessManagerCloneableTest} from "./AccessManagerCloneable.t.sol"; +import {AccessManagedMock} from "../mock/AccessManagedMock.sol"; +import {RegistryAdmin} from "../../contracts/registry/RegistryAdmin.sol"; +import {Registry} from "../../contracts/registry/Registry.sol"; +import {VersionPart, VersionPartLib} from "../../contracts/type/Version.sol"; + + +contract AccessManagerCloneableExtendedTest is Test { + + address public globalRegistry = makeAddr("globalRegistry"); + address public accessManagedCaller = makeAddr("accessManagedCaller"); + address public outsider = makeAddr("outsider"); + + address public admin = makeAddr("accessManagerAdmin"); + AccessManagerCloneable public accessManager; + + RegistryAdmin registryAdmin; + Registry registry; + AccessManagedMock accessManaged; + + function setUp() public { + accessManager = new AccessManagerCloneable(); + accessManager.initialize(admin); + + registryAdmin = new RegistryAdmin(); + registry = new Registry(registryAdmin, globalRegistry); + + VersionPart version = VersionPartLib.toVersionPart(3); + vm.prank(admin); + accessManager.completeSetup(address(registry), version); + + accessManaged = new AccessManagedMock(address(accessManager)); + + // set role 1 for accessManaged.incrementCounter1() + bytes4[] memory selector = new bytes4[](1); + selector[0] = AccessManagedMock.increaseCounter1.selector; + vm.prank(admin); + accessManager.setTargetFunctionRole(address(accessManaged), selector, 1); + + // grant role 1 to accessManagedCaller + vm.prank(admin); + accessManager.grantRole(1, accessManagedCaller, 0); + } + + function test_accessManagerCloneableLockReleaseByUnauthorizedCaller() public { + assertFalse(accessManager.isLocked(), "Release should be unlocked"); + + vm.expectRevert(abi.encodeWithSelector(AccessManagerCloneable.ErrorAccessManagerCallerNotAdmin.selector, accessManagedCaller)); + vm.prank(accessManagedCaller); + accessManager.setLocked(true); + + vm.expectRevert(abi.encodeWithSelector(AccessManagerCloneable.ErrorAccessManagerCallerNotAdmin.selector, outsider)); + vm.prank(outsider); + accessManager.setLocked(true); + } + + function test_accessManagerCloneableUnlockReleaseByUnauthorizedCaller() public { + assertFalse(accessManager.isLocked(), "Release should be unlocked"); + + vm.expectRevert(abi.encodeWithSelector(AccessManagerCloneable.ErrorAccessManagerCallerNotAdmin.selector, accessManagedCaller)); + vm.prank(accessManagedCaller); + accessManager.setLocked(false); + + vm.expectRevert(abi.encodeWithSelector(AccessManagerCloneable.ErrorAccessManagerCallerNotAdmin.selector, outsider)); + vm.prank(outsider); + accessManager.setLocked(false); + } + + function test_accessManagerCloneableLockReleaseWhenReleaseUnlockedHappyCase() public { + assertFalse(accessManager.isLocked(), "Release should be unlocked"); + + vm.prank(admin); + accessManager.setLocked(true); + + assertTrue(accessManager.isLocked(), "Release should be locked"); + + assertEq(accessManaged.counter1(), 0, "unexpected counter value (before)"); + + vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, accessManagedCaller)); + vm.prank(accessManagedCaller); + accessManaged.increaseCounter1(); + + assertEq(accessManaged.counter1(), 0, "unexpected counter value (after)"); + } + + function test_accessManagerCloneableLockReleaseWhenReleaseLockedHappyCase() public { + assertFalse(accessManager.isLocked(), "Release should be unlocked"); + + // first lock + vm.prank(admin); + accessManager.setLocked(true); + + assertTrue(accessManager.isLocked(), "Release should be locked"); + + // second lock + vm.prank(admin); + accessManager.setLocked(true); + + assertTrue(accessManager.isLocked(), "Release should be locked"); + + // call after second lock + vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, accessManagedCaller)); + vm.prank(accessManagedCaller); + accessManaged.increaseCounter1(); + } + + function test_accessManagerCloneableUnlockReleaseWhenReleaseLockedHappyCase() public { + assertFalse(accessManager.isLocked(), "Release should be unlocked"); + + // lock + vm.prank(admin); + accessManager.setLocked(true); + + assertTrue(accessManager.isLocked(), "Release should be locked"); + + // unlock + vm.prank(admin); + accessManager.setLocked(false); + + assertFalse(accessManager.isLocked(), "Release should be unlocked"); + + // call after unlock + vm.prank(accessManagedCaller); + accessManaged.increaseCounter1(); + assertEq(accessManaged.counter1(), 1); + } + + function test_accessManagerCloneableUnlockReleaseWhenReleaseUnlockedHappyCase() public { + assertFalse(accessManager.isLocked(), "Release should be unlocked"); + + // lock + vm.prank(admin); + accessManager.setLocked(true); + + // first unlock + vm.prank(admin); + accessManager.setLocked(false); + + // second unlock + vm.prank(admin); + accessManager.setLocked(false); + + assertFalse(accessManager.isLocked(), "Release should be unlocked"); + + // call after second unlock + vm.prank(accessManagedCaller); + accessManaged.increaseCounter1(); + assertEq(accessManaged.counter1(), 1); + } + + function test_accessManagerCloneableCallAccessManagedWhenReleaseUnlockedHappyCase() public { + assertFalse(accessManager.isLocked(), "Release should be unlocked"); + + vm.prank(accessManagedCaller); + accessManaged.increaseCounter1(); + assertEq(accessManaged.counter1(), 1); + } + + function test_accessManagerCloneableCallAccessManagedWhenReleaseLocked() public { + assertFalse(accessManager.isLocked(), "Release should be unlocked"); + + assertEq(accessManaged.counter1(), 0, "unexpected counter value (before)"); + + // call before lock + vm.prank(accessManagedCaller); + accessManaged.increaseCounter1(); + + assertEq(accessManaged.counter1(), 1, "unexpected counter value (after)"); + + // lock + vm.prank(admin); + accessManager.setLocked(true); + + // call after lock + vm.expectRevert( + abi.encodeWithSelector( + IAccessManaged.AccessManagedUnauthorized.selector, + accessManagedCaller)); + + vm.prank(accessManagedCaller); + accessManaged.increaseCounter1(); + } + + function test_accessManagerCloneableCallAccessManagedWhenReleaseUnlocked_byUnauthorizedCaller() public { + assertFalse(accessManager.isLocked(), "Release should be unlocked"); + + vm.prank(admin); + vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, admin)); + accessManaged.increaseCounter1(); + + vm.prank(outsider); + vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, outsider)); + accessManaged.increaseCounter1(); + } +} \ No newline at end of file diff --git a/test/authorization/InstanceAdmin.t.sol b/test/authorization/InstanceAdmin.t.sol deleted file mode 100644 index fb41941ab..000000000 --- a/test/authorization/InstanceAdmin.t.sol +++ /dev/null @@ -1,56 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -pragma solidity ^0.8.20; - -import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; -import {console} from "../../lib/forge-std/src/Script.sol"; - -import {AccessManagerCloneable} from "../../contracts/authorization/AccessManagerCloneable.sol"; -import {GifTest} from "../base/GifTest.sol"; -import {InstanceAdmin} from "../../contracts/instance/InstanceAdmin.sol"; -import {InstanceAuthorizationV3} from "../../contracts/instance/InstanceAuthorizationV3.sol"; -import {InstanceLinkedComponent} from "../../contracts/shared/InstanceLinkedComponent.sol"; -import {IRegistry} from "../../contracts/registry/IRegistry.sol"; -import {NftId, NftIdLib} from "../../contracts/type/NftId.sol"; -import {ObjectType} from "../../contracts/type/ObjectType.sol"; -import {BUNDLE, COMPONENT, DISTRIBUTION, ORACLE, POOL, PRODUCT, POLICY, RISK, REQUEST, SERVICE, STAKING} from "../../contracts/type/ObjectType.sol"; -import {RoleId} from "../../contracts/type/RoleId.sol"; -import {VersionPartLib} from "../../contracts/type/Version.sol"; - - -contract TestInstanceAdminMockInstance { - address public authority; - - constructor(address auth) { - authority = auth; - } -} - - -contract TestInstanceAdmin is - GifTest -{ - // address public someInstanceAuthz; - InstanceAdmin public someInstanceAdmin; - AccessManagerCloneable public someAccessManager; - - function setUp() public override { - super.setUp(); - - someAccessManager = AccessManagerCloneable( - Clones.clone(instance.authority())); - - someInstanceAdmin = InstanceAdmin( - Clones.clone( - address(instance.getInstanceAdmin()))); - - someInstanceAdmin.initialize( - someAccessManager, - instance.getRegistry(), - instance.getRelease()); - } - - function test_instanceAdminSetup() public { - _printAuthz(someInstanceAdmin, "instance"); - assertTrue(true, "something is wrong"); - } -} \ No newline at end of file diff --git a/test/authorization/RegistryAdminEx.sol b/test/authorization/RegistryAdminEx.sol index 3781dda2f..04ff51d2a 100644 --- a/test/authorization/RegistryAdminEx.sol +++ b/test/authorization/RegistryAdminEx.sol @@ -1,24 +1,32 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.20; -import {AccessManagedMock} from "../mock/AccessManagedMock.sol"; +import {IAuthorization} from "../../contracts/authorization/IAuthorization.sol"; import {IRegistry} from "../../contracts/registry/IRegistry.sol"; + +import {AccessManagedMock} from "../mock/AccessManagedMock.sol"; import {PUBLIC_ROLE} from "../../contracts/type/RoleId.sol"; import {RegistryAdmin} from "../../contracts/registry/RegistryAdmin.sol"; +import {RegistryAuthorization} from "../../contracts/registry/RegistryAuthorization.sol"; contract RegistryAdminEx is RegistryAdmin { AccessManagedMock public accessManagedMock; function completeSetup( - IRegistry registry, + address registry, + address authorization, address gifAdmin, address gifManager ) public virtual override { - super.completeSetup(registry, gifAdmin, gifManager); + super.completeSetup( + registry, + authorization, + gifAdmin, + gifManager); accessManagedMock = new AccessManagedMock(address(authority())); diff --git a/test/base/GifDeployer.sol b/test/base/GifDeployer.sol index 0ca51c18e..50811e4a5 100644 --- a/test/base/GifDeployer.sol +++ b/test/base/GifDeployer.sol @@ -5,22 +5,25 @@ import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IER import {AccessManager} from "@openzeppelin/contracts/access/manager/AccessManager.sol"; import {Test, console} from "../../lib/forge-std/src/Test.sol"; +import {IAccessAdmin} from "../../contracts/authorization/IAccessAdmin.sol"; +import {IAuthorization} from "../../contracts/authorization/IAuthorization.sol"; +import {IRegistry} from "../../contracts/registry/IRegistry.sol"; +import {IRelease} from "../../contracts/registry/IRelease.sol"; +import {IServiceAuthorization} from "../../contracts/authorization/IServiceAuthorization.sol"; + import {AmountLib} from "../../contracts/type/Amount.sol"; import {ObjectType, ObjectTypeLib} from "../../contracts/type/ObjectType.sol"; import {NftId, NftIdLib} from "../../contracts/type/NftId.sol"; import {ProxyManager} from "../../contracts/upgradeability/ProxyManager.sol"; import {SCHEDULED, DEPLOYING} from "../../contracts/type/StateId.sol"; import {VersionPart, VersionPartLib} from "../../contracts/type/Version.sol"; +import {RegistryAuthorization} from "../../contracts/registry/RegistryAuthorization.sol"; import {StateIdLib} from "../../contracts/type/StateId.sol"; import {TimestampLib} from "../../contracts/type/Timestamp.sol"; -import {IAccessAdmin} from "../../contracts/authorization/IAccessAdmin.sol"; // core contracts import {Dip} from "../../contracts/mock/Dip.sol"; -import {IRegistry} from "../../contracts/registry/IRegistry.sol"; -import {IRelease} from "../../contracts/registry/IRelease.sol"; -import {IServiceAuthorization} from "../../contracts/authorization/IServiceAuthorization.sol"; import {Registry} from "../../contracts/registry/Registry.sol"; import {RegistryAdmin} from "../../contracts/registry/RegistryAdmin.sol"; import {ReleaseRegistry} from "../../contracts/registry/ReleaseRegistry.sol"; @@ -128,6 +131,9 @@ contract GifDeployer is Test { mapping(ObjectType domain => DeployedServiceInfo info) public serviceForDomain; + // TODO cleanup + event LogDebug(string message, string value); + function deployCore( address globalRegistry, address gifAdmin, @@ -152,6 +158,7 @@ contract GifDeployer is Test { // 2) deploy registry admin registryAdmin = new RegistryAdmin(); + // registryAdmin.initialize(); // 3) deploy registry registry = new Registry(registryAdmin, globalRegistry); @@ -169,42 +176,37 @@ contract GifDeployer is Test { StakingStore stakingStore = new StakingStore(registry, stakingReader); // 8) deploy staking manager and staking component - bytes32 salt = bytes32(""); stakingManager = new StakingManager( address(registry), address(tokenRegistry), address(stakingStore), stakingOwner, - salt); + bytes32("")); // salt staking = stakingManager.getStaking(); // 9) initialize instance reader stakingReader.initialize( address(staking), address(stakingStore)); +emit LogDebug("F", ""); // 10) intialize registry and register staking component registry.initialize( address(releaseRegistry), address(tokenRegistry), address(staking)); +emit LogDebug("G", ""); + staking.linkToRegisteredNftId(); +emit LogDebug("H", ""); // 11) initialize registry admin - // TODO Consider making it non permitted - // no arguments - // cmp deployed contracts codehashes with precalculated ones - // check authority is the same - // check registry is the same - // whatever... - // Consider: specific completeSetup can do specific checks and require specific initial state of deployed contracts - // if state is different -> setup can not be completed... - // state: owner/admin/manager - // can be usefull for non permissioned deployment registryAdmin.completeSetup( - registry, + address(registry), + address(new RegistryAuthorization()), gifAdmin, gifManager); +emit LogDebug("I", ""); vm.stopPrank(); } diff --git a/test/base/GifTest.sol b/test/base/GifTest.sol index 770de9abf..dd0738f75 100644 --- a/test/base/GifTest.sol +++ b/test/base/GifTest.sol @@ -2,9 +2,12 @@ pragma solidity ^0.8.20; import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; - import {Test, console} from "../../lib/forge-std/src/Test.sol"; +import {IAccess} from "../../contracts/authorization/IAccess.sol"; +import {IAccessAdmin} from "../../contracts/authorization/IAccessAdmin.sol"; +import {IServiceAuthorization} from "../../contracts/authorization/IServiceAuthorization.sol"; + import {Amount, AmountLib} from "../../contracts/type/Amount.sol"; import {NftId, NftIdLib} from "../../contracts/type/NftId.sol"; import {SecondsLib} from "../../contracts/type/Seconds.sol"; @@ -13,10 +16,8 @@ import {UFixed, UFixedLib} from "../../contracts/type/UFixed.sol"; import {RoleId} from "../../contracts/type/RoleId.sol"; import {ACTIVE} from "../../contracts/type/StateId.sol"; import {Timestamp} from "../../contracts/type/Timestamp.sol"; +import {VersionPartLib} from "../../contracts/type/Version.sol"; -import {IAccess} from "../../contracts/authorization/IAccess.sol"; -import {IAccessAdmin} from "../../contracts/authorization/IAccessAdmin.sol"; -import {IServiceAuthorization} from "../../contracts/authorization/IServiceAuthorization.sol"; import {SimpleDistributionAuthorization} from "../../contracts/examples/unpermissioned/SimpleDistributionAuthorization.sol"; import {BasicOracleAuthorization} from "../../contracts/oracle/BasicOracleAuthorization.sol"; @@ -86,7 +87,7 @@ contract GifTest is GifDeployer { AccessManagerCloneable public masterAccessManager; InstanceAdmin public masterInstanceAdmin; - address public instanceAuthorizationV3; + InstanceAuthorizationV3 public instanceAuthorizationV3; BundleSet public masterBundleSet; RiskSet public masterRiskSet; InstanceStore public masterInstanceStore; @@ -303,18 +304,24 @@ contract GifTest is GifDeployer { assertEq(releaseRegistry.getState(releaseRegistry.getLatestVersion()).toInt(), ACTIVE().toInt(), "unexpected state for releaseRegistry after activateNextRelease"); } +// TODO cleanup logs +event LogDebugMaster(string key, string value); + function _deployMasterInstance() internal { // create instance supporting contracts - instanceAuthorizationV3 = address(new InstanceAuthorizationV3()); - masterInstanceAdmin = new InstanceAdmin(instanceAuthorizationV3); + masterAccessManager = new AccessManagerCloneable(); + masterInstanceAdmin = new InstanceAdmin(address(masterAccessManager)); + masterInstanceStore = new InstanceStore(); masterBundleSet = new BundleSet(); masterRiskSet = new RiskSet(); masterInstanceReader = new InstanceReader(); // crate instance +emit LogDebugMaster("1", "a"); masterInstance = new Instance(); +emit LogDebugMaster("1", "b"); masterInstance.initialize( masterInstanceAdmin, masterInstanceStore, @@ -322,15 +329,25 @@ contract GifTest is GifDeployer { masterRiskSet, masterInstanceReader, registry, + VersionPartLib.toVersionPart(3), registryOwner); - - // retrieve master access manager from instance - masterAccessManager = AccessManagerCloneable( - masterInstanceAdmin.authority()); - +emit LogDebugMaster("1", "c"); // sets master instance address in instance service // instance service is now ready to create cloned instances masterInstanceNftId = instanceService.setAndRegisterMasterInstance(address(masterInstance)); +emit LogDebugMaster("1", "d"); + // setup roles, targets and function grantings + instanceAuthorizationV3 = new InstanceAuthorizationV3(); +emit LogDebugMaster("1", "e"); + masterInstanceAdmin.completeSetup( + address(registry), + address(masterInstance), + address(instanceAuthorizationV3), + VersionPartLib.toVersionPart(3)); +emit LogDebugMaster("1", "f"); + + require(address(masterInstanceAdmin.getRegistry()) == address(registry), "unexpected master instance registry"); + // require(masterInstanceAdmin.getRelease().toInt() == 3, "unexpected master instance release"); // MUST be set after instance is set up and registered // TODO consider deleting this -> master instance just provides impl code, it own state must pe mot initialized @@ -637,6 +654,9 @@ contract GifTest is GifDeployer { { // solhint-disable no-console console.log("=========================================="); + console.log(aaName, registry.getObjectAddress(aa.getLinkedNftId())); + console.log(aaName, "nft id", aa.getLinkedNftId().toInt()); + console.log(aaName, "owner", aa.getLinkedOwner()); console.log(aaName, "admin authorization"); console.log(aaName, "admin contract:", address(aa)); console.log(aaName, "admin authority:", aa.authority()); diff --git a/test/instance/Instance.t.sol b/test/instance/Instance.t.sol index f6f40f97c..67496d7c7 100644 --- a/test/instance/Instance.t.sol +++ b/test/instance/Instance.t.sol @@ -2,207 +2,192 @@ pragma solidity ^0.8.20; import {IAccessManaged} from "@openzeppelin/contracts/access/manager/IAccessManaged.sol"; +import {console} from "../../lib/forge-std/src/Test.sol"; -import {Fee, FeeLib} from "../../contracts/type/Fee.sol"; -import {GifTest} from "../base/GifTest.sol"; import {IAccessAdmin} from "../../contracts/authorization/IAccessAdmin.sol"; +import {IComponentService} from "../../contracts/shared/IComponentService.sol"; import {IInstance} from "../../contracts/instance/IInstance.sol"; import {IInstanceService} from "../../contracts/instance/IInstanceService.sol"; +import {INftOwnable} from "../../contracts/shared/INftOwnable.sol"; + +import {Amount, AmountLib} from "../../contracts/type/Amount.sol"; +import {Fee, FeeLib} from "../../contracts/type/Fee.sol"; +import {GifTest} from "../base/GifTest.sol"; import {InstanceAdmin} from "../../contracts/instance/InstanceAdmin.sol"; import {InstanceReader} from "../../contracts/instance/InstanceReader.sol"; import {NftId} from "../../contracts/type/NftId.sol"; import {UFixed, UFixedLib} from "../../contracts/type/UFixed.sol"; -import {INftOwnable} from "../../contracts/shared/INftOwnable.sol"; contract TestInstance is GifTest { + + Amount public MAX_AMOUNT; + + function setUp() public override { super.setUp(); _prepareProduct(); // also deploys and registers distribution - } - function test_Instance_setLocked_invalidCaller() public { - // GIVEN - Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); - Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); + MAX_AMOUNT = AmountLib.max(); + } - vm.startPrank(address(this)); - // THEN - vm.expectRevert(abi.encodeWithSelector(INftOwnable.ErrorNftOwnableNotOwner.selector, address(this))); + function test_instanceSetSetUp() public { + // GIVEN setup - // WHEN - instance.setLocked(address(distribution), true); - } - - function test_Instance_setLocked_lock() public { - // GIVEN - Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); - Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); + assertEq(masterInstance.getRelease().toInt(), 3, "unexpected master instance release"); + assertEq(instance.getRelease().toInt(), 3, "unexpected instance release"); - vm.startPrank(instanceOwner); - instance.setLocked(address(distribution), true); - vm.stopPrank(); + assertEq(instance.getOwner(), instanceOwner, "unexpected instance owner"); - vm.startPrank(distributionOwner); + assertFalse(instance.isInstanceLocked(), "instance is locked"); + assertFalse(instance.isTargetLocked(address(pool)), "pool is locked"); + assertFalse(instance.isTargetLocked(address(distribution)), "distribution is locked"); - // THEN - vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, address(distributionOwner))); - - // WHEN - distribution.setFees(newMinDistributionOwnerFee, newDistributionFee); + _printAuthz(instance.getInstanceAdmin(), "instance"); } - function test_Instance_setLocked_unlock() public { - // GIVEN - Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); - Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); - vm.startPrank(instanceOwner); - instance.setLocked(address(distribution), true); - instance.setLocked(address(distribution), false); - vm.stopPrank(); + function test_instanceSetInstanceLockedInvalidCaller() public { + // GIVEN just setup - vm.startPrank(distributionOwner); + // WHEN + THEN + vm.expectRevert(abi.encodeWithSelector(INftOwnable.ErrorNftOwnableNotOwner.selector, poolOwner)); - // THEN - WHEN - make sure no revert - distribution.setFees(newMinDistributionOwnerFee, newDistributionFee); + vm.prank(poolOwner); + instance.setInstanceLocked(true); } - function test_Instance_setLocked_invalidTarget() public { - // GIVEN - Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); - Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); - vm.startPrank(instanceOwner); + function test_instanceSetInstanceLockedLock() public { + // GIVEN just setup + + // WHEN + vm.prank(instanceOwner); + instance.setInstanceLocked(true); // THEN - vm.expectRevert(abi.encodeWithSelector(IAccessAdmin.ErrorTargetUnknown.selector, address(this))); + assertTrue(instance.isInstanceLocked(), "instance is not locked"); + assertFalse(instance.isTargetLocked(address(pool)), "pool is locked"); + assertFalse(instance.isTargetLocked(address(distribution)), "distribution is locked"); - // WHEN - instance.setLocked(address(this), true); + // WHEN + THEN + vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, address(poolOwner))); + + vm.prank(poolOwner); + pool.withdrawFees(MAX_AMOUNT); } - function test_Instance_setLocked_alreadyLocked() public { - // GIVEN - Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); - Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); + function test_instanceSetInstanceLockedUnlock() public { + // GIVEN just setup + + // WHEN vm.startPrank(instanceOwner); - instance.setLocked(address(distribution), true); + instance.setInstanceLocked(true); + instance.setInstanceLocked(false); + vm.stopPrank(); // THEN - vm.expectRevert(abi.encodeWithSelector(IAccessAdmin.ErrorTargetAlreadyLocked.selector, address(distribution), true)); + assertFalse(instance.isInstanceLocked(), "instance is locked"); + assertFalse(instance.isTargetLocked(address(pool)), "pool is locked"); + assertFalse(instance.isTargetLocked(address(distribution)), "distribution is locked"); - // WHEN - instance.setLocked(address(distribution), true); + // WHEN + THEN + vm.expectRevert(abi.encodeWithSelector(IComponentService.ErrorComponentServiceWithdrawAmountIsZero.selector)); + + vm.startPrank(poolOwner); + pool.withdrawFees(MAX_AMOUNT); } - function test_Instance_setLocked_alreadyUnlocked() public { - // GIVEN - Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); - Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); - vm.startPrank(instanceOwner); - + function test_instanceSetTargetLockedWhileInstanceIsLocked() public { + // GIVEN locked instance + + vm.prank(instanceOwner); + instance.setInstanceLocked(true); + + assertTrue(instance.isInstanceLocked(), "instance is not locked (before)"); + assertFalse(instance.isTargetLocked(address(pool)), "pool is locked (before)"); + + // WHEN lock pool while instance is locked + vm.prank(instanceOwner); + instance.setTargetLocked(address(pool), true); + // THEN - vm.expectRevert(abi.encodeWithSelector(IAccessAdmin.ErrorTargetAlreadyLocked.selector, address(distribution), false)); + assertTrue(instance.isInstanceLocked(), "instance is not locked (after 1)"); + assertTrue(instance.isTargetLocked(address(pool)), "pool is not locked (after 1)"); - // WHEN - instance.setLocked(address(distribution), false); - } + // WHEN unlock pool while instance is locked + vm.prank(instanceOwner); + instance.setTargetLocked(address(pool), false); + // THEN + assertTrue(instance.isInstanceLocked(), "instance is not locked (after 2)"); + assertFalse(instance.isTargetLocked(address(pool)), "pool is locked (after 2)"); + } - function test_Instance_setLockedFromService_invalidCaller() public { + function test_instanceSetTargetLockedInvalidCaller() public { // GIVEN - Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); - Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); vm.startPrank(address(this)); // THEN - vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, address(this))); + vm.expectRevert(abi.encodeWithSelector(INftOwnable.ErrorNftOwnableNotOwner.selector, address(this))); // WHEN - instance.setLockedFromService(address(distribution), true); + instance.setTargetLocked(address(distribution), true); } - function test_Instance_setLockedFromService_lock() public { + + + function test_instanceSetTargetLockedLock() public { // GIVEN Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); - vm.startPrank(address(componentService)); - instance.setLockedFromService(address(distribution), true); + vm.startPrank(instanceOwner); + instance.setTargetLocked(address(distribution), true); vm.stopPrank(); - vm.startPrank(distributionOwner); + assertFalse(instance.isInstanceLocked(), "instance is locked"); + assertTrue(instance.isTargetLocked(address(distribution)), "distribution is not locked"); - // THEN + // WHEN + THEN vm.expectRevert(abi.encodeWithSelector(IAccessManaged.AccessManagedUnauthorized.selector, address(distributionOwner))); - // WHEN + vm.startPrank(distributionOwner); distribution.setFees(newMinDistributionOwnerFee, newDistributionFee); } - function test_Instance_setLockedFromService_unlock() public { + + function test_instanceSetTargetLockedUnlock() public { // GIVEN Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); - vm.startPrank(address(componentService)); - instance.setLockedFromService(address(distribution), true); - instance.setLockedFromService(address(distribution), false); + vm.startPrank(instanceOwner); + instance.setTargetLocked(address(distribution), true); + instance.setTargetLocked(address(distribution), false); vm.stopPrank(); - vm.startPrank(distributionOwner); + assertFalse(instance.isInstanceLocked(), "instance is locked"); + assertFalse(instance.isTargetLocked(address(distribution)), "distribution is not locked"); // THEN - WHEN - make sure no revert + vm.startPrank(distributionOwner); distribution.setFees(newMinDistributionOwnerFee, newDistributionFee); } - function test_Instance_setLockedFromService_invalidTarget() public { - // GIVEN - Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); - Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); - - vm.startPrank(address(componentService)); - - // THEN - vm.expectRevert(abi.encodeWithSelector(IAccessAdmin.ErrorTargetUnknown.selector, address(this))); - - // WHEN - instance.setLockedFromService(address(this), true); - } - - function test_Instance_setLockedFromService_alreadyLocked() public { - // GIVEN - Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); - Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); - - vm.startPrank(address(componentService)); - instance.setLockedFromService(address(distribution), true); - - // THEN - vm.expectRevert(abi.encodeWithSelector(IAccessAdmin.ErrorTargetAlreadyLocked.selector, address(distribution), true)); - - // WHEN - instance.setLockedFromService(address(distribution), true); - } - - function test_Instance_setLockedFromService_alreadyUnlocked() public { + function test_instanceSetTargetLockedInvalidTarget() public { // GIVEN - Fee memory newDistributionFee = FeeLib.toFee(UFixedLib.toUFixed(123,0), 456); - Fee memory newMinDistributionOwnerFee = FeeLib.toFee(UFixedLib.toUFixed(124,0), 457); - vm.startPrank(address(componentService)); - // THEN - vm.expectRevert(abi.encodeWithSelector(IAccessAdmin.ErrorTargetAlreadyLocked.selector, address(distribution), false)); + vm.expectRevert(abi.encodeWithSelector(IAccessAdmin.ErrorAccessAdminTargetNotCreated.selector, address(this))); // WHEN - instance.setLockedFromService(address(distribution), false); + vm.startPrank(instanceOwner); + instance.setTargetLocked(address(this), true); } - } diff --git a/test/instance/InstanceAuthorization.t.sol b/test/instance/InstanceAuthorization.t.sol new file mode 100644 index 000000000..a55878eee --- /dev/null +++ b/test/instance/InstanceAuthorization.t.sol @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: APACHE-2.0 +pragma solidity ^0.8.20; + +import {IAccessManaged} from "@openzeppelin/contracts/access/manager/IAccessManaged.sol"; +import {Test, console} from "../../lib/forge-std/src/Test.sol"; + +import {InstanceAuthorizationV3} from "../../contracts/instance/InstanceAuthorizationV3.sol"; + +import {Amount, AmountLib} from "../../contracts/type/Amount.sol"; +import {Fee, FeeLib} from "../../contracts/type/Fee.sol"; +import {GifTest} from "../base/GifTest.sol"; +import {InstanceAdmin} from "../../contracts/instance/InstanceAdmin.sol"; +import {InstanceReader} from "../../contracts/instance/InstanceReader.sol"; +import {NftId} from "../../contracts/type/NftId.sol"; +import {UFixed, UFixedLib} from "../../contracts/type/UFixed.sol"; + +contract InstanceAuthorizationV3Instance is Test { + + InstanceAuthorizationV3 public instAuthz; + + function setUp() public { + instAuthz = new InstanceAuthorizationV3(); + } + + function test_instanceAuthorizationSetup() public { + assertTrue(address(instAuthz) != address(0), "instAuthz not set"); + + // solhint-disable + console.log("main target name:", instAuthz.getMainTargetName()); + console.log("main target name (via target):", instAuthz.getMainTarget().toString()); + console.log("main target role:", instAuthz.getTargetRole(instAuthz.getMainTarget()).toInt()); + // solhint-enable + + assertEq(instAuthz.getMainTargetName(), "Instance", "unexpected main target name"); + assertEq(instAuthz.getMainTarget().toString(), "Instance", "unexpected main target"); + assertEq(instAuthz.getTargetRole(instAuthz.getMainTarget()).toInt(), 10, "unexpected main target role"); + } +} diff --git a/test/instance/InstanceReader.t.sol b/test/instance/InstanceReader.t.sol index f465ab5e5..91658007e 100644 --- a/test/instance/InstanceReader.t.sol +++ b/test/instance/InstanceReader.t.sol @@ -1,29 +1,107 @@ // SPDX-License-Identifier: APACHE-2.0 pragma solidity ^0.8.20; -import {IAccessManaged} from "@openzeppelin/contracts/access/manager/IAccessManaged.sol"; +import {IInstanceService} from "../../contracts/instance/IInstanceService.sol"; +import {INftOwnable} from "../../contracts/shared/INftOwnable.sol"; -import {Fee, FeeLib} from "../../contracts/type/Fee.sol"; import {GifTest} from "../base/GifTest.sol"; -import {IAccessAdmin} from "../../contracts/authorization/IAccessAdmin.sol"; -import {IInstance} from "../../contracts/instance/IInstance.sol"; -import {IInstanceService} from "../../contracts/instance/IInstanceService.sol"; -import {InstanceAdmin} from "../../contracts/instance/InstanceAdmin.sol"; import {InstanceReader} from "../../contracts/instance/InstanceReader.sol"; -import {NftId} from "../../contracts/type/NftId.sol"; -import {UFixed, UFixedLib} from "../../contracts/type/UFixed.sol"; -contract TestInstance is GifTest { +contract InstanceReaderTest is GifTest { - function setUp() public override { - super.setUp(); - } - - function test_instanceReader() public { + function test_instanceReaderSetUp() public { // GIVEN just set up // THEN assertEq(address(instanceReader.getRegistry()), address(registry), "unexpected registry address"); assertEq(address(instanceReader.getInstance()), address(instance), "unexpected instance address"); assertEq(instanceReader.getInstanceNftId().toInt(), instanceNftId.toInt(), "unexpected instance nft id"); } + + + function test_instanceReaderUpgradeMasterInstanceReader() public { + // GIVEN + vm.startPrank(registryOwner); + InstanceReader newMasterInstanceReader = _createNewMasterInstanceReader(); + + // WHEN + instanceService.upgradeMasterInstanceReader(address(newMasterInstanceReader)); + + // THEN + assertEq( + address(newMasterInstanceReader), + instanceService.getMasterInstanceReader(), "master instance reader not set"); + } + + function test_instanceReaderUpgradeMasterInstanceReaderNotMasterInstance() public { + // GIVEN + InstanceReader newMasterInstanceReader = new InstanceReader(); + + // THEN + vm.expectRevert( + abi.encodeWithSelector( + IInstanceService.ErrorInstanceServiceInstanceReaderInstanceMismatch.selector)); + + // WHEN + vm.startPrank(registryOwner); + instanceService.upgradeMasterInstanceReader(address(newMasterInstanceReader)); + } + + function test_instanceReaderUpgradeMasterInstanceReader_same_reader() public { + // GIVEN + vm.startPrank(registryOwner); + + // THEN + vm.expectRevert( + abi.encodeWithSelector( + IInstanceService.ErrorInstanceServiceInstanceReaderSameAsMasterInstanceReader.selector)); + + // WHEN + instanceService.upgradeMasterInstanceReader(address(masterInstanceReader)); + } + + function test_instanceReaderUpgradeInstanceReaderAuthorized() public { + // GIVEN + vm.startPrank(registryOwner); + InstanceReader newMasterInstanceReader = _createNewMasterInstanceReader(); + instanceService.upgradeMasterInstanceReader(address(newMasterInstanceReader)); + vm.stopPrank(); + + address oldInstanceReaderAddress = address(instance.getInstanceReader()); + + assertEq(registry.ownerOf(instanceNftId), instanceOwner, "instanceOwner not owner of instance nft id"); + + // WHEN + vm.startPrank(instanceOwner); + instance.upgradeInstanceReader(); + vm.stopPrank(); + + // THEN + assertFalse(oldInstanceReaderAddress == address(instance.getInstanceReader()), "instance reader not upgraded"); + } + + function test_instanceReaderUpgradeInstanceReaderNotAuthorized() public { + // GIVEN + vm.startPrank(registryOwner); + InstanceReader newMasterInstanceReader = _createNewMasterInstanceReader(); + instanceService.upgradeMasterInstanceReader(address(newMasterInstanceReader)); + vm.stopPrank(); + + // THEN + vm.expectRevert( + abi.encodeWithSelector( + INftOwnable.ErrorNftOwnableNotOwner.selector, + poolOwner)); + + // WHEN + vm.prank(poolOwner); + instance.upgradeInstanceReader(); + } + + + function _createNewMasterInstanceReader() internal returns (InstanceReader) { + InstanceReader newMIR = new InstanceReader(); + newMIR.initializeWithInstance(address(masterInstance)); + return newMIR; + } + } diff --git a/test/instance/MasterInstance.t.sol b/test/instance/MasterInstance.t.sol new file mode 100644 index 000000000..d57b1226a --- /dev/null +++ b/test/instance/MasterInstance.t.sol @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: APACHE-2.0 +pragma solidity ^0.8.20; + +import {IAccessManaged} from "@openzeppelin/contracts/access/manager/IAccessManaged.sol"; +import {console} from "../../lib/forge-std/src/Test.sol"; + +import {IAccessAdmin} from "../../contracts/authorization/IAccessAdmin.sol"; +import {IComponentService} from "../../contracts/shared/IComponentService.sol"; +import {IInstance} from "../../contracts/instance/IInstance.sol"; +import {IInstanceService} from "../../contracts/instance/IInstanceService.sol"; +import {INftOwnable} from "../../contracts/shared/INftOwnable.sol"; + +import {Amount, AmountLib} from "../../contracts/type/Amount.sol"; +import {Fee, FeeLib} from "../../contracts/type/Fee.sol"; +import {GifTest} from "../base/GifTest.sol"; +import {InstanceAdmin} from "../../contracts/instance/InstanceAdmin.sol"; +import {InstanceReader} from "../../contracts/instance/InstanceReader.sol"; +import {NftId} from "../../contracts/type/NftId.sol"; +import {UFixed, UFixedLib} from "../../contracts/type/UFixed.sol"; + +contract MasterTestInstance is GifTest { + + + function test_masterInstanceSetup() public { + // GIVEN setup + + assertTrue(address(masterInstance) != address(0), "masterInstance not set"); + } +} diff --git a/test/instance/Test.t.sol b/test/instance/Test.t.sol deleted file mode 100644 index f52dc149a..000000000 --- a/test/instance/Test.t.sol +++ /dev/null @@ -1,100 +0,0 @@ -// SPDX-License-Identifier: APACHE-2.0 -pragma solidity ^0.8.20; - -import {GifTest} from "../base/GifTest.sol"; -import {IInstance} from "../../contracts/instance/IInstance.sol"; -import {IInstanceService} from "../../contracts/instance/IInstanceService.sol"; -import {InstanceAdmin} from "../../contracts/instance/InstanceAdmin.sol"; -import {InstanceReader} from "../../contracts/instance/InstanceReader.sol"; -import {NftId} from "../../contracts/type/NftId.sol"; - -contract TestInstanceService is GifTest { - - uint256 public constant INITIAL_BALANCE = 100000; - - function test_upgradeMasterInstanceReader() public { - // GIVEN - vm.startPrank(registryOwner); - InstanceReader newMasterInstanceReader = _createNewMasterInstanceReader(); - - // WHEN - instanceService.upgradeMasterInstanceReader(address(newMasterInstanceReader)); - - // THEN - assertEq( - address(newMasterInstanceReader), - instanceService.getMasterInstanceReader(), "master instance reader not set"); - } - - function test_upgradeMasterInstanceReaderNotMasterInstance() public { - // GIVEN - InstanceReader newMasterInstanceReader = new InstanceReader(); - - // THEN - vm.expectRevert( - abi.encodeWithSelector( - IInstanceService.ErrorInstanceServiceInstanceReaderInstanceMismatch.selector)); - - // WHEN - vm.startPrank(registryOwner); - instanceService.upgradeMasterInstanceReader(address(newMasterInstanceReader)); - } - - function test_upgradeMasterInstanceReader_same_reader() public { - // GIVEN - vm.startPrank(registryOwner); - - // THEN - vm.expectRevert( - abi.encodeWithSelector( - IInstanceService.ErrorInstanceServiceInstanceReaderSameAsMasterInstanceReader.selector)); - - // WHEN - instanceService.upgradeMasterInstanceReader(address(masterInstanceReader)); - } - - function test_upgradeInstanceReaderAuthorized() public { - // GIVEN - vm.startPrank(registryOwner); - InstanceReader newMasterInstanceReader = _createNewMasterInstanceReader(); - instanceService.upgradeMasterInstanceReader(address(newMasterInstanceReader)); - vm.stopPrank(); - - address oldInstanceReaderAddress = address(instance.getInstanceReader()); - - assertEq(registry.ownerOf(instanceNftId), instanceOwner, "instanceOwner not owner of instance nft id"); - - // WHEN - vm.startPrank(instanceOwner); - instanceService.upgradeInstanceReader(instanceNftId); - vm.stopPrank(); - - // THEN - assertFalse(oldInstanceReaderAddress == address(instance.getInstanceReader()), "instance reader not upgraded"); - } - - function test_upgradeInstanceReaderNotAuthorized() public { - // GIVEN - vm.startPrank(registryOwner); - InstanceReader newMasterInstanceReader = _createNewMasterInstanceReader(); - instanceService.upgradeMasterInstanceReader(address(newMasterInstanceReader)); - vm.stopPrank(); - - // THEN - vm.expectRevert( - abi.encodeWithSelector( - IInstanceService.ErrorInstanceServiceRequestUnauhorized.selector, - address(this))); - - // WHEN - instanceService.upgradeInstanceReader(instanceNftId); - } - - - function _createNewMasterInstanceReader() internal returns (InstanceReader) { - InstanceReader newMIR = new InstanceReader(); - newMIR.initializeWithInstance(address(masterInstance)); - return newMIR; - } - -} diff --git a/test/instance/TestInstanceService.t.sol b/test/instance/TestInstanceService.t.sol deleted file mode 100644 index f52dc149a..000000000 --- a/test/instance/TestInstanceService.t.sol +++ /dev/null @@ -1,100 +0,0 @@ -// SPDX-License-Identifier: APACHE-2.0 -pragma solidity ^0.8.20; - -import {GifTest} from "../base/GifTest.sol"; -import {IInstance} from "../../contracts/instance/IInstance.sol"; -import {IInstanceService} from "../../contracts/instance/IInstanceService.sol"; -import {InstanceAdmin} from "../../contracts/instance/InstanceAdmin.sol"; -import {InstanceReader} from "../../contracts/instance/InstanceReader.sol"; -import {NftId} from "../../contracts/type/NftId.sol"; - -contract TestInstanceService is GifTest { - - uint256 public constant INITIAL_BALANCE = 100000; - - function test_upgradeMasterInstanceReader() public { - // GIVEN - vm.startPrank(registryOwner); - InstanceReader newMasterInstanceReader = _createNewMasterInstanceReader(); - - // WHEN - instanceService.upgradeMasterInstanceReader(address(newMasterInstanceReader)); - - // THEN - assertEq( - address(newMasterInstanceReader), - instanceService.getMasterInstanceReader(), "master instance reader not set"); - } - - function test_upgradeMasterInstanceReaderNotMasterInstance() public { - // GIVEN - InstanceReader newMasterInstanceReader = new InstanceReader(); - - // THEN - vm.expectRevert( - abi.encodeWithSelector( - IInstanceService.ErrorInstanceServiceInstanceReaderInstanceMismatch.selector)); - - // WHEN - vm.startPrank(registryOwner); - instanceService.upgradeMasterInstanceReader(address(newMasterInstanceReader)); - } - - function test_upgradeMasterInstanceReader_same_reader() public { - // GIVEN - vm.startPrank(registryOwner); - - // THEN - vm.expectRevert( - abi.encodeWithSelector( - IInstanceService.ErrorInstanceServiceInstanceReaderSameAsMasterInstanceReader.selector)); - - // WHEN - instanceService.upgradeMasterInstanceReader(address(masterInstanceReader)); - } - - function test_upgradeInstanceReaderAuthorized() public { - // GIVEN - vm.startPrank(registryOwner); - InstanceReader newMasterInstanceReader = _createNewMasterInstanceReader(); - instanceService.upgradeMasterInstanceReader(address(newMasterInstanceReader)); - vm.stopPrank(); - - address oldInstanceReaderAddress = address(instance.getInstanceReader()); - - assertEq(registry.ownerOf(instanceNftId), instanceOwner, "instanceOwner not owner of instance nft id"); - - // WHEN - vm.startPrank(instanceOwner); - instanceService.upgradeInstanceReader(instanceNftId); - vm.stopPrank(); - - // THEN - assertFalse(oldInstanceReaderAddress == address(instance.getInstanceReader()), "instance reader not upgraded"); - } - - function test_upgradeInstanceReaderNotAuthorized() public { - // GIVEN - vm.startPrank(registryOwner); - InstanceReader newMasterInstanceReader = _createNewMasterInstanceReader(); - instanceService.upgradeMasterInstanceReader(address(newMasterInstanceReader)); - vm.stopPrank(); - - // THEN - vm.expectRevert( - abi.encodeWithSelector( - IInstanceService.ErrorInstanceServiceRequestUnauhorized.selector, - address(this))); - - // WHEN - instanceService.upgradeInstanceReader(instanceNftId); - } - - - function _createNewMasterInstanceReader() internal returns (InstanceReader) { - InstanceReader newMIR = new InstanceReader(); - newMIR.initializeWithInstance(address(masterInstance)); - return newMIR; - } - -} diff --git a/test/staking/Staking.t.sol b/test/staking/Staking.t.sol index 70350d0ba..82ba93318 100644 --- a/test/staking/Staking.t.sol +++ b/test/staking/Staking.t.sol @@ -36,6 +36,8 @@ contract StakingTest is GifTest { } function test_stakingSetUp() public { + _printAuthz(registryAdmin, "registry setup"); + assertEq(staking.getWallet(), address(staking.getTokenHandler()), "unexpected staking wallet"); assertEq(dip.allowance(staking.getWallet(), address(staking.getTokenHandler())), type(uint256).max, "unexpected allowance for staking token handler"); } From dfd1c744c3628b769b8180782bf08bfba959d004 Mon Sep 17 00:00:00 2001 From: Matthias Zimmermann Date: Mon, 26 Aug 2024 00:01:22 +0000 Subject: [PATCH 5/7] various cleanups --- contracts/instance/InstanceAdmin.sol | 37 ----------------------- contracts/instance/InstanceService.sol | 17 ----------- contracts/product/PolicyService.sol | 41 -------------------------- contracts/registry/RegistryAdmin.sol | 10 ------- test/base/GifDeployer.sol | 7 ----- test/base/GifTest.sol | 18 ++--------- 6 files changed, 3 insertions(+), 127 deletions(-) diff --git a/contracts/instance/InstanceAdmin.sol b/contracts/instance/InstanceAdmin.sol index 41c4e4681..7c18a7041 100644 --- a/contracts/instance/InstanceAdmin.sol +++ b/contracts/instance/InstanceAdmin.sol @@ -56,41 +56,13 @@ contract InstanceAdmin is _; } -// TODO cleanup logs -event LogDebug3(string key, string value); - /// @dev Only used for master instance admin. constructor(address accessManager) { -emit LogDebug3("2a", ""); initialize( accessManager, "MasterInstanceAdmin"); -emit LogDebug3("2b", ""); } - // TODO cleanup - // function initialize( - // AccessManagerCloneable clonedAccessManager, - // IRegistry registry, - // VersionPart release - // ) - // public - // initializer() - // { - // // checks - - // // effects - // __AccessAdmin_init( - // address(clonedAccessManager), - // "InstanceAdmin"); - - // clonedAccessManager.completeSetup( - // address(registry), - // release); - - // _registry = registry; - // } - /// @dev Completes the initialization of this instance admin using the provided instance, registry and version. /// Important: Initialization of instance admin is only complete after calling this function. @@ -127,21 +99,15 @@ emit LogDebug3("2b", ""); // link nft ownability to instance _linkToNftOwnable(instance); -emit LogDebug3("3d", ""); // create instance role and target _setupInstance(instance); -emit LogDebug3("3e", ""); // add instance authorization _createRoles(_authorization); -emit LogDebug3("3f", ""); // _createTargets(_authorization); -emit LogDebug3("3g", ""); _setupInstanceHelperTargetsWithRoles(); -emit LogDebug3("3h", ""); _createTargetAuthorizations(_authorization); -emit LogDebug3("3i", ""); } @@ -189,13 +155,10 @@ emit LogDebug3("3i", ""); return _release; } - event LogDebug(string key, string value); // create instance role and target function _setupInstance(address instance) internal { - emit LogDebug("main target", _authorization.getMainTarget().toString()); - // create instance role RoleId instanceRoleId = _authorization.getTargetRole( _authorization.getMainTarget()); diff --git a/contracts/instance/InstanceService.sol b/contracts/instance/InstanceService.sol index e48bc531c..abf8fa99e 100644 --- a/contracts/instance/InstanceService.sol +++ b/contracts/instance/InstanceService.sol @@ -95,8 +95,6 @@ contract InstanceService is IInstance(instanceAddress).getInstanceAdmin().setTargetLocked(target, locked); } -// TODO cleanup logs -event LogDebug4(string key, string value); function createInstance() external @@ -114,36 +112,28 @@ event LogDebug4(string key, string value); InstanceAdmin instanceAdmin = _cloneNewInstanceAdmin(); instance = _createInstance(instanceAdmin, instanceOwner); -emit LogDebug4("3a", "0"); // register cloned instance with registry instanceNftId = _registryService.registerInstance( instance, instanceOwner).nftId; // MUST be set after instance is set up and registered -emit LogDebug4("3a", "1"); IAuthorization instanceAuthorization = InstanceAdmin(_masterInstanceAdmin).getInstanceAuthorization(); -emit LogDebug4("3a", "2"); instanceAdmin.completeSetup( address(getRegistry()), address(instance), address(instanceAuthorization), getRelease()); -emit LogDebug4("3a", "3"); // hard checks for newly cloned instance assert(address(instance.getRegistry()) == address(getRegistry())); assert(instance.getRelease() == getRelease()); -emit LogDebug4("3a", "4"); - // register cloned instance as staking target _stakingService.createInstanceTarget( instanceNftId, TargetManagerLib.getDefaultLockingPeriod(), TargetManagerLib.getDefaultRewardRate()); -emit LogDebug4("3a", "5"); - emit LogInstanceCloned( instanceNftId, address(instance)); @@ -339,13 +329,6 @@ emit LogDebug4("3a", "5"); getRelease(), instanceOwner); - // TODO cleanup - // instanceAdmin.completeSetup( - // address(getRegistry()), - // address(clonedInstance), - // address(instanceAdmin.getInstanceAuthorization()), - // getRelease()); - return clonedInstance; } diff --git a/contracts/product/PolicyService.sol b/contracts/product/PolicyService.sol index 0930b87d0..af5254cdb 100644 --- a/contracts/product/PolicyService.sol +++ b/contracts/product/PolicyService.sol @@ -545,31 +545,6 @@ contract PolicyService is } } - // TODO cleanup - /// @dev checks the balance and allowance of the policy holder - // function _checkPremiumBalanceAndAllowance( - // IERC20Metadata token, - // address tokenHandlerAddress, - // address policyHolder, - // Amount premiumAmount - // ) - // internal - // virtual - // view - // { - // uint256 premium = premiumAmount.toInt(); - // uint256 balance = token.balanceOf(policyHolder); - // uint256 allowance = token.allowance(policyHolder, tokenHandlerAddress); - - // if (balance < premium) { - // revert ErrorPolicyServiceBalanceInsufficient(policyHolder, premium, balance); - // } - - // if (allowance < premium) { - // revert ErrorPolicyServiceAllowanceInsufficient(policyHolder, tokenHandlerAddress, premium, allowance); - // } - // } - function _policyHolderPolicyActivated( NftId policyNftId, @@ -629,22 +604,6 @@ contract PolicyService is } - // TODO cleanup - // function _getTokenHandler( - // InstanceReader instanceReader, - // NftId productNftId - // ) - // internal - // virtual - // view - // returns ( - // TokenHandler tokenHandler - // ) - // { - // tokenHandler = instanceReader.getTokenHandler(productNftId).tokenHandler; - // } - - function _getDistributionNftAndWallets( InstanceReader instanceReader, NftId productNftId diff --git a/contracts/registry/RegistryAdmin.sol b/contracts/registry/RegistryAdmin.sol index 6928d0b72..278cb8016 100644 --- a/contracts/registry/RegistryAdmin.sol +++ b/contracts/registry/RegistryAdmin.sol @@ -87,7 +87,6 @@ contract RegistryAdmin is "RegistryAdmin"); } -event LogDebug2(string message, uint256 value); function completeSetup( address registry, @@ -100,9 +99,6 @@ event LogDebug2(string message, uint256 value); reinitializer(type(uint8).max) onlyDeployer() { - // TODO cleanup logs - emit LogDebug2("a", 0); - // checks _checkRegistry(registry); @@ -128,20 +124,14 @@ event LogDebug2(string message, uint256 value); _linkToNftOwnable(_registry); _setupRegistry(_registry); -emit LogDebug2("b", 0); // setup authorization for registry and supporting contracts _createRoles(_authorization); -emit LogDebug2("c", 0); _grantRoleToAccount(GIF_ADMIN_ROLE(), gifAdmin); -emit LogDebug2("d", 0); _grantRoleToAccount(GIF_MANAGER_ROLE(), gifManager); -emit LogDebug2("e", 0); _createTargets(_authorization); -emit LogDebug2("f", 0); _createTargetAuthorizations(_authorization); -emit LogDebug2("g", 0); } diff --git a/test/base/GifDeployer.sol b/test/base/GifDeployer.sol index 50811e4a5..724f75c8b 100644 --- a/test/base/GifDeployer.sol +++ b/test/base/GifDeployer.sol @@ -131,9 +131,6 @@ contract GifDeployer is Test { mapping(ObjectType domain => DeployedServiceInfo info) public serviceForDomain; - // TODO cleanup - event LogDebug(string message, string value); - function deployCore( address globalRegistry, address gifAdmin, @@ -188,17 +185,14 @@ contract GifDeployer is Test { stakingReader.initialize( address(staking), address(stakingStore)); -emit LogDebug("F", ""); // 10) intialize registry and register staking component registry.initialize( address(releaseRegistry), address(tokenRegistry), address(staking)); -emit LogDebug("G", ""); staking.linkToRegisteredNftId(); -emit LogDebug("H", ""); // 11) initialize registry admin registryAdmin.completeSetup( @@ -206,7 +200,6 @@ emit LogDebug("H", ""); address(new RegistryAuthorization()), gifAdmin, gifManager); -emit LogDebug("I", ""); vm.stopPrank(); } diff --git a/test/base/GifTest.sol b/test/base/GifTest.sol index dd0738f75..f1e803ca7 100644 --- a/test/base/GifTest.sol +++ b/test/base/GifTest.sol @@ -22,10 +22,7 @@ import {VersionPartLib} from "../../contracts/type/Version.sol"; import {SimpleDistributionAuthorization} from "../../contracts/examples/unpermissioned/SimpleDistributionAuthorization.sol"; import {BasicOracleAuthorization} from "../../contracts/oracle/BasicOracleAuthorization.sol"; import {BasicProductAuthorization} from "../../contracts/product/BasicProductAuthorization.sol"; -// TODO cleanup -// import {BasicPoolAuthorization} from "../../contracts/pool/BasicPoolAuthorization.sol"; import {SimplePoolAuthorization} from "../../contracts/examples/unpermissioned/SimplePoolAuthorization.sol"; -// import {SimpleProductAuthorization} from "../../contracts/examples/unpermissioned/SimpleProductAuthorization.sol"; import {RegistryAdmin} from "../../contracts/registry/RegistryAdmin.sol"; import {ReleaseRegistry} from "../../contracts/registry/ReleaseRegistry.sol"; import {ServiceAuthorizationV3} from "../../contracts/registry/ServiceAuthorizationV3.sol"; @@ -304,8 +301,6 @@ contract GifTest is GifDeployer { assertEq(releaseRegistry.getState(releaseRegistry.getLatestVersion()).toInt(), ACTIVE().toInt(), "unexpected state for releaseRegistry after activateNextRelease"); } -// TODO cleanup logs -event LogDebugMaster(string key, string value); function _deployMasterInstance() internal { @@ -319,9 +314,7 @@ event LogDebugMaster(string key, string value); masterInstanceReader = new InstanceReader(); // crate instance -emit LogDebugMaster("1", "a"); masterInstance = new Instance(); -emit LogDebugMaster("1", "b"); masterInstance.initialize( masterInstanceAdmin, masterInstanceStore, @@ -331,28 +324,23 @@ emit LogDebugMaster("1", "b"); registry, VersionPartLib.toVersionPart(3), registryOwner); -emit LogDebugMaster("1", "c"); + // sets master instance address in instance service // instance service is now ready to create cloned instances masterInstanceNftId = instanceService.setAndRegisterMasterInstance(address(masterInstance)); -emit LogDebugMaster("1", "d"); + // setup roles, targets and function grantings instanceAuthorizationV3 = new InstanceAuthorizationV3(); -emit LogDebugMaster("1", "e"); masterInstanceAdmin.completeSetup( address(registry), address(masterInstance), address(instanceAuthorizationV3), VersionPartLib.toVersionPart(3)); -emit LogDebugMaster("1", "f"); require(address(masterInstanceAdmin.getRegistry()) == address(registry), "unexpected master instance registry"); - // require(masterInstanceAdmin.getRelease().toInt() == 3, "unexpected master instance release"); + require(masterInstanceAdmin.getRelease().toInt() == 3, "unexpected master instance release"); // MUST be set after instance is set up and registered - // TODO consider deleting this -> master instance just provides impl code, it own state must pe mot initialized - //masterInstanceAdmin.initializeInstanceAuthorization(address(masterInstance)); - // lock master instance nft chainNft.transferFrom(registryOwner, NFT_LOCK_ADDRESS, masterInstanceNftId.toInt()); From 0050c3bea614623b2b2b53e06413afae2a0d5fe5 Mon Sep 17 00:00:00 2001 From: Matthias Zimmermann Date: Mon, 26 Aug 2024 00:23:21 +0000 Subject: [PATCH 6/7] fix merge conflict, fix issue #401 --- contracts/instance/InstanceAdmin.sol | 41 ++++++++++++++++------------ test/base/GifTest.sol | 1 + 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/contracts/instance/InstanceAdmin.sol b/contracts/instance/InstanceAdmin.sol index d9d1bbb23..8d4853c18 100644 --- a/contracts/instance/InstanceAdmin.sol +++ b/contracts/instance/InstanceAdmin.sol @@ -10,7 +10,7 @@ import {IInstance} from "./IInstance.sol"; import {AccessAdmin} from "../authorization/AccessAdmin.sol"; import {AccessManagerCloneable} from "../authorization/AccessManagerCloneable.sol"; import {NftId} from "../type/NftId.sol"; -import {ObjectType, INSTANCE} from "../type/ObjectType.sol"; +import {ObjectType, INSTANCE, ORACLE} from "../type/ObjectType.sol"; import {RoleId, RoleIdLib} from "../type/RoleId.sol"; import {Str, StrLib} from "../type/String.sol"; import {TokenHandler} from "../shared/TokenHandler.sol"; @@ -138,8 +138,8 @@ contract InstanceAdmin is _checkAuthorization(address(authorization), expectedType, getRelease(), false); // effects - // setup target and role for component (including token handler) - _setupComponentAndTokenHandler(component); + // setup target and role for component (including token handler if applicable) + _setupComponentAndTokenHandler(component, expectedType); // create other roles and function authorizations _createRoles(authorization); @@ -246,7 +246,10 @@ contract InstanceAdmin is // ------------------- Internal functions ------------------- // - function _setupComponentAndTokenHandler(IInstanceLinkedComponent component) + function _setupComponentAndTokenHandler( + IInstanceLinkedComponent component, + ObjectType componentType + ) internal { @@ -263,25 +266,27 @@ contract InstanceAdmin is true, // checkAuthority false); // custom - // create component's token handler target - NftId componentNftId = _registry.getNftIdForAddress(address(component)); - address tokenHandler = address( - _instance.getInstanceReader().getComponentInfo( - componentNftId).tokenHandler); - - _createTarget( - tokenHandler, - authorization.getTokenHandlerName(), - true, - false); + // create component's token handler target if app + if (componentType != ORACLE()) { + NftId componentNftId = _registry.getNftIdForAddress(address(component)); + address tokenHandler = address( + _instance.getInstanceReader().getComponentInfo( + componentNftId).tokenHandler); + + _createTarget( + tokenHandler, + authorization.getTokenHandlerName(), + true, + false); + + // token handler does not require its own role + // token handler is not calling other components + } // assign component role to component _grantRoleToAccount( componentRoleId, address(component)); - - // token handler does not require its own role - // token handler is not calling other components } diff --git a/test/base/GifTest.sol b/test/base/GifTest.sol index 7887733a3..c19637e89 100644 --- a/test/base/GifTest.sol +++ b/test/base/GifTest.sol @@ -22,6 +22,7 @@ import {VersionPartLib} from "../../contracts/type/Version.sol"; import {BasicDistributionAuthorization} from "../../contracts/distribution/BasicDistributionAuthorization.sol"; import {BasicOracleAuthorization} from "../../contracts/oracle/BasicOracleAuthorization.sol"; import {BasicProductAuthorization} from "../../contracts/product/BasicProductAuthorization.sol"; +import {SimpleDistributionAuthorization} from "../../contracts/examples/unpermissioned/SimpleDistributionAuthorization.sol"; import {SimplePoolAuthorization} from "../../contracts/examples/unpermissioned/SimplePoolAuthorization.sol"; import {RegistryAdmin} from "../../contracts/registry/RegistryAdmin.sol"; import {ReleaseRegistry} from "../../contracts/registry/ReleaseRegistry.sol"; From 4a3fdec06ff01b08563a8d179d0d47a750c640ce Mon Sep 17 00:00:00 2001 From: Matthias Zimmermann Date: Mon, 26 Aug 2024 16:52:07 +0000 Subject: [PATCH 7/7] reduce size of InstanceAdmin (introduce InstanceAdminLib) --- contracts/authorization/AccessAdmin.sol | 355 ++++++------------ contracts/authorization/AccessAdminLib.sol | 183 +++++++++ .../authorization/AccessManagerCloneable.sol | 3 +- contracts/authorization/Authorization.sol | 4 +- contracts/authorization/IAccessAdmin.sol | 15 +- contracts/instance/InstanceAdmin.sol | 3 +- contracts/registry/RegistryAdmin.sol | 267 ++----------- contracts/registry/ReleaseAdmin.sol | 11 +- contracts/registry/ReleaseRegistry.sol | 56 +-- contracts/shared/ContractLib.sol | 18 + scripts/libs/instance.ts | 49 ++- scripts/libs/libraries.ts | 26 +- scripts/libs/registry.ts | 40 +- scripts/libs/services.ts | 1 + test/GifDeployer.t.sol | 23 +- test/authorization/AccessAdmin.t.sol | 57 ++- .../authorization/AccessAdminManageMock.t.sol | 69 ++-- test/authorization/RegistryAdminEx.sol | 3 +- test/base/GifDeployer.sol | 1 - test/release/ReleaseRegistryConcrete.t.sol | 17 +- 20 files changed, 568 insertions(+), 633 deletions(-) create mode 100644 contracts/authorization/AccessAdminLib.sol diff --git a/contracts/authorization/AccessAdmin.sol b/contracts/authorization/AccessAdmin.sol index 116951408..e96e2ad57 100644 --- a/contracts/authorization/AccessAdmin.sol +++ b/contracts/authorization/AccessAdmin.sol @@ -9,6 +9,7 @@ import {IAccessAdmin} from "./IAccessAdmin.sol"; import {IAuthorization} from "./IAuthorization.sol"; import {IRegistry} from "../registry/IRegistry.sol"; +import {AccessAdminLib} from "./AccessAdminLib.sol"; import {AccessManagerCloneable} from "./AccessManagerCloneable.sol"; import {ContractLib} from "../shared/ContractLib.sol"; import {NftId, NftIdLib} from "../type/NftId.sol"; @@ -98,18 +99,20 @@ contract AccessAdmin is _; } - modifier onlyRoleAdmin(RoleId roleId) { + modifier onlyRoleMember(RoleId roleId, address account) { _checkRoleExists(roleId, false); - if (!hasAdminRole(msg.sender, roleId)) { - revert ErrorAccessAdminNotAdminOfRole(_roleInfo[roleId].adminRoleId); + if (!hasRole(account, roleId)) { + revert ErrorAccessAdminNotRoleOwner(roleId, account); } _; } - modifier onlyRoleMember(RoleId roleId) { - if (!hasRole(msg.sender, roleId)) { - revert ErrorAccessAdminNotRoleOwner(roleId); + modifier onlyRoleAdmin(RoleId roleId, address account) { + _checkRoleExists(roleId, false); + + if (!hasAdminRole(account, roleId)) { + revert ErrorAccessAdminNotAdminOfRole(_roleInfo[roleId].adminRoleId, account); } _; } @@ -127,7 +130,7 @@ contract AccessAdmin is } modifier onlyExistingTarget(address target) { - _checkTarget(target); + _checkTargetExists(target); _; } @@ -191,92 +194,6 @@ contract AccessAdmin is } - /// @dev check if target exists and reverts if it doesn't - function _checkTarget( - address target - ) - internal - view - { - // check not yet created - if (!targetExists(target)) { - revert ErrorAccessAdminTargetNotCreated(target); - } - } - - function _checkIsRegistered( - address registry, - address target, - ObjectType expectedType - ) - internal - view - { - ObjectType tagetType = IRegistry(registry).getObjectInfo(target).objectType; - if (tagetType.eqz()) { - revert ErrorAccessAdminNotRegistered(target); - } - - if (tagetType != expectedType) { - revert ErrorAccessAdminTargetTypeMismatch(target, expectedType, tagetType); - } - } - - /// @dev check if target exists and and has expected type, reverts if it doesn't - function _checkTargetWithType( - address target, - ObjectType expectedType - ) - internal - view - { - // check target exists - _checkTarget(target); - - // check target type - ObjectType tagetType = _authority.getRegistry().getObjectInfo(target).objectType; - if (tagetType != expectedType) { - revert ErrorAccessAdminTargetTypeMismatch(target, expectedType, tagetType); - } - - // target release is checked during component/instance registration - } - - - function _checkAuthorization( - address authorization, - ObjectType expectedDomain, - VersionPart expectedRelease, - bool checkAlreadyInitialized - ) - internal - view - { - // checks - // check not yet initialized - if (checkAlreadyInitialized && address(_authorization) != address(0)) { - revert ErrorAccessAdminAlreadyInitialized(address(_authorization)); - } - - // check contract type matches - if (!ContractLib.supportsInterface(authorization, type(IAuthorization).interfaceId)) { - revert ErrorAccessAdminNotAuthorization(authorization); - } - - // check domain matches - ObjectType domain = IAuthorization(authorization).getDomain(); - if (domain != expectedDomain) { - revert ErrorAccessAdminDomainMismatch(authorization, expectedDomain, domain); - } - - // check release matches - VersionPart authorizationRelease = IAuthorization(authorization).getRelease(); - if (authorizationRelease != expectedRelease) { - revert ErrorAccessAdminReleaseMismatch(authorization, getRelease(), authorizationRelease); - } - } - - function getRelease() public view virtual returns (VersionPart release) { return _authority.getRelease(); } @@ -297,46 +214,13 @@ contract AccessAdmin is } - - function isLocked() public view returns (bool locked) { - return _authority.isLocked(); - } - - function _linkToNftOwnable(address registerable) internal { - if (!getRegistry().isRegistered(registerable)) { - revert ErrorAccessAdminNotRegistered(registerable); - } - - _linkedNftId = getRegistry().getNftIdForAddress(registerable); + function getAuthorization() public view returns (IAuthorization authorization) { + return _authorization; } - function _initializeAdminAndPublicRoles() - internal - virtual - onlyInitializing() - { - RoleId adminRoleId = RoleIdLib.toRoleId(_authority.ADMIN_ROLE()); - - // setup admin role - _createRoleUnchecked( - ADMIN_ROLE(), - toRole({ - adminRoleId: ADMIN_ROLE(), - roleType: RoleType.Contract, - maxMemberCount: 1, - name: ADMIN_ROLE_NAME})); - // add this contract as admin role member - _roleMembers[adminRoleId].add(address(this)); - - // setup public role - _createRoleUnchecked( - PUBLIC_ROLE(), - toRole({ - adminRoleId: ADMIN_ROLE(), - roleType: RoleType.Gif, - maxMemberCount: type(uint32).max, - name: PUBLIC_ROLE_NAME})); + function isLocked() public view returns (bool locked) { + return _authority.isLocked(); } //--- view functions for roles ------------------------------------------// @@ -361,12 +245,12 @@ contract AccessAdmin is return _roleInfo[roleId].createdAt.gtz(); } - function getRoleInfo(RoleId roleId) external view returns (RoleInfo memory) { - return _roleInfo[roleId]; + function getRoleForName(string memory name) external view returns (RoleId roleId) { + return _roleForName[StrLib.toStr(name)].roleId; } - function getRoleForName(Str name) external view returns (RoleNameInfo memory) { - return _roleForName[name]; + function getRoleInfo(RoleId roleId) external view returns (RoleInfo memory) { + return _roleInfo[roleId]; } function roleMembers(RoleId roleId) external view returns (uint256 numberOfMembers) { @@ -443,33 +327,48 @@ contract AccessAdmin is selector.toBytes4())); } - function canCall(address caller, address target, Selector selector) external virtual view returns (bool can) { - (can, ) = _authority.canCall(caller, target, selector.toBytes4()); + function deployer() public view returns (address) { + return _deployer; } - function toRole(RoleId adminRoleId, RoleType roleType, uint32 maxMemberCount, string memory name) public view returns (RoleInfo memory) { - return RoleInfo({ - name: StrLib.toStr(name), - adminRoleId: adminRoleId, - roleType: roleType, - maxMemberCount: maxMemberCount, - createdAt: TimestampLib.blockTimestamp(), - pausedAt: TimestampLib.max() - }); - } + //--- internal/private functions -------------------------------------------------// - function toFunction(bytes4 selector, string memory name) public view returns (FunctionInfo memory) { - return FunctionInfo({ - name: StrLib.toStr(name), - selector: SelectorLib.toSelector(selector), - createdAt: TimestampLib.blockTimestamp()}); - } + function _linkToNftOwnable(address registerable) internal { + if (!getRegistry().isRegistered(registerable)) { + revert ErrorAccessAdminNotRegistered(registerable); + } - function deployer() public view returns (address) { - return _deployer; + _linkedNftId = getRegistry().getNftIdForAddress(registerable); } - //--- internal/private functions -------------------------------------------------// + function _initializeAdminAndPublicRoles() + internal + virtual + onlyInitializing() + { + RoleId adminRoleId = RoleIdLib.toRoleId(_authority.ADMIN_ROLE()); + + // setup admin role + _createRoleUnchecked( + ADMIN_ROLE(), + AccessAdminLib.toRole({ + adminRoleId: ADMIN_ROLE(), + roleType: RoleType.Contract, + maxMemberCount: 1, + name: ADMIN_ROLE_NAME})); + + // add this contract as admin role member + _roleMembers[adminRoleId].add(address(this)); + + // setup public role + _createRoleUnchecked( + PUBLIC_ROLE(), + AccessAdminLib.toRole({ + adminRoleId: ADMIN_ROLE(), + roleType: RoleType.Gif, + maxMemberCount: type(uint32).max, + name: PUBLIC_ROLE_NAME})); + } function _createTargetWithRole( address target, @@ -649,32 +548,21 @@ contract AccessAdmin is ) internal { - // check role does not yet exist - if(roleExists(roleId)) { - revert ErrorAccessAdminRoleAlreadyCreated( - roleId, - _roleInfo[roleId].name.toString()); - } - - // check admin role exists - if(!roleExists(info.adminRoleId)) { - revert ErrorAccessAdminRoleAdminNotExisting(info.adminRoleId); - } - - // check role name is not empty - if(info.name.length() == 0) { - revert ErrorAccessAdminRoleNameEmpty(roleId); - } + AccessAdminLib.checkRoleCreation(this, roleId, info); + _createRoleUnchecked(roleId, info); + } - // check role name is not used for another role - if(_roleForName[info.name].exists) { - revert ErrorAccessAdminRoleNameAlreadyExists( - roleId, - info.name.toString(), - _roleForName[info.name].roleId); - } - _createRoleUnchecked(roleId, info); + function _createTarget( + address target, + string memory targetName, + bool checkAuthority, + bool custom + ) + internal + { + AccessAdminLib.checkTargetCreation(this, target, targetName, checkAuthority); + _createTargetUnchecked(target, targetName, custom); } @@ -700,49 +588,15 @@ contract AccessAdmin is } - function _createTarget( + function _createTargetUnchecked( address target, string memory targetName, - bool checkAuthority, bool custom ) internal { - // check target does not yet exist - if(targetExists(target)) { - revert ErrorAccessAdminTargetAlreadyCreated( - target, - _targetInfo[target].name.toString()); - } - - // check target name is not empty - Str name = StrLib.toStr(targetName); - if(name.length() == 0) { - revert ErrorAccessAdminTargetNameEmpty(target); - } - - // check target name is not used for another target - if( _targetForName[name] != address(0)) { - revert ErrorAccessAdminTargetNameAlreadyExists( - target, - targetName, - _targetForName[name]); - } - - // check target is an access managed contract - if (!_isAccessManaged(target)) { - revert ErrorAccessAdminTargetNotAccessManaged(target); - } - - // check target shares authority with this contract - if (checkAuthority) { - address targetAuthority = AccessManagedUpgradeable(target).authority(); - if (targetAuthority != authority()) { - revert ErrorAccessAdminTargetAuthorityMismatch(authority(), targetAuthority); - } - } - // create target info + Str name = StrLib.toStr(targetName); _targetInfo[target] = TargetInfo({ name: name, isCustom: custom, @@ -767,31 +621,6 @@ contract AccessAdmin is } - function _isAccessManaged(address target) - internal - view - returns (bool) - { - if (!ContractLib.isContract(target)) { - return false; - } - - (bool success, ) = target.staticcall( - abi.encodeWithSelector( - IAccessManagedChecker.authority.selector)); - - return success; - } - - - function _getAccountName(address account) internal view returns (string memory) { - if (targetExists(account)) { - return _targetInfo[account].name.toString(); - } - return ""; - } - - function _getRoleName(RoleId roleId) internal view returns (string memory) { if (roleExists(roleId)) { return _roleInfo[roleId].name.toString(); @@ -800,7 +629,25 @@ contract AccessAdmin is } - function _checkRoleExists( + function _checkAuthorization( + address authorization, + ObjectType expectedDomain, + VersionPart expectedRelease, + bool checkAlreadyInitialized + ) + internal + view + { + AccessAdminLib.checkAuthorization( + address(_authorization), + authorization, + expectedDomain, + expectedRelease, + checkAlreadyInitialized); + } + + + function _checkRoleExists( RoleId roleId, bool onlyActiveRole ) @@ -823,6 +670,32 @@ contract AccessAdmin is } + + /// @dev check if target exists and reverts if it doesn't + function _checkTargetExists( + address target + ) + internal + view + { + // check not yet created + if (!targetExists(target)) { + revert ErrorAccessAdminTargetNotCreated(target); + } + } + + + function _checkIsRegistered( + address registry, + address target, + ObjectType expectedType + ) + internal + view + { + AccessAdminLib.checkIsRegistered(registry, target, expectedType); + } + function _checkRegistry(address registry) internal view { if (!ContractLib.isRegistry(registry)) { revert ErrorAccessAdminNotRegistry(registry); diff --git a/contracts/authorization/AccessAdminLib.sol b/contracts/authorization/AccessAdminLib.sol new file mode 100644 index 000000000..a647ae0dc --- /dev/null +++ b/contracts/authorization/AccessAdminLib.sol @@ -0,0 +1,183 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.20; + +import {AccessManagedUpgradeable} from "@openzeppelin/contracts-upgradeable/access/manager/AccessManagedUpgradeable.sol"; + +import {IAccess} from "./IAccess.sol"; +import {IAccessAdmin} from "./IAccessAdmin.sol"; +import {IAuthorization} from "./IAuthorization.sol"; +import {IRegistry} from "../registry/IRegistry.sol"; + +import {ContractLib} from "../shared/ContractLib.sol"; +import {ObjectType} from "../type/ObjectType.sol"; +import {RoleId} from "../type/RoleId.sol"; +import {SelectorLib} from "../type/Selector.sol"; +import {Str, StrLib} from "../type/String.sol"; +import {TimestampLib} from "../type/Timestamp.sol"; +import {VersionPart} from "../type/Version.sol"; + + +library AccessAdminLib { // ACCESS_ADMIN_LIB + + function checkRoleCreation( + IAccessAdmin accessAdmin, + RoleId roleId, + IAccess.RoleInfo memory info + ) + public + view + { + // check role does not yet exist // ACCESS_ADMIN_LIB role creation checks + if(accessAdmin.roleExists(roleId)) { + revert IAccessAdmin.ErrorAccessAdminRoleAlreadyCreated( + roleId, + accessAdmin.getRoleInfo(roleId).name.toString()); + } + + // check admin role exists + if(!accessAdmin.roleExists(info.adminRoleId)) { + revert IAccessAdmin.ErrorAccessAdminRoleAdminNotExisting(info.adminRoleId); + } + + // check role name is not empty + if(info.name.length() == 0) { + revert IAccessAdmin.ErrorAccessAdminRoleNameEmpty(roleId); + } + + // check role name is not used for another role + RoleId roleIdForName = accessAdmin.getRoleForName(StrLib.toString(info.name)); + if(roleIdForName.gtz()) { + revert IAccessAdmin.ErrorAccessAdminRoleNameAlreadyExists( + roleId, + info.name.toString(), + roleIdForName); + } + } + + function checkTargetCreation( + IAccessAdmin accessAdmin, + address target, + string memory targetName, + bool checkAuthority + ) + public + view + { + // check target does not yet exist + if(accessAdmin.targetExists(target)) { + revert IAccessAdmin.ErrorAccessAdminTargetAlreadyCreated( + target, + accessAdmin.getTargetInfo(target).name.toString()); + } + + // check target name is not empty + Str name = StrLib.toStr(targetName); + if(name.length() == 0) { + revert IAccessAdmin.ErrorAccessAdminTargetNameEmpty(target); + } + + // check target name is not used for another target + address targetForName = accessAdmin.getTargetForName(name); + if(targetForName != address(0)) { + revert IAccessAdmin.ErrorAccessAdminTargetNameAlreadyExists( + target, + targetName, + targetForName); + } + + // check target is an access managed contract + if (!ContractLib.isAccessManaged(target)) { + revert IAccessAdmin.ErrorAccessAdminTargetNotAccessManaged(target); + } + + // check target shares authority with this contract + if (checkAuthority) { + address targetAuthority = AccessManagedUpgradeable(target).authority(); + if (targetAuthority != accessAdmin.authority()) { + revert IAccessAdmin.ErrorAccessAdminTargetAuthorityMismatch(accessAdmin.authority(), targetAuthority); + } + } + + } + + + function checkAuthorization( + address authorizationOld, + address authorization, + ObjectType expectedDomain, + VersionPart expectedRelease, + bool checkAlreadyInitialized + ) + public + view + { + // checks + // check not yet initialized + if (checkAlreadyInitialized && authorizationOld != address(0)) { + revert IAccessAdmin.ErrorAccessAdminAlreadyInitialized(authorizationOld); + } + + // check contract type matches + if (!ContractLib.supportsInterface(authorization, type(IAuthorization).interfaceId)) { + revert IAccessAdmin.ErrorAccessAdminNotAuthorization(authorization); + } + + // check domain matches + ObjectType domain = IAuthorization(authorization).getDomain(); + if (domain != expectedDomain) { + revert IAccessAdmin.ErrorAccessAdminDomainMismatch(authorization, expectedDomain, domain); + } + + // check release matches + VersionPart authorizationRelease = IAuthorization(authorization).getRelease(); + if (authorizationRelease != expectedRelease) { + revert IAccessAdmin.ErrorAccessAdminReleaseMismatch(authorization, expectedRelease, authorizationRelease); + } + } + + + function checkIsRegistered( + address registry, + address target, + ObjectType expectedType + ) + public + view + { + ObjectType tagetType = IRegistry(registry).getObjectInfo(target).objectType; + if (tagetType.eqz()) { + revert IAccessAdmin.ErrorAccessAdminNotRegistered(target); + } + + if (tagetType != expectedType) { + revert IAccessAdmin.ErrorAccessAdminTargetTypeMismatch(target, expectedType, tagetType); + } + } + + function toRole(RoleId adminRoleId, IAccessAdmin.RoleType roleType, uint32 maxMemberCount, string memory name) + public + view + returns (IAccess.RoleInfo memory) + { + return IAccess.RoleInfo({ + name: StrLib.toStr(name), + adminRoleId: adminRoleId, + roleType: roleType, + maxMemberCount: maxMemberCount, + createdAt: TimestampLib.blockTimestamp(), + pausedAt: TimestampLib.max() + }); + } + + function toFunction(bytes4 selector, string memory name) + public + view + returns (IAccess.FunctionInfo memory) + { + return IAccess.FunctionInfo({ + name: StrLib.toStr(name), + selector: SelectorLib.toSelector(selector), + createdAt: TimestampLib.blockTimestamp()}); + } + +} \ No newline at end of file diff --git a/contracts/authorization/AccessManagerCloneable.sol b/contracts/authorization/AccessManagerCloneable.sol index d3c6a3c49..4ecba7d7f 100644 --- a/contracts/authorization/AccessManagerCloneable.sol +++ b/contracts/authorization/AccessManagerCloneable.sol @@ -2,12 +2,11 @@ pragma solidity ^0.8.20; import {AccessManagerUpgradeable} from "@openzeppelin/contracts-upgradeable/access/manager/AccessManagerUpgradeable.sol"; -import {IAccessManaged} from "@openzeppelin/contracts/access/manager/IAccessManaged.sol"; import {IAccessManager} from "@openzeppelin/contracts/access/manager/IAccessManager.sol"; import {InitializableERC165} from "../shared/InitializableERC165.sol"; import {RegistryLinked} from "../shared/RegistryLinked.sol"; -import {VersionPart, VersionLib} from "../type/Version.sol"; +import {VersionPart} from "../type/Version.sol"; /// @dev An AccessManager based on OpenZeppelin that is cloneable and has a central lock property. diff --git a/contracts/authorization/Authorization.sol b/contracts/authorization/Authorization.sol index e154ce4a8..07d7c337e 100644 --- a/contracts/authorization/Authorization.sol +++ b/contracts/authorization/Authorization.sol @@ -5,7 +5,7 @@ import {IAccess} from "./IAccess.sol"; import {IAuthorization} from "./IAuthorization.sol"; import {InitializableERC165} from "../shared/InitializableERC165.sol"; -import {ObjectType, ObjectTypeLib, PRODUCT, ORACLE, DISTRIBUTION, POOL} from "../type/ObjectType.sol"; +import {ObjectType, ObjectTypeLib} from "../type/ObjectType.sol"; import {RoleId, RoleIdLib, ADMIN_ROLE} from "../type/RoleId.sol"; import {SelectorLib} from "../type/Selector.sol"; import {Str, StrLib} from "../type/String.sol"; @@ -28,7 +28,7 @@ contract Authorization is string internal _mainTargetName = "Component"; string internal _tokenHandlerName = "ComponentTH"; - ObjectType _domain; + ObjectType internal _domain; Str internal _mainTarget; Str internal _tokenHandlerTarget; Str[] internal _targets; diff --git a/contracts/authorization/IAccessAdmin.sol b/contracts/authorization/IAccessAdmin.sol index 6bb16ed16..70773f578 100644 --- a/contracts/authorization/IAccessAdmin.sol +++ b/contracts/authorization/IAccessAdmin.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.20; import {IAccessManaged} from "@openzeppelin/contracts/access/manager/IAccessManaged.sol"; import {IAccess} from "./IAccess.sol"; +import {IAuthorization} from "./IAuthorization.sol"; import {IRegistryLinked} from "../shared/IRegistryLinked.sol"; import {IRelease} from "../registry/IRelease.sol"; @@ -33,10 +34,10 @@ interface IAccessAdmin is error ErrorAccessAdminNotDeployer(); // only role admin modifier - error ErrorAccessAdminNotAdminOfRole(RoleId adminRoleId); + error ErrorAccessAdminNotAdminOfRole(RoleId adminRoleId, address account); // only role owner modifier - error ErrorAccessAdminNotRoleOwner(RoleId roleId); + error ErrorAccessAdminNotRoleOwner(RoleId roleId, address account); // initialization error ErrorAccessAdminNotRegistry(address registry); @@ -135,6 +136,7 @@ interface IAccessAdmin is //--- view functions ----------------------------------------------------// + function getAuthorization() external view returns (IAuthorization authorization); function getLinkedNftId() external view returns (NftId linkedNftId); function getLinkedOwner() external view returns (address linkedOwner); @@ -146,8 +148,8 @@ interface IAccessAdmin is function getPublicRole() external view returns (RoleId roleId); function roleExists(RoleId roleId) external view returns (bool exists); + function getRoleForName(string memory name) external view returns (RoleId roleId); function getRoleInfo(RoleId roleId) external view returns (RoleInfo memory roleInfo); - function getRoleForName(Str name) external view returns (RoleNameInfo memory); function hasRole(address account, RoleId roleId) external view returns (bool); function hasAdminRole(address account, RoleId roleId) external view returns (bool); @@ -155,17 +157,14 @@ interface IAccessAdmin is function getRoleMember(RoleId roleId, uint256 idx) external view returns (address account); function targetExists(address target) external view returns (bool exists); - function isTargetLocked(address target) external view returns (bool locked); + function getTargetForName(Str name) external view returns (address target); function targets() external view returns (uint256 numberOfTargets); function getTargetAddress(uint256 idx) external view returns (address target); function getTargetInfo(address target) external view returns (TargetInfo memory targetInfo); - function getTargetForName(Str name) external view returns (address target); + function isTargetLocked(address target) external view returns (bool locked); function authorizedFunctions(address target) external view returns (uint256 numberOfFunctions); function getAuthorizedFunction(address target, uint256 idx) external view returns (FunctionInfo memory func, RoleId roleId); - function canCall(address caller, address target, Selector selector) external view returns (bool can); - function toRole(RoleId adminRoleId, RoleType roleType, uint32 maxMemberCount, string memory name) external view returns (RoleInfo memory); - function toFunction(bytes4 selector, string memory name) external view returns (FunctionInfo memory); function deployer() external view returns (address); } \ No newline at end of file diff --git a/contracts/instance/InstanceAdmin.sol b/contracts/instance/InstanceAdmin.sol index 8d4853c18..6c7c6d7cf 100644 --- a/contracts/instance/InstanceAdmin.sol +++ b/contracts/instance/InstanceAdmin.sol @@ -427,7 +427,6 @@ contract InstanceAdmin is function _setupInstanceHelperTargetsWithRoles() internal { - // _checkAndCreateTargetWithRole(address(_instance), INSTANCE_TARGET_NAME); // create module targets _checkAndCreateTargetWithRole(address(_instance.getInstanceStore()), INSTANCE_STORE_TARGET_NAME); @@ -435,7 +434,7 @@ contract InstanceAdmin is _checkAndCreateTargetWithRole(address(_instance.getBundleSet()), BUNDLE_SET_TARGET_NAME); _checkAndCreateTargetWithRole(address(_instance.getRiskSet()), RISK_SET_TARGET_NAME); - // create targets for services that need to access the module targets + // create targets for services that need to access the instance targets ObjectType[] memory serviceDomains = _authorization.getServiceDomains(); VersionPart release = _authorization.getRelease(); ObjectType serviceDomain; diff --git a/contracts/registry/RegistryAdmin.sol b/contracts/registry/RegistryAdmin.sol index 278cb8016..e5ab042dd 100644 --- a/contracts/registry/RegistryAdmin.sol +++ b/contracts/registry/RegistryAdmin.sol @@ -8,6 +8,7 @@ import {IService} from "../shared/IService.sol"; import {IStaking} from "../staking/IStaking.sol"; import {AccessAdmin} from "../authorization/AccessAdmin.sol"; +import {AccessAdminLib} from "../authorization/AccessAdminLib.sol"; import {AccessManagerCloneable} from "../authorization/AccessManagerCloneable.sol"; import {ContractLib} from "../shared/ContractLib.sol"; import {ObjectType, REGISTRY, STAKING, POOL, RELEASE} from "../type/ObjectType.sol"; @@ -71,16 +72,6 @@ contract RegistryAdmin is address private _staking; address private _stakingStore; - // function initialize() - // external - // { - // AccessManagerCloneable accessManager = new AccessManagerCloneable(); - - // initialize( - // address(accessManager), - // "RegistryAdmin"); - // } - constructor() { initialize( address(new AccessManagerCloneable()), @@ -135,6 +126,31 @@ contract RegistryAdmin is } + + function grantServiceRoleForAllVersions( + IService service, + ObjectType domain + ) + external + restricted() + { + _grantRoleToAccount( + RoleIdLib.roleForTypeAndAllVersions(domain), + address(service)); + } + + //--- view functions ----------------------------------------------------// + + function getGifAdminRole() external view returns (RoleId) { + return GIF_ADMIN_ROLE(); + } + + function getGifManagerRole() external view returns (RoleId) { + return GIF_MANAGER_ROLE(); + } + + //--- private initialization functions -------------------------------------------// + // create registry role and target function _setupRegistry(address registry) internal { @@ -224,30 +240,6 @@ contract RegistryAdmin is } - function grantServiceRoleForAllVersions( - IService service, - ObjectType domain - ) - external - restricted() - { - _grantRoleToAccount( - RoleIdLib.roleForTypeAndAllVersions(domain), - address(service)); - } - - //--- view functions ----------------------------------------------------// - - function getGifAdminRole() external view returns (RoleId) { - return GIF_ADMIN_ROLE(); - } - - function getGifManagerRole() external view returns (RoleId) { - return GIF_MANAGER_ROLE(); - } - - //--- private initialization functions -------------------------------------------// - function _createTargets(address authorization) private onlyInitializing() @@ -263,211 +255,4 @@ contract RegistryAdmin is _createTarget(_tokenRegistry, TOKEN_REGISTRY_TARGET_NAME, true, false); _createTarget(tokenHandler, TOKEN_HANDLER_TARGET_NAME, true, false); } - - function _setupGifAdminRole(address gifAdmin) - private - onlyInitializing() - { - // create gif admin role - _createRole( - GIF_ADMIN_ROLE(), - toRole({ - adminRoleId: ADMIN_ROLE(), - roleType: RoleType.Gif, - maxMemberCount: 2, // TODO decide on max member count - name: GIF_ADMIN_ROLE_NAME})); - - // grant permissions to the gif admin role for registry contract - FunctionInfo[] memory functions; - functions = new FunctionInfo[](1); - functions[0] = toFunction(IRegistry.registerRegistry.selector, "registerRegistry"); - _authorizeTargetFunctions(_registry, GIF_ADMIN_ROLE(), functions); - - // grant permissions to the gif admin role for release registry contract - functions = new FunctionInfo[](3); - functions[0] = toFunction(ReleaseRegistry.createNextRelease.selector, "createNextRelease"); - functions[1] = toFunction(ReleaseRegistry.activateNextRelease.selector, "activateNextRelease"); - functions[2] = toFunction(ReleaseRegistry.setActive.selector, "setActive"); - _authorizeTargetFunctions(_releaseRegistry, GIF_ADMIN_ROLE(), functions); - - _grantRoleToAccount(GIF_ADMIN_ROLE(), gifAdmin); - } - - function _setupGifManagerRole(address gifManager) - private - onlyInitializing() - { - // create gif manager role - _createRole( - GIF_MANAGER_ROLE(), - toRole({ - adminRoleId: ADMIN_ROLE(), - roleType: RoleType.Gif, - maxMemberCount: 1, // TODO decide on max member count - name: GIF_MANAGER_ROLE_NAME})); - - // grant permissions to the gif manager role for token registry contract - FunctionInfo[] memory functions; - functions = new FunctionInfo[](5); - functions[0] = toFunction(TokenRegistry.registerToken.selector, "registerToken"); - functions[1] = toFunction(TokenRegistry.registerRemoteToken.selector, "registerRemoteToken"); - functions[2] = toFunction(TokenRegistry.setActive.selector, "setActive"); - functions[3] = toFunction(TokenRegistry.setActiveForVersion.selector, "setActiveForVersion"); - // TODO find a better way (only needed for testing) - functions[4] = toFunction(TokenRegistry.setActiveWithVersionCheck.selector, "setActiveWithVersionCheck"); - _authorizeTargetFunctions(_tokenRegistry, GIF_MANAGER_ROLE(), functions); - - // grant permissions to the gif manager role for release registry contract - functions = new FunctionInfo[](2); - functions[0] = toFunction(ReleaseRegistry.prepareNextRelease.selector, "prepareNextRelease"); - functions[1] = toFunction(ReleaseRegistry.registerService.selector, "registerService"); - _authorizeTargetFunctions(_releaseRegistry, GIF_MANAGER_ROLE(), functions); - - _grantRoleToAccount(GIF_MANAGER_ROLE(), gifManager); - } - - function _setupRegistryRoles() - private - onlyInitializing() - { - // TODO use RELEASE_REGISTRY_ROLE instead - // create and grant release registry role - RoleId releaseRegistryRoleId = RoleIdLib.roleForType(RELEASE()); - _createRole( - releaseRegistryRoleId, - toRole({ - adminRoleId: ADMIN_ROLE(), - roleType: RoleType.Contract, - maxMemberCount: 1, - name: RELEASE_REGISTRY_ROLE_NAME})); - _grantRoleToAccount(releaseRegistryRoleId, _releaseRegistry); - - // grant permissions to the release registry role for release admin contract - FunctionInfo[] memory functions; - functions = new FunctionInfo[](1); - functions[0] = toFunction(RegistryAdmin.grantServiceRoleForAllVersions.selector, "grantServiceRoleForAllVersions"); - _authorizeTargetFunctions(address(this), releaseRegistryRoleId, functions); - - // grant permissions to the release registry role for registry contract - functions = new FunctionInfo[](1); - functions[0] = toFunction(IRegistry.registerService.selector, "registerService"); - _authorizeTargetFunctions(_registry, releaseRegistryRoleId, functions); - - // create registry service role - RoleId registryServiceRoleId = RoleIdLib.roleForTypeAndAllVersions(REGISTRY()); - _createRole( - registryServiceRoleId, - toRole({ - adminRoleId: ADMIN_ROLE(), - roleType: RoleType.Contract, - maxMemberCount: _getMaxRelases(), - name: REGISTRY_SERVICE_ROLE_NAME})); - - // grant permissions to the registry service role for registry contract - functions = new FunctionInfo[](2); - functions[0] = toFunction(IRegistry.register.selector, "register"); - functions[1] = toFunction(IRegistry.registerWithCustomType.selector, "registerWithCustomType"); - _authorizeTargetFunctions(_registry, registryServiceRoleId, functions); - } - - - function _setupStakingRoles() - private - onlyInitializing() - { - // create and grant staking contract role - RoleId stakingRoleId = RoleIdLib.roleForType(STAKING()); - _createRole( - stakingRoleId, - toRole({ - adminRoleId: ADMIN_ROLE(), - roleType: RoleType.Contract, - maxMemberCount: 1, - name: STAKING_ROLE_NAME})); - _grantRoleToAccount(stakingRoleId, _staking); - - // grant permissions to the staking role for staking store contract - FunctionInfo[] memory functions; - functions = new FunctionInfo[](14); - functions[0] = toFunction(StakingStore.setStakingRate.selector, "setStakingRate"); - functions[1] = toFunction(StakingStore.createTarget.selector, "createTarget"); - functions[2] = toFunction(StakingStore.updateTarget.selector, "updateTarget"); - functions[3] = toFunction(StakingStore.increaseReserves.selector, "increaseReserves"); - functions[4] = toFunction(StakingStore.decreaseReserves.selector, "decreaseReserves"); - functions[5] = toFunction(StakingStore.increaseTotalValueLocked.selector, "increaseTotalValueLocked"); - functions[6] = toFunction(StakingStore.decreaseTotalValueLocked.selector, "decreaseTotalValueLocked"); - functions[7] = toFunction(StakingStore.create.selector, "create"); - functions[8] = toFunction(StakingStore.update.selector, "update"); - functions[9] = toFunction(StakingStore.increaseStake.selector, "increaseStake"); - functions[10] = toFunction(StakingStore.restakeRewards.selector, "restakeRewards"); - functions[11] = toFunction(StakingStore.updateRewards.selector, "updateRewards"); - functions[12] = toFunction(StakingStore.claimUpTo.selector, "claimUpTo"); - functions[13] = toFunction(StakingStore.unstakeUpTo.selector, "unstakeUpTo"); - _authorizeTargetFunctions(_stakingStore, stakingRoleId, functions); - - // grant permissions to the staking role for token handler contract - IStaking staking = IStaking(_staking); - functions = new FunctionInfo[](2); - functions[0] = toFunction(TokenHandler.pullToken.selector, "pullToken"); - functions[1] = toFunction(TokenHandler.pushToken.selector, "pushToken"); - _authorizeTargetFunctions(address(staking.getTokenHandler()), stakingRoleId, functions); - - // create staking service role - RoleId stakingServiceRoleId = RoleIdLib.roleForTypeAndAllVersions(STAKING()); - _createRole( - stakingServiceRoleId, - toRole({ - adminRoleId: ADMIN_ROLE(), - roleType: RoleType.Contract, - maxMemberCount: _getMaxRelases(), - name: STAKING_SERVICE_ROLE_NAME})); - - // grant permissions to the staking service role for staking contract - functions = new FunctionInfo[](11); - functions[0] = toFunction(IStaking.registerTarget.selector, "registerTarget"); - functions[1] = toFunction(IStaking.setLockingPeriod.selector, "setLockingPeriod"); - functions[2] = toFunction(IStaking.setRewardRate.selector, "setRewardRate"); - functions[3] = toFunction(IStaking.refillRewardReserves.selector, "refillRewardReserves"); - functions[4] = toFunction(IStaking.withdrawRewardReserves.selector, "withdrawRewardReserves"); - functions[5] = toFunction(IStaking.createStake.selector, "createStake"); - functions[6] = toFunction(IStaking.stake.selector, "stake"); - functions[7] = toFunction(IStaking.unstake.selector, "unstake"); - functions[8] = toFunction(IStaking.restake.selector, "restake"); - functions[9] = toFunction(IStaking.updateRewards.selector, "updateRewards"); - functions[10] = toFunction(IStaking.claimRewards.selector, "claimRewards"); - _authorizeTargetFunctions(_staking, stakingServiceRoleId, functions); - - // grant permissions to the staking service role for staking token handler - functions = new FunctionInfo[](3); - functions[0] = toFunction(TokenHandler.approve.selector, "approve"); - functions[1] = toFunction(TokenHandler.pullToken.selector, "pullToken"); - functions[2] = toFunction(TokenHandler.pushToken.selector, "pushToken"); - _authorizeTargetFunctions( - address(IComponent(_staking).getTokenHandler()), stakingServiceRoleId, functions); - - // create pool service role - RoleId poolServiceRoleId = RoleIdLib.roleForTypeAndAllVersions(POOL()); - _createRole( - poolServiceRoleId, - toRole({ - adminRoleId: ADMIN_ROLE(), - roleType: RoleType.Contract, - maxMemberCount: _getMaxRelases(), - name: POOL_SERVICE_ROLE_NAME})); - - // grant permissions to the pool service role for staking contract - functions = new FunctionInfo[](2); - functions[0] = toFunction(IStaking.increaseTotalValueLocked.selector, "increaseTotalValueLocked"); - functions[1] = toFunction(IStaking.decreaseTotalValueLocked.selector, "decreaseTotalValueLocked"); - _authorizeTargetFunctions(_staking, poolServiceRoleId, functions); - - // grant permissions to public role for staking contract - functions = new FunctionInfo[](1); - functions[0] = toFunction(Staking.approveTokenHandler.selector, "approveTokenHandler"); - _authorizeTargetFunctions(_staking, PUBLIC_ROLE(), functions); - } - - function _getMaxRelases() internal pure returns (uint32) { - return uint32(VersionPartLib.releaseMax().toInt()); - } } \ No newline at end of file diff --git a/contracts/registry/ReleaseAdmin.sol b/contracts/registry/ReleaseAdmin.sol index 0a3f5be18..bfe9cc21b 100644 --- a/contracts/registry/ReleaseAdmin.sol +++ b/contracts/registry/ReleaseAdmin.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.20; import {AccessAdmin} from "../authorization/AccessAdmin.sol"; +import {AccessAdminLib} from "../authorization/AccessAdminLib.sol"; import {AccessManagerCloneable} from "../authorization/AccessManagerCloneable.sol"; import {IAccess} from "../authorization/IAccess.sol"; import {IService} from "../shared/IService.sol"; @@ -158,7 +159,7 @@ contract ReleaseAdmin is if(!roleExists(serviceRoleId)) { _createRole( serviceRoleId, - toRole({ + AccessAdminLib.toRole({ adminRoleId: ADMIN_ROLE(), roleType: RoleType.Contract, maxMemberCount: 1, @@ -203,7 +204,7 @@ contract ReleaseAdmin is // create role for authorized domain _createRole( authorizedRoleId, - toRole({ + AccessAdminLib.toRole({ adminRoleId: ADMIN_ROLE(), roleType: RoleType.Contract, maxMemberCount: 1, @@ -235,7 +236,7 @@ contract ReleaseAdmin is _createRole( RELEASE_REGISTRY_ROLE(), - toRole({ + AccessAdminLib.toRole({ adminRoleId: ADMIN_ROLE(), roleType: RoleType.Contract, maxMemberCount: 1, @@ -243,8 +244,8 @@ contract ReleaseAdmin is FunctionInfo[] memory functions; functions = new FunctionInfo[](2); - functions[0] = toFunction(ReleaseAdmin.authorizeService.selector, "authorizeService"); - functions[1] = toFunction(ReleaseAdmin.setServiceLocked.selector, "setServiceLocked"); + functions[0] = AccessAdminLib.toFunction(ReleaseAdmin.authorizeService.selector, "authorizeService"); + functions[1] = AccessAdminLib.toFunction(ReleaseAdmin.setServiceLocked.selector, "setServiceLocked"); _authorizeTargetFunctions(address(this), RELEASE_REGISTRY_ROLE(), functions); _grantRoleToAccount(RELEASE_REGISTRY_ROLE(), releaseRegistry); diff --git a/contracts/registry/ReleaseRegistry.sol b/contracts/registry/ReleaseRegistry.sol index 874dd7eff..2d8d554dd 100644 --- a/contracts/registry/ReleaseRegistry.sol +++ b/contracts/registry/ReleaseRegistry.sol @@ -4,27 +4,25 @@ pragma solidity ^0.8.20; import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; import {AccessManaged} from "@openzeppelin/contracts/access/manager/AccessManaged.sol"; -import {ContractLib} from "../shared/ContractLib.sol"; -import {NftId} from "../type/NftId.sol"; -import {ObjectType, ObjectTypeLib, POOL, RELEASE, REGISTRY, SERVICE, STAKING} from "../type/ObjectType.sol"; -import {TimestampLib, zeroTimestamp} from "../type/Timestamp.sol"; -import {Seconds} from "../type/Seconds.sol"; -import {StateId, SCHEDULED, DEPLOYING, DEPLOYED, SKIPPED, ACTIVE, PAUSED} from "../type/StateId.sol"; -import {VersionPart, VersionPartLib} from "../type/Version.sol"; - -import {IService} from "../shared/IService.sol"; - import {IAccessAdmin} from "../authorization/IAccessAdmin.sol"; import {AccessManagerCloneable} from "../authorization/AccessManagerCloneable.sol"; - import {IRegistry} from "./IRegistry.sol"; import {IRelease} from "./IRelease.sol"; import {IRegistryLinked} from "../shared/IRegistryLinked.sol"; +import {IService} from "../shared/IService.sol"; import {IServiceAuthorization} from "../authorization/IServiceAuthorization.sol"; + +import {ContractLib} from "../shared/ContractLib.sol"; +import {NftId} from "../type/NftId.sol"; +import {ObjectType, ObjectTypeLib, POOL, RELEASE, REGISTRY, SERVICE, STAKING} from "../type/ObjectType.sol"; import {RegistryAdmin} from "./RegistryAdmin.sol"; import {Registry} from "./Registry.sol"; -import {ReleaseLifecycle} from "./ReleaseLifecycle.sol"; import {ReleaseAdmin} from "./ReleaseAdmin.sol"; +import {ReleaseLifecycle} from "./ReleaseLifecycle.sol"; +import {Seconds} from "../type/Seconds.sol"; +import {StateId, SCHEDULED, DEPLOYING, DEPLOYED, SKIPPED, ACTIVE, PAUSED} from "../type/StateId.sol"; +import {TimestampLib, zeroTimestamp} from "../type/Timestamp.sol"; +import {VersionPart, VersionPartLib} from "../type/Version.sol"; /// @dev The ReleaseRegistry manages the lifecycle of major GIF releases and their services. /// The creation of a new GIF release is a multi-step process: @@ -73,7 +71,7 @@ contract ReleaseRegistry is error ErrorReleaseRegistryServiceSelfRegistration(IService service); error ErrorReleaseRegistryServiceOwnerRegistered(IService service, address owner); - RegistryAdmin public immutable _admin; + RegistryAdmin public immutable _registryAdmin; Registry public immutable _registry; mapping(VersionPart release => IRelease.ReleaseInfo info) internal _releaseInfo; @@ -87,6 +85,7 @@ contract ReleaseRegistry is uint256 internal _registeredServices = 0; uint256 internal _servicesToRegister = 0; + // TODO move master relase admin outside constructor (same construction as for registry admin) constructor(Registry registry) AccessManaged(msg.sender) { @@ -97,10 +96,9 @@ contract ReleaseRegistry is setAuthority(registry.getAuthority()); _registry = registry; - _admin = RegistryAdmin(_registry.getRegistryAdminAddress()); - + _registryAdmin = RegistryAdmin(_registry.getRegistryAdminAddress()); _masterReleaseAdmin = new ReleaseAdmin( - address(new AccessManagerCloneable())); + _cloneNewAccessManager()); _next = VersionPartLib.toVersionPart(INITIAL_GIF_VERSION - 1); } @@ -254,16 +252,16 @@ contract ReleaseRegistry is revert ErrorReleaseRegistryRegistryServiceMissing(release); } - _admin.grantServiceRoleForAllVersions(IService(service), REGISTRY()); + _registryAdmin.grantServiceRoleForAllVersions(IService(service), REGISTRY()); service = _registry.getServiceAddress(STAKING(), release); if(service != address(0)) { - _admin.grantServiceRoleForAllVersions(IService(service), STAKING()); + _registryAdmin.grantServiceRoleForAllVersions(IService(service), STAKING()); } service = _registry.getServiceAddress(POOL(), release); if(service != address(0)) { - _admin.grantServiceRoleForAllVersions(IService(service), POOL()); + _registryAdmin.grantServiceRoleForAllVersions(IService(service), POOL()); } _setReleaseLocked(release, false); @@ -351,7 +349,7 @@ contract ReleaseRegistry is } function getRegistryAdmin() external view returns (address) { - return address(_admin); + return address(_registryAdmin); } //--- IRegistryLinked ------------------------------------------------------// @@ -369,16 +367,10 @@ contract ReleaseRegistry is _releaseInfo[release].releaseAdmin).setReleaseLocked(locked); } - function _cloneNewReleaseAdmin(VersionPart release) private returns (ReleaseAdmin clonedAdmin) { - // clone release specific access manager - AccessManagerCloneable clonedAccessManager = AccessManagerCloneable( - Clones.clone( - _masterReleaseAdmin.authority())); - // clone and setup release specific release admin clonedAdmin = ReleaseAdmin( Clones.clone(address(_masterReleaseAdmin))); @@ -389,7 +381,7 @@ contract ReleaseRegistry is release.toString())); clonedAdmin.initialize( - address(clonedAccessManager), + address(_cloneNewAccessManager()), releaseAdminName); clonedAdmin.completeSetup( @@ -402,6 +394,14 @@ contract ReleaseRegistry is } + function _cloneNewAccessManager() + private + returns (address accessManager) + { + return Clones.clone(address(_registryAdmin.authority())); + } + + function _verifyServiceAuthorization( IServiceAuthorization serviceAuthorization, VersionPart releaseVersion, @@ -413,7 +413,7 @@ contract ReleaseRegistry is { // authorization contract supports IServiceAuthorization interface if(!serviceAuthorization.supportsInterface(type(IServiceAuthorization).interfaceId)) { - revert ErrorReleaseRegistryNotServiceAuth(address(serviceAuthorization)); + revert ErrorReleaseRegistryNotServiceAuth(address(serviceAuthorization)); } // authorizaions contract version matches with release version diff --git a/contracts/shared/ContractLib.sol b/contracts/shared/ContractLib.sol index 51dac1cc6..993c7f7ec 100644 --- a/contracts/shared/ContractLib.sol +++ b/contracts/shared/ContractLib.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.20; import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; import {IAccessManager} from "@openzeppelin/contracts/access/manager/IAccessManager.sol"; +import {IAccessManaged} from "@openzeppelin/contracts/access/manager/IAccessManaged.sol"; // import {InstanceAdmin} from "../instance/InstanceAdmin.sol"; import {IRegistry} from "../registry/IRegistry.sol"; @@ -170,6 +171,23 @@ library ContractLib { } + function isAccessManaged(address target) + public + view + returns (bool) + { + if (!isContract(target)) { + return false; + } + + (bool success, ) = target.staticcall( + abi.encodeWithSelector( + IAccessManaged.authority.selector)); + + return success; + } + + function isRegistered(address registry, address caller, ObjectType expectedType) public view returns (bool) { NftId nftId = IRegistry(registry).getNftIdForAddress(caller); if (nftId.eqz()) { diff --git a/scripts/libs/instance.ts b/scripts/libs/instance.ts index ff0009568..c5f292c2d 100644 --- a/scripts/libs/instance.ts +++ b/scripts/libs/instance.ts @@ -35,6 +35,7 @@ export async function deployAndRegisterMasterInstance( registry: RegistryAddresses, services: ServiceAddresses, ): Promise { + logger.info("======== Starting deployment of master instance ========"); const { address: masterInstanceAuthorizationV3Address } = await deployContract( @@ -53,13 +54,37 @@ export async function deployAndRegisterMasterInstance( } ); + const { address: masterAccessManagerAddress, contract: masterAccessManagerBaseContract } = await deployContract( + "AccessManagerCloneable", + owner, + undefined, + { + libraries: { + ContractLib: libraries.contractLibAddress, + VersionPartLib: libraries.versionPartLibAddress, + } + } + ); + const masterAccessManager = masterAccessManagerBaseContract as AccessManagerCloneable; + + prepareVerificationData( + "AccessManagerCloneable", + await masterAccessManager, + [], + undefined + ); + const { address: masterInstanceAdminAddress, contract: masterInstanceAdminContract } = await deployContract( "InstanceAdmin", owner, - [masterInstanceAuthorizationV3Address], + [ + masterAccessManagerAddress + ], { libraries: { + AccessAdminLib: libraries.accessAdminLibAddress, ContractLib: libraries.contractLibAddress, + NftIdLib: libraries.nftIdLibAddress, RoleIdLib: libraries.roleIdLibAddress, SelectorLib: libraries.selectorLibAddress, SelectorSetLib: libraries.selectorSetLibAddress, @@ -71,13 +96,6 @@ export async function deployAndRegisterMasterInstance( ); const masterInstanceAdmin = masterInstanceAdminContract as InstanceAdmin; - prepareVerificationData( - "AccessManagerCloneable", - await masterInstanceAdmin.authority(), - [], - undefined - ); - const { address: masterInstanceStoreAddress, contract: masterInstanceStoreContract } = await deployContract( "InstanceStore", owner, @@ -175,7 +193,8 @@ export async function deployAndRegisterMasterInstance( masterInstanceBundleSet, masterInstanceRiskSet, masterInstanceReader, - registry.registryAddress, + registry.registryAddress, + 3, resolveAddress(owner), getTxOpts()), "masterInstance initialize", @@ -194,6 +213,18 @@ export async function deployAndRegisterMasterInstance( // nftId is the first field of the ObjectInfo struct const masterInstanceNfdId = (logRegistrationInfo as unknown); + // wire instance admin to registry, instance and instance authorization + await executeTx( + () => masterInstanceAdmin.completeSetup( + registry.registryAddress, + masterInstanceAddress, + masterInstanceAuthorizationV3Address, + 3, + getTxOpts()), + "masterInstanceAdmin completeSetup", + [masterInstanceAdmin.interface] + ); + await executeTx( () => registry.chainNft.transferFrom( resolveAddress(owner), diff --git a/scripts/libs/libraries.ts b/scripts/libs/libraries.ts index 603bff002..57ca36291 100644 --- a/scripts/libs/libraries.ts +++ b/scripts/libs/libraries.ts @@ -37,6 +37,7 @@ export type LibraryAddresses = { tokenHandlerDeployerLibAddress: AddressLike; objectSetHelperLibAddress: AddressLike; policyServiceLibAddress: AddressLike; + accessAdminLibAddress: AddressLike; } export const LIBRARY_ADDRESSES: Map = new Map(); @@ -90,7 +91,11 @@ export async function deployLibraries(owner: Signer): Promise "ObjectTypeLib", owner, undefined, - undefined); + { + libraries: { + VersionPartLib: versionPartLibAddress, + } + }); LIBRARY_ADDRESSES.set("ObjectTypeLib", objectTypeLibAddress); const { address: roleIdLibAddress } = await deployContract( @@ -344,7 +349,23 @@ export async function deployLibraries(owner: Signer): Promise } }); LIBRARY_ADDRESSES.set("PolicyServiceLib", policyServiceLibAddress); - + + const { address: accessAdminLibAddress } = await deployContract( + "AccessAdminLib", + owner, + undefined, + { + libraries: { + ContractLib: contractLibAddress, + ObjectTypeLib: objectTypeLibAddress, + RoleIdLib: roleIdLibAddress, + SelectorLib: selectorLibAddress, + StrLib: strLibAddress, + TimestampLib: timestampLibAddress, + } + }); + LIBRARY_ADDRESSES.set("AccessAdminLib", accessAdminLibAddress); + logger.info("======== Finished deployment of libraries ========"); dumpLibraryAddressesToFile(LIBRARY_ADDRESSES); @@ -381,6 +402,7 @@ export async function deployLibraries(owner: Signer): Promise tokenHandlerDeployerLibAddress, objectSetHelperLibAddress, policyServiceLibAddress, + accessAdminLibAddress, }; } diff --git a/scripts/libs/registry.ts b/scripts/libs/registry.ts index e5f04f37e..a63732b9b 100644 --- a/scripts/libs/registry.ts +++ b/scripts/libs/registry.ts @@ -6,6 +6,7 @@ import { Dip, Registry, RegistryAdmin, + RegistryAuthorization, ReleaseRegistry, ServiceAuthorizationV3, Staking, StakingManager, @@ -36,6 +37,9 @@ export type RegistryAddresses = { chainNftAddress: AddressLike; chainNft: ChainNft; + registryAuthorizationAddress : AddressLike; + registryAuthorization: RegistryAuthorization; + releaseRegistryAddress : AddressLike; releaseRegistry: ReleaseRegistry; @@ -77,6 +81,25 @@ export async function deployAndInitializeRegistry(owner: Signer, libraries: Libr const dip = dipBaseContract as Dip; // const dipMainnetAddress = "0xc719d010b63e5bbf2c0551872cd5316ed26acd83"; + logger.info("-------- Starting deployment Registry Authorization ----------------"); + + const { address: registryAuthorizationAddress, contract: registryAuthorizationBaseContract } = await deployContract( + "RegistryAuthorization", + owner, + [ + ], + { + libraries: { + ObjectTypeLib: libraries.objectTypeLibAddress, + RoleIdLib: libraries.roleIdLibAddress, + SelectorLib: libraries.selectorLibAddress, + StrLib: libraries.strLibAddress, + TimestampLib: libraries.timestampLibAddress, + VersionPartLib: libraries.versionPartLibAddress, + } + }); + + const registryAuthorization = registryAuthorizationBaseContract as RegistryAuthorization; logger.info("-------- Starting deployment Registry Admin ----------------"); @@ -86,6 +109,7 @@ export async function deployAndInitializeRegistry(owner: Signer, libraries: Libr [], { libraries: { + AccessAdminLib: libraries.accessAdminLibAddress, ContractLib: libraries.contractLibAddress, NftIdLib: libraries.nftIdLibAddress, RoleIdLib: libraries.roleIdLibAddress, @@ -128,16 +152,18 @@ export async function deployAndInitializeRegistry(owner: Signer, libraries: Libr [registryAddress], { libraries: { + AccessAdminLib: libraries.accessAdminLibAddress, ContractLib: libraries.contractLibAddress, - StateIdLib: libraries.stateIdLibAddress, - TimestampLib: libraries.timestampLibAddress, - VersionLib: libraries.versionLibAddress, - VersionPartLib: libraries.versionPartLibAddress, + NftIdLib: libraries.nftIdLibAddress, ObjectTypeLib: libraries.objectTypeLibAddress, RoleIdLib: libraries.roleIdLibAddress, SelectorLib: libraries.selectorLibAddress, SelectorSetLib: libraries.selectorSetLibAddress, + StateIdLib: libraries.stateIdLibAddress, StrLib: libraries.strLibAddress, + TimestampLib: libraries.timestampLibAddress, + VersionLib: libraries.versionLibAddress, + VersionPartLib: libraries.versionPartLibAddress, } }); @@ -245,7 +271,7 @@ export async function deployAndInitializeRegistry(owner: Signer, libraries: Libr ); await executeTx( - async () => await registryAdmin.completeSetup(registry, owner, owner, getTxOpts()), + async () => await registryAdmin.completeSetup(registry, registryAuthorization, owner, owner, getTxOpts()), "registryAdmin.completeSetup", [registryAdmin.interface] ); @@ -275,6 +301,7 @@ export async function deployAndInitializeRegistry(owner: Signer, libraries: Libr logger.info(`Dip deployed at ${dipAddress}`); + logger.info(`RegistryAuthorization deployeqd at ${registryAuthorizationAddress}`); logger.info(`RegistryAdmin deployeqd at ${registryAdmin}`); logger.info(`Registry deployed at ${registryAddress}`); logger.info(`ChainNft deployed at ${chainNftAddress}`); @@ -320,6 +347,9 @@ export async function deployAndInitializeRegistry(owner: Signer, libraries: Libr chainNftAddress: chainNftAddress, chainNft: chainNft, + registryAuthorizationAddress: registryAuthorizationAddress, + registryAuthorization: registryAuthorization, + releaseRegistryAddress: releaseRegistryAddress, releaseRegistry: releaseRegistry, diff --git a/scripts/libs/services.ts b/scripts/libs/services.ts index db8029543..9b542cdc6 100644 --- a/scripts/libs/services.ts +++ b/scripts/libs/services.ts @@ -208,6 +208,7 @@ export async function deployAndRegisterServices(owner: Signer, registry: Registr TargetManagerLib: libraries.targetManagerLibAddress, TimestampLib: libraries.timestampLibAddress, VersionLib: libraries.versionLibAddress, + VersionPartLib: libraries.versionPartLibAddress, }}); const instanceServiceManager = instanceServiceManagerBaseContract as InstanceServiceManager; diff --git a/test/GifDeployer.t.sol b/test/GifDeployer.t.sol index a2cc1078f..4d403fa37 100644 --- a/test/GifDeployer.t.sol +++ b/test/GifDeployer.t.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.20; +import {IAccessManager} from "@openzeppelin/contracts/access/manager/IAccessManager.sol"; import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; import {console} from "../lib/forge-std/src/Test.sol"; @@ -118,20 +119,14 @@ contract GifDeployerTest is GifDeployer { assertTrue(registryAdmin.hasRole(gifManager, GIF_MANAGER_ROLE()), "registry owner not manager"); // check sample admin access - assertTrue( - registryAdmin.canCall( - gifAdmin, // caller - address(releaseRegistry), // target - _toSelector(ReleaseRegistry.createNextRelease.selector)), - "gif manager cannot call registerToken"); + IAccessManager authority = IAccessManager(registryAdmin.authority()); + bool can; + (can,) = authority.canCall(gifAdmin, address(registryAdmin), TokenRegistry.registerToken.selector); + assertFalse(can, "gif admin cannot call registerToken"); // check sample manager access - assertTrue( - registryAdmin.canCall( - gifManager, // caller - address(tokenRegistry), // target - _toSelector(TokenRegistry.registerToken.selector)), - "gif manager cannot call registerToken"); + (can,) = authority.canCall(gifManager, address(tokenRegistry), TokenRegistry.registerToken.selector); + assertTrue(can, "gif manager cannot call registerToken"); // check linked contracts assertEq(address(releaseRegistry.getRegistry()), address(registry), "unexpected registry address"); @@ -141,10 +136,6 @@ contract GifDeployerTest is GifDeployer { // TODO amend once full gif setup is streamlined } - function _toSelector(bytes4 selector) internal pure returns (Selector) { - return SelectorLib.toSelector(selector); - } - function test_deployerCoreStakingManager() public { assertTrue(address(stakingManager) != address(0), "staking manager address zero"); diff --git a/test/authorization/AccessAdmin.t.sol b/test/authorization/AccessAdmin.t.sol index 4eb6e17e5..f23e88368 100644 --- a/test/authorization/AccessAdmin.t.sol +++ b/test/authorization/AccessAdmin.t.sol @@ -9,6 +9,7 @@ import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.s import {Test, console} from "../../lib/forge-std/src/Test.sol"; import {AccessAdmin} from "../../contracts/authorization/AccessAdmin.sol"; +import {AccessAdminLib} from "../../contracts/authorization/AccessAdminLib.sol"; import {AccessManagerCloneable} from "../../contracts/authorization/AccessManagerCloneable.sol"; import {AccessManagedMock} from "../mock/AccessManagedMock.sol"; import {IAccess} from "../../contracts/authorization/IAccess.sol"; @@ -56,7 +57,7 @@ contract AccessAdminForTesting is AccessAdmin { _managerRoleId = RoleIdLib.toRoleId(MANAGER_ROLE); _createRole( _managerRoleId, - toRole( + AccessAdminLib.toRole( getAdminRole(), RoleType.Custom, 3, // max accounts with this role @@ -65,20 +66,20 @@ contract AccessAdminForTesting is AccessAdmin { // grant public role access to grant and revoke, renounce FunctionInfo[] memory functions; functions = new FunctionInfo[](4); - functions[0] = toFunction(AccessAdminForTesting.grantManagerRole.selector, "grantManagerRole"); - functions[1] = toFunction(AccessAdminForTesting.grantRole.selector, "grantRole"); - functions[2] = toFunction(AccessAdminForTesting.revokeRole.selector, "revokeRole"); - functions[3] = toFunction(AccessAdminForTesting.renounceRole.selector, "renounceRole"); + functions[0] = AccessAdminLib.toFunction(AccessAdminForTesting.grantManagerRole.selector, "grantManagerRole"); + functions[1] = AccessAdminLib.toFunction(AccessAdminForTesting.grantRole.selector, "grantRole"); + functions[2] = AccessAdminLib.toFunction(AccessAdminForTesting.revokeRole.selector, "revokeRole"); + functions[3] = AccessAdminLib.toFunction(AccessAdminForTesting.renounceRole.selector, "renounceRole"); _authorizeTargetFunctions(address(this), getPublicRole(), functions); // grant manager role access to the specified functions functions = new FunctionInfo[](6); - functions[0] = toFunction(AccessAdminForTesting.createRole.selector, "createRole"); - functions[1] = toFunction(AccessAdminForTesting.createRoleExtended.selector, "createRoleExtended"); - functions[2] = toFunction(AccessAdminForTesting.createTarget.selector, "createTarget"); - functions[3] = toFunction(AccessAdminForTesting.setTargetLocked.selector, "setTargetLocked"); - functions[4] = toFunction(AccessAdminForTesting.authorizeFunctions.selector, "authorizeFunctions"); - functions[5] = toFunction(AccessAdminForTesting.unauthorizeFunctions.selector, "unauthorizeFunctions"); + functions[0] = AccessAdminLib.toFunction(AccessAdminForTesting.createRole.selector, "createRole"); + functions[1] = AccessAdminLib.toFunction(AccessAdminForTesting.createRoleExtended.selector, "createRoleExtended"); + functions[2] = AccessAdminLib.toFunction(AccessAdminForTesting.createTarget.selector, "createTarget"); + functions[3] = AccessAdminLib.toFunction(AccessAdminForTesting.setTargetLocked.selector, "setTargetLocked"); + functions[4] = AccessAdminLib.toFunction(AccessAdminForTesting.authorizeFunctions.selector, "authorizeFunctions"); + functions[5] = AccessAdminLib.toFunction(AccessAdminForTesting.unauthorizeFunctions.selector, "unauthorizeFunctions"); _authorizeTargetFunctions(address(this), getManagerRole(), functions); _grantRoleToAccount(_managerRoleId, _deployer); @@ -104,7 +105,7 @@ contract AccessAdminForTesting is AccessAdmin { { _createRole( roleId, - toRole( + AccessAdminLib.toRole( adminRoleId, RoleType.Custom, type(uint32).max, @@ -123,7 +124,7 @@ contract AccessAdminForTesting is AccessAdmin { { _createRole( roleId, - toRole( + AccessAdminLib.toRole( adminRoleId, roleType, maxOneRoleMember, @@ -136,7 +137,7 @@ contract AccessAdminForTesting is AccessAdmin { ) external virtual - onlyRoleAdmin(roleId) + onlyRoleAdmin(roleId, msg.sender) restricted() { _grantRoleToAccount(roleId, account); @@ -148,8 +149,8 @@ contract AccessAdminForTesting is AccessAdmin { ) external virtual - onlyRoleAdmin(roleId) restricted() + onlyRoleAdmin(roleId, msg.sender) { _revokeRoleFromAccount(roleId, account); } @@ -159,8 +160,8 @@ contract AccessAdminForTesting is AccessAdmin { ) external virtual - onlyRoleMember(roleId) restricted() + onlyRoleMember(roleId, msg.sender) { _revokeRoleFromAccount(roleId, msg.sender); } @@ -796,7 +797,8 @@ contract AccessAdminTest is AccessAdminBaseTest { vm.expectRevert( abi.encodeWithSelector( IAccessAdmin.ErrorAccessAdminNotAdminOfRole.selector, - newAdminRoleId)); + newAdminRoleId, + accessAdminDeployer)); vm.startPrank(accessAdminDeployer); accessAdmin.grantRole(account1, newRoleId); @@ -847,7 +849,8 @@ contract AccessAdminTest is AccessAdminBaseTest { vm.expectRevert( abi.encodeWithSelector( IAccessAdmin.ErrorAccessAdminNotAdminOfRole.selector, - newAdminRoleId)); + newAdminRoleId, + roleAdmin)); vm.startPrank(roleAdmin); accessAdmin.grantRole(account1, newRoleId); @@ -963,11 +966,12 @@ contract AccessAdminTest is AccessAdminBaseTest { vm.startPrank(account1); - // remove role account1 no longer has + // attempt to renounce role account1 no longer has vm.expectRevert( abi.encodeWithSelector( IAccessAdmin.ErrorAccessAdminNotRoleOwner.selector, - newRoleId)); + newRoleId, + account1)); accessAdmin.renounceRole(newRoleId); @@ -987,7 +991,8 @@ contract AccessAdminTest is AccessAdminBaseTest { vm.expectRevert( abi.encodeWithSelector( IAccessAdmin.ErrorAccessAdminNotRoleOwner.selector, - newRoleId)); + newRoleId, + account2)); accessAdmin.renounceRole(newRoleId); @@ -1052,7 +1057,7 @@ contract AccessAdminTest is AccessAdminBaseTest { // renouncing non existent role -> role unknown vm.expectRevert( abi.encodeWithSelector( - IAccessAdmin.ErrorAccessAdminNotRoleOwner.selector, + IAccessAdmin.ErrorAccessAdminRoleUnknown.selector, missingRoleId)); accessAdmin.renounceRole(missingRoleId); @@ -1283,8 +1288,6 @@ contract AccessAdminTest is AccessAdminBaseTest { RoleId missingRoleId = RoleIdLib.toRoleId(1313); assertFalse(aa.roleExists(missingRoleId), "missing role exists"); - assertFalse(aa.getRoleForName(StrLib.toStr("NoSuchRole")).exists, "NoSuchRole exists"); - // minimal check on access manager of access admin AccessManager accessManager = AccessManager(aa.authority()); bool isMember; @@ -1327,18 +1330,12 @@ contract AccessAdminTest is AccessAdminBaseTest { console.log("checking role", expectedName); assertTrue(aa.roleExists(roleId), "role does not exist"); - assertEq(aa.getRoleForName(StrLib.toStr(expectedName)).roleId.toInt(), roleId.toInt(), "unexpected roleId for getRoleForName"); IAccessAdmin.RoleInfo memory info = aa.getRoleInfo(roleId); assertEq(info.adminRoleId.toInt(), expectedAdminRoleId.toInt(), "unexpected admin role (role info)"); assertEq(info.name.toString(), expectedName, "unexpected role name"); assertEq(info.maxMemberCount, expectedMaxMemberCount, "unexpected maxMemberCount"); assertTrue(info.createdAt.gtz(), "role does not exist"); - - Str roleName = StrLib.toStr(expectedName); - IAccessAdmin.RoleNameInfo memory nameInfo = aa.getRoleForName(roleName); - assertTrue(nameInfo.exists, "role name info missing"); - assertEq(nameInfo.roleId.toInt(), roleId.toInt(), "unexpected role name rold id"); } function _printRoleMembers(AccessAdmin aa, RoleId roleId) internal { diff --git a/test/authorization/AccessAdminManageMock.t.sol b/test/authorization/AccessAdminManageMock.t.sol index 2071a1dd1..ea5b9a250 100644 --- a/test/authorization/AccessAdminManageMock.t.sol +++ b/test/authorization/AccessAdminManageMock.t.sol @@ -3,11 +3,13 @@ pragma solidity ^0.8.20; import {AccessManager} from "@openzeppelin/contracts/access/manager/AccessManager.sol"; import {IAccessManaged} from "@openzeppelin/contracts/access/manager/IAccessManaged.sol"; +import {IAccessManager} from "@openzeppelin/contracts/access/manager/IAccessManager.sol"; import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol"; import {Test, console} from "../../lib/forge-std/src/Test.sol"; import {AccessAdmin} from "../../contracts/authorization/AccessAdmin.sol"; +import {AccessAdminLib} from "../../contracts/authorization/AccessAdminLib.sol"; import {AccessAdminForTesting} from "./AccessAdmin.t.sol"; import {AccessManagedMock} from "../mock/AccessManagedMock.sol"; import {IAccess} from "../../contracts/authorization/IAccess.sol"; @@ -88,10 +90,10 @@ contract AccessAdminManageMockTest is AccessAdminBaseTest { RoleId adminRole = accessAdmin.getAdminRole(); RoleId managerRole = accessAdmin.getManagerRole(); - IAccess.FunctionInfo memory increaseCounter1 = accessAdmin.toFunction( + IAccess.FunctionInfo memory increaseCounter1 = AccessAdminLib.toFunction( AccessManagedMock.increaseCounter1.selector, "increaseCounter1"); - IAccess.FunctionInfo memory increaseCounter2 = accessAdmin.toFunction( + IAccess.FunctionInfo memory increaseCounter2 = AccessAdminLib.toFunction( AccessManagedMock.increaseCounter2.selector, "increaseCounter2"); _checkIncreaseCounter1Unauthorized(outsider, "outsider (before authorized)"); @@ -148,7 +150,7 @@ contract AccessAdminManageMockTest is AccessAdminBaseTest { AccessManager accessManager = AccessManager(accessAdmin.authority()); RoleId adminRole = accessAdmin.getAdminRole(); - IAccess.FunctionInfo memory increaseCounter1 = accessAdmin.toFunction( + IAccess.FunctionInfo memory increaseCounter1 = AccessAdminLib.toFunction( AccessManagedMock.increaseCounter1.selector, "increaseCounter1"); // WHEN + THEN @@ -227,10 +229,10 @@ contract AccessAdminManageMockTest is AccessAdminBaseTest { RoleId managerRole = accessAdmin.getManagerRole(); // grant manager role access to increaseCounter2 - IAccess.FunctionInfo memory increaseCounter1 = accessAdmin.toFunction( + IAccess.FunctionInfo memory increaseCounter1 = AccessAdminLib.toFunction( AccessManagedMock.increaseCounter1.selector, "increaseCounter1"); - IAccess.FunctionInfo memory increaseCounter2 = accessAdmin.toFunction( + IAccess.FunctionInfo memory increaseCounter2 = AccessAdminLib.toFunction( AccessManagedMock.increaseCounter2.selector, "increaseCounter2"); // WHEN @@ -246,18 +248,16 @@ contract AccessAdminManageMockTest is AccessAdminBaseTest { // check accessAdmin (with admin role) can access restricted functions regardless of granted roles // check aa deployer (with manager role) can access only granted restricted functions - address caller = accessAdminAddress; bool allowed; + (allowed,) = accessManager.canCall(accessAdminAddress, target, Selector.unwrap(increaseCounter1.selector)); + assertTrue(allowed, "accessAdminAddress (admin role) can't access (unlocked) increaseCounter1 (a)"); + (allowed,) = accessManager.canCall(accessAdminDeployer, target, Selector.unwrap(increaseCounter1.selector)); + assertFalse(allowed, "accessAdminDeployer (manager role) can't access (unlocked) increaseCounter1 (a)"); - allowed = accessAdmin.canCall(accessAdminAddress, target, increaseCounter1.selector); - assertTrue(allowed, "accessAdminAddress (admin role) can't access (unlocked) increaseCounter1"); - allowed = accessAdmin.canCall(accessAdminDeployer, target, increaseCounter1.selector); - assertFalse(allowed, "accessAdminDeployer (manager role) can't access (unlocked) increaseCounter1"); - - allowed = accessAdmin.canCall(accessAdminAddress, target, increaseCounter2.selector); - assertFalse(allowed, "accessAdminAddress (admin role) can't access (unlocked) increaseCounter2"); - allowed = accessAdmin.canCall(accessAdminDeployer, target, increaseCounter2.selector); - assertTrue(allowed, "accessAdminDeployer (manager role) can't access (unlocked) increaseCounter2"); + (allowed,) = accessManager.canCall(accessAdminAddress, target, Selector.unwrap(increaseCounter2.selector)); + assertFalse(allowed, "accessAdminAddress (admin role) can't access (unlocked) increaseCounter2 (a)"); + (allowed,) = accessManager.canCall(accessAdminDeployer, target, Selector.unwrap(increaseCounter2.selector)); + assertTrue(allowed, "accessAdminDeployer (manager role) can't access (unlocked) increaseCounter2 (a)"); // print target setup after given _printTarget(accessAdmin, target); @@ -271,15 +271,15 @@ contract AccessAdminManageMockTest is AccessAdminBaseTest { // THEN assertTrue(accessAdmin.isTargetLocked(target), "target not closed"); - allowed = accessAdmin.canCall(accessAdminAddress, target, increaseCounter1.selector); - assertFalse(allowed, "accessAdminAddress (admin role) can access (locked) increaseCounter1"); - allowed = accessAdmin.canCall(accessAdminDeployer, target, increaseCounter1.selector); - assertFalse(allowed, "accessAdminDeployer (manager role) can access (locked) increaseCounter1"); + (allowed,) = accessManager.canCall(accessAdminAddress, target, Selector.unwrap(increaseCounter1.selector)); + assertFalse(allowed, "accessAdminAddress (admin role) can access (locked) increaseCounter1 (b)"); + (allowed,) = accessManager.canCall(accessAdminDeployer, target, Selector.unwrap(increaseCounter1.selector)); + assertFalse(allowed, "accessAdminDeployer (manager role) can access (locked) increaseCounter1 (b)"); - allowed = accessAdmin.canCall(accessAdminAddress, target, increaseCounter2.selector); - assertFalse(allowed, "accessAdminAddress (admin role) can access (locked) increaseCounter2"); - allowed = accessAdmin.canCall(accessAdminDeployer, target, increaseCounter2.selector); - assertFalse(allowed, "accessAdminDeployer (manager role) can access (locked) increaseCounter2"); + (allowed,) = accessManager.canCall(accessAdminAddress, target, Selector.unwrap(increaseCounter2.selector)); + assertFalse(allowed, "accessAdminAddress (admin role) can access (locked) increaseCounter2 (b)"); + (allowed,) = accessManager.canCall(accessAdminDeployer, target, Selector.unwrap(increaseCounter2.selector)); + assertFalse(allowed, "accessAdminDeployer (manager role) can access (locked) increaseCounter2 (b)"); // WHEN - unlock mock target again vm.startPrank(accessAdminDeployer); @@ -289,15 +289,15 @@ contract AccessAdminManageMockTest is AccessAdminBaseTest { // THEN - admin must be able again to call assertFalse(accessAdmin.isTargetLocked(target), "target is closed"); - allowed = accessAdmin.canCall(accessAdminAddress, target, increaseCounter1.selector); - assertTrue(allowed, "accessAdminAddress (admin role) can't access (unlocked) increaseCounter1"); - allowed = accessAdmin.canCall(accessAdminDeployer, target, increaseCounter1.selector); - assertFalse(allowed, "accessAdminDeployer (manager role) can't access (unlocked) increaseCounter1"); + (allowed,) = accessManager.canCall(accessAdminAddress, target, Selector.unwrap(increaseCounter1.selector)); + assertTrue(allowed, "accessAdminAddress (admin role) can't access (unlocked) increaseCounter1 (c)"); + (allowed,) = accessManager.canCall(accessAdminDeployer, target, Selector.unwrap(increaseCounter1.selector)); + assertFalse(allowed, "accessAdminDeployer (manager role) can't access (unlocked) increaseCounter1 (c)"); - allowed = accessAdmin.canCall(accessAdminAddress, target, increaseCounter2.selector); - assertFalse(allowed, "accessAdminAddress (admin role) can't access (unlocked) increaseCounter2"); - allowed = accessAdmin.canCall(accessAdminDeployer, target, increaseCounter2.selector); - assertTrue(allowed, "accessAdminDeployer (manager role) can't access (unlocked) increaseCounter2"); + (allowed,) = accessManager.canCall(accessAdminAddress, target, Selector.unwrap(increaseCounter2.selector)); + assertFalse(allowed, "accessAdminAddress (admin role) can't access (unlocked) increaseCounter2 (c)"); + (allowed,) = accessManager.canCall(accessAdminDeployer, target, Selector.unwrap(increaseCounter2.selector)); + assertTrue(allowed, "accessAdminDeployer (manager role) can't access (unlocked) increaseCounter2 (c)"); } function _checkAccessAdmin( @@ -366,8 +366,6 @@ contract AccessAdminManageMockTest is AccessAdminBaseTest { RoleId missingRoleId = RoleIdLib.toRoleId(1313); assertFalse(aa.roleExists(missingRoleId), "missing role exists"); - assertFalse(aa.getRoleForName(StrLib.toStr("NoSuchRole")).exists, "NoSuchRole exists"); - // minimal check on access manager of access admin AccessManager accessManager = AccessManager(aa.authority()); bool isMember; @@ -395,11 +393,6 @@ contract AccessAdminManageMockTest is AccessAdminBaseTest { assertEq(info.adminRoleId.toInt(), expectedAdminRoleId.toInt(), "unexpected admin role (role info)"); assertEq(info.name.toString(), expectedName, "unexpected role name"); assertTrue(info.createdAt.gtz(), "role does not exist"); - - Str roleName = StrLib.toStr(expectedName); - IAccessAdmin.RoleNameInfo memory nameInfo = aa.getRoleForName(roleName); - assertTrue(nameInfo.exists, "role name info missing"); - assertEq(nameInfo.roleId.toInt(), roleId.toInt(), "unexpected role name rold id"); } function _printRoleMembers(AccessAdmin aa, RoleId roleId) internal { diff --git a/test/authorization/RegistryAdminEx.sol b/test/authorization/RegistryAdminEx.sol index 04ff51d2a..482765f68 100644 --- a/test/authorization/RegistryAdminEx.sol +++ b/test/authorization/RegistryAdminEx.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.20; import {IAuthorization} from "../../contracts/authorization/IAuthorization.sol"; import {IRegistry} from "../../contracts/registry/IRegistry.sol"; +import {AccessAdminLib} from "../../contracts/authorization/AccessAdminLib.sol"; import {AccessManagedMock} from "../mock/AccessManagedMock.sol"; import {PUBLIC_ROLE} from "../../contracts/type/RoleId.sol"; import {RegistryAdmin} from "../../contracts/registry/RegistryAdmin.sol"; @@ -36,7 +37,7 @@ contract RegistryAdminEx is RegistryAdmin { // grant permissions to public role for access managed mock FunctionInfo[] memory functions; functions = new FunctionInfo[](1); - functions[0] = toFunction(AccessManagedMock.increaseCounter1.selector, "increaseCounter1"); + functions[0] = AccessAdminLib.toFunction(AccessManagedMock.increaseCounter1.selector, "increaseCounter1"); _authorizeTargetFunctions(address(accessManagedMock), PUBLIC_ROLE(), functions); } } \ No newline at end of file diff --git a/test/base/GifDeployer.sol b/test/base/GifDeployer.sol index 724f75c8b..b9b4846df 100644 --- a/test/base/GifDeployer.sol +++ b/test/base/GifDeployer.sol @@ -155,7 +155,6 @@ contract GifDeployer is Test { // 2) deploy registry admin registryAdmin = new RegistryAdmin(); - // registryAdmin.initialize(); // 3) deploy registry registry = new Registry(registryAdmin, globalRegistry); diff --git a/test/release/ReleaseRegistryConcrete.t.sol b/test/release/ReleaseRegistryConcrete.t.sol index 047d81856..3e3cc15ee 100644 --- a/test/release/ReleaseRegistryConcrete.t.sol +++ b/test/release/ReleaseRegistryConcrete.t.sol @@ -334,7 +334,13 @@ contract ReleaseRegistryConcreteTest is GifDeployer, FoundryRandom { // prepare IServiceAuthorization nextAuthMock = new ServiceAuthorizationMockWithRegistryService(nextVersion); bytes32 nextSalt = bytes32(randomNumber(type(uint256).max)); - address nextAdmin = StdUtils.computeCreateAddress(address(releaseRegistry), vm.getNonce(address(releaseRegistry)) + 1); + address nextAdmin = _getNextContractAddress(address(releaseRegistry), 0); + + // // solhint-disable + // console.log("nextAdmin(0)", i, _getNextContractAddress(address(releaseRegistry), 0)); + // console.log("nextAdmin(1)", i, _getNextContractAddress(address(releaseRegistry), 1)); + // console.log("nextAdmin(2)", i, _getNextContractAddress(address(releaseRegistry), 2)); + // // solhint-enable vm.expectEmit(address(releaseRegistry)); emit LogReleaseCreation(IAccessAdmin(nextAdmin), nextVersion, nextSalt); @@ -391,6 +397,13 @@ contract ReleaseRegistryConcreteTest is GifDeployer, FoundryRandom { } } + function _getNextContractAddress(address deployer, uint256 nonceOffset) internal returns (address) { + // return StdUtils.computeCreateAddress( + return vm.computeCreateAddress( + deployer, + vm.getNonce(deployer) + nonceOffset); + } + function test_releaseRegistry_createRelease_whenReleaseDeployedHappyCase() public { VersionPart nextVersion = VersionPartLib.toVersionPart(releaseRegistry.INITIAL_GIF_VERSION()); @@ -1116,7 +1129,7 @@ contract ReleaseRegistryConcreteTest is GifDeployer, FoundryRandom { VersionPart expectedVersion = VersionPartLib.toVersionPart(releaseRegistry.INITIAL_GIF_VERSION()); ServiceAuthorizationMockWithRegistryService serviceAuth = new ServiceAuthorizationMockWithRegistryService(expectedVersion); bytes32 salt = "0x1234"; - address admin = StdUtils.computeCreateAddress(address(releaseRegistry), vm.getNonce(address(releaseRegistry)) + 1); + address admin = _getNextContractAddress(address(releaseRegistry), 0); vm.prank(gifAdmin); releaseRegistry.createNextRelease();