From 693bc10f7b13dc6257ed84429c8383de2304431e Mon Sep 17 00:00:00 2001 From: Artem Chystiakov Date: Mon, 30 Oct 2023 19:35:44 +0200 Subject: [PATCH] updated integration --- contracts/core/ContractsRegistry.sol | 14 ++++++++ contracts/factory/PoolRegistry.sol | 16 +++++++++- .../interfaces/core/IContractsRegistry.sol | 4 --- .../interfaces/factory/IPoolRegistry.sol | 4 --- docs/core/IContractsRegistry.md | 15 --------- docs/factory/IPoolRegistry.md | 15 --------- test/core/ContractsRegistry.test.js | 27 ++++++++++++---- test/factory/PoolFactory.test.js | 32 ++++++++++++++++--- test/factory/PoolRegistry.test.js | 24 ++++++++++---- 9 files changed, 94 insertions(+), 57 deletions(-) diff --git a/contracts/core/ContractsRegistry.sol b/contracts/core/ContractsRegistry.sol index a29a74b6..4bbc6424 100644 --- a/contracts/core/ContractsRegistry.sol +++ b/contracts/core/ContractsRegistry.sol @@ -44,6 +44,20 @@ contract ContractsRegistry is IContractsRegistry, OwnableContractsRegistry, UUPS _setSphereXEngine(CORE_PROPERTIES_NAME, sphereXEngine); } + function protectContractFunctions( + string calldata contractName, + bytes4[] calldata selectors + ) external onlyOwner { + SphereXProxyBase(getContract(contractName)).addProtectedFuncSigs(selectors); + } + + function unprotectContractFunctions( + string calldata contractName, + bytes4[] calldata selectors + ) external onlyOwner { + SphereXProxyBase(getContract(contractName)).removeProtectedFuncSigs(selectors); + } + function getUserRegistryContract() external view override returns (address) { return getContract(USER_REGISTRY_NAME); } diff --git a/contracts/factory/PoolRegistry.sol b/contracts/factory/PoolRegistry.sol index 7537d9b2..db3d5d5d 100644 --- a/contracts/factory/PoolRegistry.sol +++ b/contracts/factory/PoolRegistry.sol @@ -52,7 +52,7 @@ contract PoolRegistry is IPoolRegistry, OwnablePoolContractsRegistry { _addProxyPool(name, poolAddress); } - function toggleSphereXEngine(bool on) external override onlyOwner { + function toggleSphereXEngine(bool on) external onlyOwner { address sphereXEngine = on ? _poolSphereXEngine : address(0); _setSphereXEngine(GOV_POOL_NAME, sphereXEngine); @@ -67,6 +67,20 @@ contract PoolRegistry is IPoolRegistry, OwnablePoolContractsRegistry { _setSphereXEngine(POLYNOMIAL_POWER_NAME, sphereXEngine); } + function protectPoolFunctions( + string calldata poolName, + bytes4[] calldata selectors + ) external onlyOwner { + SphereXProxyBase(getProxyBeacon(poolName)).addProtectedFuncSigs(selectors); + } + + function unprotectPoolFunctions( + string calldata poolName, + bytes4[] calldata selectors + ) external onlyOwner { + SphereXProxyBase(getProxyBeacon(poolName)).removeProtectedFuncSigs(selectors); + } + function isGovPool(address potentialPool) external view override returns (bool) { return isPool(GOV_POOL_NAME, potentialPool); } diff --git a/contracts/interfaces/core/IContractsRegistry.sol b/contracts/interfaces/core/IContractsRegistry.sol index 594c79a3..7f6e7a65 100644 --- a/contracts/interfaces/core/IContractsRegistry.sol +++ b/contracts/interfaces/core/IContractsRegistry.sol @@ -7,10 +7,6 @@ pragma solidity ^0.8.20; * contracts, provide upgradeability mechanism and dependency injection mechanism. */ interface IContractsRegistry { - /// @notice The function to toggle the SphereX engine for all the contracts handled by the registry - /// @param on whether to turn the engine on or off - function toggleSphereXEngine(bool on) external; - /// @notice Used in dependency injection mechanism /// @return UserRegistry contract address function getUserRegistryContract() external view returns (address); diff --git a/contracts/interfaces/factory/IPoolRegistry.sol b/contracts/interfaces/factory/IPoolRegistry.sol index 477fde5b..d30f927c 100644 --- a/contracts/interfaces/factory/IPoolRegistry.sol +++ b/contracts/interfaces/factory/IPoolRegistry.sol @@ -12,10 +12,6 @@ interface IPoolRegistry { /// @param poolAddress the address of the pool to add function addProxyPool(string calldata name, address poolAddress) external; - /// @notice The function to toggle the SphereX engine to all the contracts handled by the registry - /// @param on whether to turn the engine on or off - function toggleSphereXEngine(bool on) external; - /// @notice The function to check if the given address is a valid GovPool /// @param potentialPool the address to inspect /// @return true if the address is a GovPool, false otherwise diff --git a/docs/core/IContractsRegistry.md b/docs/core/IContractsRegistry.md index cbc42a43..6c59263c 100644 --- a/docs/core/IContractsRegistry.md +++ b/docs/core/IContractsRegistry.md @@ -16,21 +16,6 @@ the other contracts used by the protocol. Its purpose is to keep track of the pr contracts, provide upgradeability mechanism and dependency injection mechanism. ## Functions info -### toggleSphereXEngine (0x419aa023) - -```solidity -function toggleSphereXEngine(bool on) external -``` - -The function to toggle the SphereX engine for all the contracts handled by the registry - - -Parameters: - -| Name | Type | Description | -| :--- | :--- | :----------------------------------- | -| on | bool | whether to turn the engine on or off | - ### getUserRegistryContract (0x435403b4) ```solidity diff --git a/docs/factory/IPoolRegistry.md b/docs/factory/IPoolRegistry.md index 78f16ce8..3885ce57 100644 --- a/docs/factory/IPoolRegistry.md +++ b/docs/factory/IPoolRegistry.md @@ -32,21 +32,6 @@ Parameters: | name | string | the type of the pool | | poolAddress | address | the address of the pool to add | -### toggleSphereXEngine (0x419aa023) - -```solidity -function toggleSphereXEngine(bool on) external -``` - -The function to toggle the SphereX engine to all the contracts handled by the registry - - -Parameters: - -| Name | Type | Description | -| :--- | :--- | :----------------------------------- | -| on | bool | whether to turn the engine on or off | - ### isGovPool (0x9e475551) ```solidity diff --git a/test/core/ContractsRegistry.test.js b/test/core/ContractsRegistry.test.js index 3f171e7b..0398e0e4 100644 --- a/test/core/ContractsRegistry.test.js +++ b/test/core/ContractsRegistry.test.js @@ -2,14 +2,12 @@ const { assert } = require("chai"); const { toBN, accounts } = require("../../scripts/utils/utils"); const Reverter = require("../helpers/reverter"); const truffleAssert = require("truffle-assertions"); -const { ZERO_ADDR } = require("../../scripts/utils/constants"); const { impersonate } = require("../helpers/impersonator"); const ContractsRegistry = artifacts.require("ContractsRegistry"); const ERC1967Proxy = artifacts.require("ERC1967Proxy"); const ERC20Mock = artifacts.require("ERC20Mock"); const ERC20MockUpgraded = artifacts.require("ERC20MockUpgraded"); -const ProtectedTransparentProxy = artifacts.require("ProtectedTransparentProxy"); const SphereXEngineMock = artifacts.require("SphereXEngineMock"); const SphereXCalleeMock = artifacts.require("SphereXCalleeMock"); @@ -177,8 +175,8 @@ describe("ContractsRegistry", () => { describe("SphereX", () => { let sphereXCalleeProxy; - let transparentProxy; let protectedMethodSelector; + let NAME; beforeEach(async () => { protectedMethodSelector = web3.eth.abi.encodeFunctionSignature("protectedMethod()"); @@ -192,21 +190,26 @@ describe("ContractsRegistry", () => { await contractsRegistry.addProxyContract(await contractsRegistry.PRICE_FEED_NAME(), sphereXCallee.address); await contractsRegistry.addProxyContract(await contractsRegistry.CORE_PROPERTIES_NAME(), sphereXCallee.address); + NAME = await contractsRegistry.USER_REGISTRY_NAME(); + sphereXCalleeProxy = await SphereXCalleeMock.at(await contractsRegistry.getUserRegistryContract()); - transparentProxy = await ProtectedTransparentProxy.at(sphereXCalleeProxy.address); await impersonate(contractsRegistry.address); }); it("should protect when sphereXEngine and selector are on", async () => { await contractsRegistry.toggleSphereXEngine(true); - await transparentProxy.addProtectedFuncSigs([protectedMethodSelector], { from: contractsRegistry.address }); + await contractsRegistry.protectContractFunctions(NAME, [protectedMethodSelector]); await truffleAssert.passes(sphereXCalleeProxy.protectedMethod()); await sphereXEngine.toggleRevert(); await truffleAssert.reverts(sphereXCalleeProxy.protectedMethod(), "SphereXEngineMock: malicious tx"); + + await contractsRegistry.unprotectContractFunctions(NAME, [protectedMethodSelector]); + + await sphereXCalleeProxy.protectedMethod(); }); it("should not protect when selector is off", async () => { @@ -220,18 +223,28 @@ describe("ContractsRegistry", () => { it("should not protect when sphereXEngine is off", async () => { await contractsRegistry.toggleSphereXEngine(true); await contractsRegistry.toggleSphereXEngine(false); - await transparentProxy.addProtectedFuncSigs([protectedMethodSelector], { from: contractsRegistry.address }); + await contractsRegistry.protectContractFunctions(NAME, [protectedMethodSelector]); await sphereXEngine.toggleRevert(); await truffleAssert.passes(sphereXCalleeProxy.protectedMethod()); }); - it("should not set engine if not an operator", async () => { + it("should not work with engine if not an owner", async () => { await truffleAssert.reverts( contractsRegistry.toggleSphereXEngine(true, { from: SECOND }), "Ownable: caller is not the owner" ); + + await truffleAssert.reverts( + contractsRegistry.protectContractFunctions(NAME, [protectedMethodSelector], { from: SECOND }), + "Ownable: caller is not the owner" + ); + + await truffleAssert.reverts( + contractsRegistry.unprotectContractFunctions(NAME, [protectedMethodSelector], { from: SECOND }), + "Ownable: caller is not the owner" + ); }); }); }); diff --git a/test/factory/PoolFactory.test.js b/test/factory/PoolFactory.test.js index 06072e22..853fbb4c 100644 --- a/test/factory/PoolFactory.test.js +++ b/test/factory/PoolFactory.test.js @@ -48,7 +48,7 @@ const TokenSaleProposalRecoverLib = artifacts.require("TokenSaleProposalRecover" const GovValidatorsCreateLib = artifacts.require("GovValidatorsCreate"); const GovValidatorsVoteLib = artifacts.require("GovValidatorsVote"); const GovValidatorsExecuteLib = artifacts.require("GovValidatorsExecute"); -const SphereXEngineMock = artifacts.require("SphereXEngineMock"); +const SphereXEngine = artifacts.require("SphereXEngine"); ContractsRegistry.numberFormat = "BigNumber"; ERC20Mock.numberFormat = "BigNumber"; @@ -78,6 +78,7 @@ describe("PoolFactory", () => { let testERC20; let testERC721; let babt; + let sphereXEngine; const reverter = new Reverter(); @@ -153,12 +154,14 @@ describe("PoolFactory", () => { const _poolRegistry = await PoolRegistry.new(); const _poolFactory = await PoolFactory.new(); const uniswapV2Router = await UniswapV2RouterMock.new(); - const _sphereXEngine = await SphereXEngineMock.new(); + sphereXEngine = await SphereXEngine.new(0, OWNER); await contractsRegistry.__OwnableContractsRegistry_init(); - await contractsRegistry.addContract(await contractsRegistry.SPHEREX_ENGINE_NAME(), _sphereXEngine.address); - await contractsRegistry.addContract(await contractsRegistry.POOL_SPHEREX_ENGINE_NAME(), _sphereXEngine.address); + await sphereXEngine.grantRole(await sphereXEngine.SENDER_ADDER_ROLE(), contractsRegistry.address); + + await contractsRegistry.addContract(await contractsRegistry.SPHEREX_ENGINE_NAME(), sphereXEngine.address); + await contractsRegistry.addContract(await contractsRegistry.POOL_SPHEREX_ENGINE_NAME(), sphereXEngine.address); await contractsRegistry.addProxyContract(await contractsRegistry.CORE_PROPERTIES_NAME(), _coreProperties.address); await contractsRegistry.addProxyContract(await contractsRegistry.PRICE_FEED_NAME(), _priceFeed.address); @@ -179,13 +182,14 @@ describe("PoolFactory", () => { poolFactory = await PoolFactory.at(await contractsRegistry.getPoolFactoryContract()); const priceFeed = await PriceFeed.at(await contractsRegistry.getPriceFeedContract()); + await sphereXEngine.grantRole(await sphereXEngine.SENDER_ADDER_ROLE(), poolFactory.address); + await priceFeed.__PriceFeed_init(); await poolRegistry.__OwnablePoolContractsRegistry_init(); await coreProperties.__CoreProperties_init(DEFAULT_CORE_PROPERTIES); await contractsRegistry.injectDependencies(await contractsRegistry.POOL_FACTORY_NAME()); await contractsRegistry.injectDependencies(await contractsRegistry.POOL_REGISTRY_NAME()); - await contractsRegistry.injectDependencies(await contractsRegistry.POOL_REGISTRY_NAME()); await contractsRegistry.injectDependencies(await contractsRegistry.CORE_PROPERTIES_NAME()); await contractsRegistry.injectDependencies(await contractsRegistry.PRICE_FEED_NAME()); @@ -466,6 +470,24 @@ describe("PoolFactory", () => { assert.equal(await votePower.transformVotes(ZERO_ADDR, 2), 2); }); + it("should revert with protection on and deploy with off", async () => { + let POOL_PARAMETERS = getGovPoolSaleConfiguredParams(); + const predictedGovAddresses = await poolFactory.predictGovAddresses(OWNER, POOL_PARAMETERS.name); + + await sphereXEngine.configureRules("0x0000000000000001"); + await poolRegistry.protectPoolFunctions(await poolRegistry.TOKEN_SALE_PROPOSAL_NAME(), ["0x69130451"]); + await poolRegistry.toggleSphereXEngine(true); + + POOL_PARAMETERS.tokenSaleParams.tiersParams.pop(); + POOL_PARAMETERS.settingsParams.additionalProposalExecutors[0] = predictedGovAddresses.govTokenSale; + + await truffleAssert.reverts(poolFactory.deployGovPool(POOL_PARAMETERS), "SphereX error: disallowed tx pattern"); + + await poolRegistry.unprotectPoolFunctions(await poolRegistry.TOKEN_SALE_PROPOSAL_NAME(), ["0x69130451"]); + + await poolFactory.deployGovPool(POOL_PARAMETERS); + }); + it("should deploy pool with empty token sale", async () => { let POOL_PARAMETERS = getGovPoolSaleConfiguredParams(); diff --git a/test/factory/PoolRegistry.test.js b/test/factory/PoolRegistry.test.js index 1b1b0346..82ea23af 100644 --- a/test/factory/PoolRegistry.test.js +++ b/test/factory/PoolRegistry.test.js @@ -4,7 +4,6 @@ const Reverter = require("../helpers/reverter"); const truffleAssert = require("truffle-assertions"); const { DEFAULT_CORE_PROPERTIES } = require("../utils/constants"); const { impersonate } = require("../helpers/impersonator"); -const { ZERO_ADDR } = require("../../scripts/utils/constants"); const ContractsRegistry = artifacts.require("ContractsRegistry"); const PoolRegistry = artifacts.require("PoolRegistry"); @@ -122,7 +121,6 @@ describe("PoolRegistry", () => { describe("SphereX", () => { let sphereXCalleeProxy; - let poolBeaconProxy; let protectedPublicBeaconProxy; let protectedMethodSelector; @@ -156,7 +154,7 @@ describe("PoolRegistry", () => { ] ); - poolBeaconProxy = await PoolBeacon.at(await poolRegistry.getProxyBeacon(GOV_NAME)); + const poolBeaconProxy = await PoolBeacon.at(await poolRegistry.getProxyBeacon(GOV_NAME)); protectedPublicBeaconProxy = await ProtectedPublicBeaconProxy.new(poolBeaconProxy.address, "0x"); sphereXCalleeProxy = await SphereXCalleeMock.at(protectedPublicBeaconProxy.address); @@ -166,13 +164,17 @@ describe("PoolRegistry", () => { it("should protect when sphereXEngine and selector are on", async () => { await poolRegistry.toggleSphereXEngine(true); - await poolBeaconProxy.addProtectedFuncSigs([protectedMethodSelector], { from: poolRegistry.address }); + await poolRegistry.protectPoolFunctions(GOV_NAME, [protectedMethodSelector]); await truffleAssert.passes(sphereXCalleeProxy.protectedMethod()); await sphereXEngine.toggleRevert(); await truffleAssert.reverts(sphereXCalleeProxy.protectedMethod(), "SphereXEngineMock: malicious tx"); + + await poolRegistry.unprotectPoolFunctions(GOV_NAME, [protectedMethodSelector]); + + await sphereXCalleeProxy.protectedMethod(); }); it("should not protect when selector is off", async () => { @@ -187,18 +189,28 @@ describe("PoolRegistry", () => { await poolRegistry.toggleSphereXEngine(true); await poolRegistry.toggleSphereXEngine(false); - await poolBeaconProxy.addProtectedFuncSigs([protectedMethodSelector], { from: poolRegistry.address }); + await poolRegistry.protectPoolFunctions(GOV_NAME, [protectedMethodSelector]); await sphereXEngine.toggleRevert(); await truffleAssert.passes(sphereXCalleeProxy.protectedMethod()); }); - it("should not set engine if not an operator", async () => { + it("should not work with engine if not an operator", async () => { await truffleAssert.reverts( poolRegistry.toggleSphereXEngine(true, { from: SECOND }), "Ownable: caller is not the owner" ); + + await truffleAssert.reverts( + poolRegistry.protectPoolFunctions(GOV_NAME, [protectedMethodSelector], { from: SECOND }), + "Ownable: caller is not the owner" + ); + + await truffleAssert.reverts( + poolRegistry.unprotectPoolFunctions(GOV_NAME, [protectedMethodSelector], { from: SECOND }), + "Ownable: caller is not the owner" + ); }); it("should return correct implementation", async () => {