From 1df86172dd00ea2830357a162537592e25ab11e5 Mon Sep 17 00:00:00 2001 From: Dima Lekhovitsky Date: Tue, 14 Jan 2025 00:32:37 +0200 Subject: [PATCH] fix: minor fixes --- contracts/factories/AbstractFactory.sol | 8 ++++--- contracts/factories/CreditFactory.sol | 12 ++++++++-- .../factories/InterestRateModelFactory.sol | 2 +- contracts/factories/PoolFactory.sol | 4 +--- contracts/factories/PriceOracleFactory.sol | 3 +-- contracts/instance/InstanceManager.sol | 12 +++++----- contracts/interfaces/factories/IFactory.sol | 6 +++++ contracts/test/helpers/GlobalSetup.sol | 22 ++++++++++--------- .../test/helpers/InstanceManagerHelper.sol | 11 +++++----- contracts/test/suite/NewChainDeploySuite.sol | 8 ++++--- 10 files changed, 52 insertions(+), 36 deletions(-) diff --git a/contracts/factories/AbstractFactory.sol b/contracts/factories/AbstractFactory.sol index e32eae2..5e8cae7 100644 --- a/contracts/factories/AbstractFactory.sol +++ b/contracts/factories/AbstractFactory.sol @@ -13,12 +13,16 @@ import {AP_MARKET_CONFIGURATOR_FACTORY, NO_VERSION_CONTROL} from "../libraries/C import {DeployerTrait} from "../traits/DeployerTrait.sol"; abstract contract AbstractFactory is DeployerTrait, IFactory { + address public immutable override marketConfiguratorFactory; + modifier onlyMarketConfigurators() { _ensureCallerIsMarketConfigurator(); _; } - constructor(address addressProvider_) DeployerTrait(addressProvider_) {} + constructor(address addressProvider_) DeployerTrait(addressProvider_) { + marketConfiguratorFactory = _getAddressOrRevert(AP_MARKET_CONFIGURATOR_FACTORY, NO_VERSION_CONTROL); + } // ------------- // // CONFIGURATION // @@ -37,8 +41,6 @@ abstract contract AbstractFactory is DeployerTrait, IFactory { // --------- // function _ensureCallerIsMarketConfigurator() internal view { - // QUESTION: can MCF be upgraded? - address marketConfiguratorFactory = _getAddressOrRevert(AP_MARKET_CONFIGURATOR_FACTORY, NO_VERSION_CONTROL); if (!IMarketConfiguratorFactory(marketConfiguratorFactory).isMarketConfigurator(msg.sender)) { revert CallerIsNotMarketConfiguratorException(msg.sender); } diff --git a/contracts/factories/CreditFactory.sol b/contracts/factories/CreditFactory.sol index ca8547b..77f48c1 100644 --- a/contracts/factories/CreditFactory.sol +++ b/contracts/factories/CreditFactory.sol @@ -353,8 +353,16 @@ contract CreditFactory is AbstractFactory, ICreditFactory { if (decodedCreditManager != creditManager) revert InvalidConstructorParamsException(); - // FIXME: unlike other contracts, this might be deployed multiple times, so using the same salt - // can be an issue. Same thing can happen to rate keepers, IRMs, etc. + // NOTE: allowing a previously forbidden adapter is considered a valid operation, + // so we just return the contract address if it's already deployed + address adapter = _computeAddressLatestPatch({ + contractType: _getContractType(DOMAIN_ADAPTER, params.postfix), + minorVersion: version, + constructorParams: params.constructorParams, + salt: bytes32(bytes20(marketConfigurator)), + deployer: address(this) + }); + if (adapter.code.length != 0) return adapter; return _deployLatestPatch({ contractType: _getContractType(DOMAIN_ADAPTER, params.postfix), diff --git a/contracts/factories/InterestRateModelFactory.sol b/contracts/factories/InterestRateModelFactory.sol index ce04344..fba9ecc 100644 --- a/contracts/factories/InterestRateModelFactory.sol +++ b/contracts/factories/InterestRateModelFactory.sol @@ -43,7 +43,7 @@ contract InterestRateModelFactory is AbstractMarketFactory, IInterestRateModelFa contractType: _getContractType(DOMAIN_IRM, params.postfix), minorVersion: version, constructorParams: params.constructorParams, - salt: bytes32(bytes20(msg.sender)) + salt: bytes32(bytes20(pool)) }); return DeployResult({ diff --git a/contracts/factories/PoolFactory.sol b/contracts/factories/PoolFactory.sol index 5e10056..d5a801d 100644 --- a/contracts/factories/PoolFactory.sol +++ b/contracts/factories/PoolFactory.sol @@ -21,9 +21,7 @@ import {IMarketConfigurator} from "../interfaces/IMarketConfigurator.sol"; import {Call, DeployResult} from "../interfaces/Types.sol"; import {CallBuilder} from "../libraries/CallBuilder.sol"; -import { - AP_POOL_FACTORY, AP_POOL_QUOTA_KEEPER, DOMAIN_POOL, NO_VERSION_CONTROL -} from "../libraries/ContractLiterals.sol"; +import {AP_POOL_FACTORY, AP_POOL_QUOTA_KEEPER, DOMAIN_POOL} from "../libraries/ContractLiterals.sol"; import {AbstractFactory} from "./AbstractFactory.sol"; import {AbstractMarketFactory} from "./AbstractMarketFactory.sol"; diff --git a/contracts/factories/PriceOracleFactory.sol b/contracts/factories/PriceOracleFactory.sol index 9132770..492094d 100644 --- a/contracts/factories/PriceOracleFactory.sol +++ b/contracts/factories/PriceOracleFactory.sol @@ -61,8 +61,7 @@ contract PriceOracleFactory is AbstractMarketFactory, IPriceOracleFactory { /// @notice Constructor /// @param addressProvider_ Address provider contract address constructor(address addressProvider_) AbstractFactory(addressProvider_) { - // TODO: Dima check the version problem - priceFeedStore = _getAddressOrRevert(AP_PRICE_FEED_STORE, 3_10); + priceFeedStore = _getAddressOrRevert(AP_PRICE_FEED_STORE, NO_VERSION_CONTROL); } // ---------- // diff --git a/contracts/instance/InstanceManager.sol b/contracts/instance/InstanceManager.sol index 26067d7..562b343 100644 --- a/contracts/instance/InstanceManager.sol +++ b/contracts/instance/InstanceManager.sol @@ -88,17 +88,15 @@ contract InstanceManager is Ownable, IVersion { } } - function deploySystemContract(bytes32 _contractName, uint256 _version) external onlyCrossChainGovernance { + function deploySystemContract(bytes32 _contractName, uint256 _version, bool _saveVersion) + external + onlyCrossChainGovernance + { // deploy contract // set address in address provider address newSystemContract = _deploySystemContract(_contractName, _version); - if (_contractName == AP_MARKET_CONFIGURATOR_FACTORY) { - _setAddress(_contractName, newSystemContract, false); - _setAddress(_contractName, newSystemContract, true); - } else { - _setAddress(_contractName, newSystemContract, true); - } + _setAddress(_contractName, newSystemContract, _saveVersion); } function _deploySystemContract(bytes32 _contractName, uint256 _version) internal returns (address) { diff --git a/contracts/interfaces/factories/IFactory.sol b/contracts/interfaces/factories/IFactory.sol index 63781a4..88b1c62 100644 --- a/contracts/interfaces/factories/IFactory.sol +++ b/contracts/interfaces/factories/IFactory.sol @@ -17,6 +17,12 @@ interface IFactory is IVersion, IDeployerTrait { error ForbiddenEmergencyConfigurationCallException(bytes4 selector); error InvalidConstructorParamsException(); + // --------------- // + // STATE VARIABLES // + // --------------- // + + function marketConfiguratorFactory() external view returns (address); + // ------------- // // CONFIGURATION // // ------------- // diff --git a/contracts/test/helpers/GlobalSetup.sol b/contracts/test/helpers/GlobalSetup.sol index 3e3fd37..a4ba449 100644 --- a/contracts/test/helpers/GlobalSetup.sol +++ b/contracts/test/helpers/GlobalSetup.sol @@ -121,6 +121,7 @@ struct UploadableContract { struct DeploySystemContractCall { bytes32 contractType; uint256 version; + bool saveVersion; } // It deploys all the system contracts and related ones @@ -141,14 +142,14 @@ contract GlobalSetup is Test, InstanceManagerHelper { _submitProposalAndSign(calls); DeploySystemContractCall[8] memory deployCalls = [ - DeploySystemContractCall({contractType: AP_PRICE_FEED_STORE, version: 3_10}), - DeploySystemContractCall({contractType: AP_POOL_FACTORY, version: 3_10}), - DeploySystemContractCall({contractType: AP_CREDIT_FACTORY, version: 3_10}), - DeploySystemContractCall({contractType: AP_PRICE_ORACLE_FACTORY, version: 3_10}), - DeploySystemContractCall({contractType: AP_INTEREST_RATE_MODEL_FACTORY, version: 3_10}), - DeploySystemContractCall({contractType: AP_RATE_KEEPER_FACTORY, version: 3_10}), - DeploySystemContractCall({contractType: AP_LOSS_POLICY_FACTORY, version: 3_10}), - DeploySystemContractCall({contractType: AP_MARKET_CONFIGURATOR_FACTORY, version: 3_10}) + DeploySystemContractCall({contractType: AP_PRICE_FEED_STORE, version: 3_10, saveVersion: false}), + DeploySystemContractCall({contractType: AP_MARKET_CONFIGURATOR_FACTORY, version: 3_10, saveVersion: false}), + DeploySystemContractCall({contractType: AP_POOL_FACTORY, version: 3_10, saveVersion: true}), + DeploySystemContractCall({contractType: AP_CREDIT_FACTORY, version: 3_10, saveVersion: true}), + DeploySystemContractCall({contractType: AP_PRICE_ORACLE_FACTORY, version: 3_10, saveVersion: true}), + DeploySystemContractCall({contractType: AP_INTEREST_RATE_MODEL_FACTORY, version: 3_10, saveVersion: true}), + DeploySystemContractCall({contractType: AP_RATE_KEEPER_FACTORY, version: 3_10, saveVersion: true}), + DeploySystemContractCall({contractType: AP_LOSS_POLICY_FACTORY, version: 3_10, saveVersion: true}) ]; uint256 uploadContractsLen = contractsToUpload.length; @@ -164,8 +165,9 @@ contract GlobalSetup is Test, InstanceManagerHelper { } for (uint256 i = 0; i < deploySystemContractsLen; ++i) { - calls[uploadContractsLen + i] = - _generateDeploySystemContractCall(deployCalls[i].contractType, deployCalls[i].version); + calls[uploadContractsLen + i] = _generateDeploySystemContractCall( + deployCalls[i].contractType, deployCalls[i].version, deployCalls[i].saveVersion + ); } _submitProposalAndSign(calls); diff --git a/contracts/test/helpers/InstanceManagerHelper.sol b/contracts/test/helpers/InstanceManagerHelper.sol index 3b02bb8..09aee09 100644 --- a/contracts/test/helpers/InstanceManagerHelper.sol +++ b/contracts/test/helpers/InstanceManagerHelper.sol @@ -11,7 +11,8 @@ import {BytecodeRepository} from "../../../contracts/global/BytecodeRepository.s import { AP_INSTANCE_MANAGER, AP_BYTECODE_REPOSITORY, - AP_PRICE_FEED_STORE + AP_PRICE_FEED_STORE, + NO_VERSION_CONTROL } from "../../../contracts/libraries/ContractLiterals.sol"; import {CrossChainCall} from "../../../contracts/interfaces/ICrossChainMultisig.sol"; import {IBytecodeRepository} from "../../../contracts/interfaces/IBytecodeRepository.sol"; @@ -47,14 +48,14 @@ contract InstanceManagerHelper is BCRHelpers, CCGHelper { ); } - function _generateDeploySystemContractCall(bytes32 _contractName, uint256 _version) + function _generateDeploySystemContractCall(bytes32 _contractName, uint256 _version, bool _saveVersion) internal returns (CrossChainCall memory) { return CrossChainCall({ chainId: 0, target: address(instanceManager), - callData: abi.encodeCall(InstanceManager.deploySystemContract, (_contractName, _version)) + callData: abi.encodeCall(InstanceManager.deploySystemContract, (_contractName, _version, _saveVersion)) }); } @@ -86,7 +87,7 @@ contract InstanceManagerHelper is BCRHelpers, CCGHelper { function _allowPriceFeed(address token, address _priceFeed) internal { address ap = instanceManager.addressProvider(); - address priceFeedStore = IAddressProvider(ap).getAddressOrRevert(AP_PRICE_FEED_STORE, 3_10); + address priceFeedStore = IAddressProvider(ap).getAddressOrRevert(AP_PRICE_FEED_STORE, NO_VERSION_CONTROL); vm.prank(instanceOwner); instanceManager.configureLocal( priceFeedStore, abi.encodeCall(IPriceFeedStore.allowPriceFeed, (token, _priceFeed)) @@ -95,7 +96,7 @@ contract InstanceManagerHelper is BCRHelpers, CCGHelper { function _addPriceFeed(address _priceFeed, uint32 _stalenessPeriod) internal { address ap = instanceManager.addressProvider(); - address priceFeedStore = IAddressProvider(ap).getAddressOrRevert(AP_PRICE_FEED_STORE, 3_10); + address priceFeedStore = IAddressProvider(ap).getAddressOrRevert(AP_PRICE_FEED_STORE, NO_VERSION_CONTROL); vm.prank(instanceOwner); instanceManager.configureLocal( priceFeedStore, abi.encodeCall(IPriceFeedStore.addPriceFeed, (_priceFeed, _stalenessPeriod)) diff --git a/contracts/test/suite/NewChainDeploySuite.sol b/contracts/test/suite/NewChainDeploySuite.sol index 435bca1..d4fe463 100644 --- a/contracts/test/suite/NewChainDeploySuite.sol +++ b/contracts/test/suite/NewChainDeploySuite.sol @@ -36,7 +36,8 @@ import { AP_LOSS_POLICY_DEFAULT, AP_CREDIT_MANAGER, AP_CREDIT_FACADE, - AP_CREDIT_CONFIGURATOR + AP_CREDIT_CONFIGURATOR, + NO_VERSION_CONTROL } from "../../libraries/ContractLiterals.sol"; import {SignedProposal, Bytecode} from "../../interfaces/Types.sol"; @@ -72,6 +73,7 @@ import {GlobalSetup} from "../../test/helpers/GlobalSetup.sol"; contract NewChainDeploySuite is Test, GlobalSetup { address internal riskCurator; + address constant TREASURY = 0x3E965117A51186e41c2BB58b729A1e518A715e5F; address constant WETH = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2; address constant GEAR = 0xBa3335588D9403515223F109EdC4eB7269a9Ab5D; address constant USDC = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48; @@ -88,7 +90,7 @@ contract NewChainDeploySuite is Test, GlobalSetup { // activate instance CrossChainCall[] memory calls = new CrossChainCall[](1); - calls[0] = _generateActivateCall(1, instanceOwner, address(0), WETH, GEAR); + calls[0] = _generateActivateCall(1, instanceOwner, TREASURY, WETH, GEAR); _submitProposalAndSign(calls); // Configure instance @@ -107,7 +109,7 @@ contract NewChainDeploySuite is Test, GlobalSetup { function test_NCD_01_createMarket() public { address ap = instanceManager.addressProvider(); - address mcf = IAddressProvider(ap).getAddressOrRevert(AP_MARKET_CONFIGURATOR_FACTORY, 3_10); + address mcf = IAddressProvider(ap).getAddressOrRevert(AP_MARKET_CONFIGURATOR_FACTORY, NO_VERSION_CONTROL); address poolFactory = IAddressProvider(ap).getAddressOrRevert(AP_POOL_FACTORY, 3_10);