From d86682533fbefb8e0197da1143edd836cf54ebd3 Mon Sep 17 00:00:00 2001 From: Roberto Cantu Date: Thu, 6 Apr 2023 09:25:58 -0500 Subject: [PATCH 1/6] Begin implementation of chain settings contract --- contracts/components/Roles.sol | 1 + .../chain_settings/ChainSettingsRegistry.sol | 42 +++++++++++++++ .../ChainSettingsRegistryCore.sol | 53 +++++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100644 contracts/components/chain_settings/ChainSettingsRegistry.sol create mode 100644 contracts/components/chain_settings/ChainSettingsRegistryCore.sol diff --git a/contracts/components/Roles.sol b/contracts/components/Roles.sol index 8447f4d0..4121bef8 100644 --- a/contracts/components/Roles.sol +++ b/contracts/components/Roles.sol @@ -20,6 +20,7 @@ bytes32 constant SCANNER_POOL_ADMIN_ROLE = keccak256("SCANNER_POOL_ADMIN_ROLE"); bytes32 constant SCANNER_2_SCANNER_POOL_MIGRATOR_ROLE = keccak256("SCANNER_2_SCANNER_POOL_MIGRATOR_ROLE"); bytes32 constant DISPATCHER_ROLE = keccak256("DISPATCHER_ROLE"); bytes32 constant MIGRATION_EXECUTOR_ROLE = keccak256("MIGRATION_EXECUTOR_ROLE"); +bytes32 constant CHAIN_SETTINGS_ROLE = keccak256("CHAIN_SETTINGS_ROLE"); // Staking bytes32 constant SLASHER_ROLE = keccak256("SLASHER_ROLE"); diff --git a/contracts/components/chain_settings/ChainSettingsRegistry.sol b/contracts/components/chain_settings/ChainSettingsRegistry.sol new file mode 100644 index 00000000..f20e057c --- /dev/null +++ b/contracts/components/chain_settings/ChainSettingsRegistry.sol @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: UNLICENSED +// See Forta Network License: https://github.com/forta-network/forta-contracts/blob/master/LICENSE.md + +pragma solidity ^0.8.9; + +import "../BaseComponentUpgradeable.sol"; +import "./ChainSettingsRegistryCore.sol"; + +contract ChainSettingsRegistry is BaseComponentUpgradeable, ChainSettingsRegistryCore { + string public constant version = "0.1.0"; + + /// @custom:oz-upgrades-unsafe-allow constructor + constructor(address forwarder) initializer ForwardedContext(forwarder) {} + + /** + * @notice Initializer method, access point to initialize inheritance tree. + * @param __manager address of AccessManager. + */ + function initialize( + address __manager + ) public initializer { + __BaseComponentUpgradeable_init(__manager); + } + + /** + * @notice Helper to get either msg msg.sender if not a meta transaction, signer of forwarder metatx if it is. + * @inheritdoc ForwardedContext + */ + function _msgSender() internal view virtual override(BaseComponentUpgradeable) returns (address sender) { + return super._msgSender(); + } + + /** + * @notice Helper to get msg.data if not a meta transaction, forwarder data in metatx if it is. + * @inheritdoc ForwardedContext + */ + function _msgData() internal view virtual override(BaseComponentUpgradeable) returns (bytes calldata) { + return super._msgData(); + } + + uint256[50] private __gap; +} diff --git a/contracts/components/chain_settings/ChainSettingsRegistryCore.sol b/contracts/components/chain_settings/ChainSettingsRegistryCore.sol new file mode 100644 index 00000000..9712f8fa --- /dev/null +++ b/contracts/components/chain_settings/ChainSettingsRegistryCore.sol @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: UNLICENSED +// See Forta Network License: https://github.com/forta-network/forta-contracts/blob/master/LICENSE.md + +pragma solidity ^0.8.9; + +import "../BaseComponentUpgradeable.sol"; + +abstract contract ChainSettingsRegistryCore is BaseComponentUpgradeable { + + mapping(uint256 => string) private _chainIdToChainSettingsMetadata; + mapping(bytes32 => bool) private _chainSettingstMetadataUniqueness; + + error MetadataNotUnique(bytes32 hash); + + event ChainSettingsUpdated(uint256 chainId, string metadata); + + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() {} + + function updateChainSettings(uint256 chainId, string memory metadata) public onlyRole(CHAIN_SETTINGS_ROLE) { + _chainSettingsUpdate(chainId, metadata); + } + + function _chainSettingsUpdate(uint256 chainId, string memory metadata) internal { + + bytes32 oldHash = keccak256(bytes(_chainIdToChainSettingsMetadata[chainId])); + bytes32 newHash = keccak256(bytes(metadata)); + if (_chainSettingstMetadataUniqueness[newHash]) revert MetadataNotUnique(newHash); + _chainSettingstMetadataUniqueness[newHash] = true; + _chainSettingstMetadataUniqueness[oldHash] = false; + + _chainIdToChainSettingsMetadata[chainId] = metadata; + emit ChainSettingsUpdated(chainId, metadata); + } + + /** + * @notice Helper to get either msg msg.sender if not a meta transaction, signer of forwarder metatx if it is. + * @inheritdoc ForwardedContext + */ + function _msgSender() internal view virtual override(BaseComponentUpgradeable) returns (address sender) { + return super._msgSender(); + } + + /** + * @notice Helper to get msg.data if not a meta transaction, forwarder data in metatx if it is. + * @inheritdoc ForwardedContext + */ + function _msgData() internal view virtual override(BaseComponentUpgradeable) returns (bytes calldata) { + return super._msgData(); + } + + uint256[50] private __gap; +} \ No newline at end of file From 4981877b813788937320f96eb5d7d2009a42264e Mon Sep 17 00:00:00 2001 From: Roberto Cantu Date: Tue, 11 Apr 2023 14:47:52 -0500 Subject: [PATCH 2/6] Logic for chain settings contract, tests setup --- .../chain_settings/ChainSettingsRegistry.sol | 4 +- .../ChainSettingsRegistryCore.sol | 76 ++++++++++-- scripts/deployments/platform.js | 14 +++ test/components/chainsettings.test.js | 108 ++++++++++++++++++ test/fixture.js | 1 + 5 files changed, 190 insertions(+), 13 deletions(-) create mode 100644 test/components/chainsettings.test.js diff --git a/contracts/components/chain_settings/ChainSettingsRegistry.sol b/contracts/components/chain_settings/ChainSettingsRegistry.sol index f20e057c..22fcc527 100644 --- a/contracts/components/chain_settings/ChainSettingsRegistry.sol +++ b/contracts/components/chain_settings/ChainSettingsRegistry.sol @@ -26,7 +26,7 @@ contract ChainSettingsRegistry is BaseComponentUpgradeable, ChainSettingsRegistr * @notice Helper to get either msg msg.sender if not a meta transaction, signer of forwarder metatx if it is. * @inheritdoc ForwardedContext */ - function _msgSender() internal view virtual override(BaseComponentUpgradeable) returns (address sender) { + function _msgSender() internal view virtual override(BaseComponentUpgradeable, ChainSettingsRegistryCore) returns (address sender) { return super._msgSender(); } @@ -34,7 +34,7 @@ contract ChainSettingsRegistry is BaseComponentUpgradeable, ChainSettingsRegistr * @notice Helper to get msg.data if not a meta transaction, forwarder data in metatx if it is. * @inheritdoc ForwardedContext */ - function _msgData() internal view virtual override(BaseComponentUpgradeable) returns (bytes calldata) { + function _msgData() internal view virtual override(BaseComponentUpgradeable, ChainSettingsRegistryCore) returns (bytes calldata) { return super._msgData(); } diff --git a/contracts/components/chain_settings/ChainSettingsRegistryCore.sol b/contracts/components/chain_settings/ChainSettingsRegistryCore.sol index 9712f8fa..ace8f1e2 100644 --- a/contracts/components/chain_settings/ChainSettingsRegistryCore.sol +++ b/contracts/components/chain_settings/ChainSettingsRegistryCore.sol @@ -7,32 +7,77 @@ import "../BaseComponentUpgradeable.sol"; abstract contract ChainSettingsRegistryCore is BaseComponentUpgradeable { - mapping(uint256 => string) private _chainIdToChainSettingsMetadata; - mapping(bytes32 => bool) private _chainSettingstMetadataUniqueness; + uint256 constant MAX_CHAIN_IDS_PER_UPDATE = 5; + uint256 private _supportedChainIdsAmount; + mapping(uint256 => bool) private _chainIdSupported; + mapping(uint256 => string) private _chainIdMetadata; + // chainId => metadata => uniqueness + mapping(uint256 => mapping(bytes32 => bool)) private _chainIdMetadataUniqueness; + + error ChainIdsAmountExceeded(uint256 exceedingAmount); + error ChainIdAlreadySupported(uint256 chainId); + error ChainIdUnsupported(uint256 chainId); error MetadataNotUnique(bytes32 hash); event ChainSettingsUpdated(uint256 chainId, string metadata); + event ChainIdSupported(uint256 chainId); /// @custom:oz-upgrades-unsafe-allow constructor constructor() {} - function updateChainSettings(uint256 chainId, string memory metadata) public onlyRole(CHAIN_SETTINGS_ROLE) { - _chainSettingsUpdate(chainId, metadata); + // Update supported chain ids, both the amount and the ids themselves + function updateSupportedChains(uint256[] calldata chainIds, string calldata metadata) external onlyRole(CHAIN_SETTINGS_ROLE) { + // Cap on how many chain ids can be updated at once + if(chainIds.length > MAX_CHAIN_IDS_PER_UPDATE) revert ChainIdsAmountExceeded(chainIds.length - MAX_CHAIN_IDS_PER_UPDATE); + + for(uint256 i = 0; i < chainIds.length; i++) { + if(_chainIdSupported[chainIds[i]]) revert ChainIdAlreadySupported(chainIds[i]); + _updateSupportedChainIds(chainIds[i]); + _chainSettingsUpdate(chainIds[i], metadata); + } + + _supportedChainIdsAmount += chainIds.length; } - function _chainSettingsUpdate(uint256 chainId, string memory metadata) internal { + // Update chain settings to be fetched later + function updateChainSettings(uint256[] calldata chainIds, string calldata metadata) external onlyRole(CHAIN_SETTINGS_ROLE) { + if(chainIds.length > _supportedChainIdsAmount) revert ChainIdsAmountExceeded(chainIds.length - _supportedChainIdsAmount); + + for(uint256 i = 0; i < chainIds.length; i++) { + if(!_chainIdSupported[chainIds[i]]) revert ChainIdUnsupported(chainIds[i]); + _chainSettingsUpdate(chainIds[i], metadata); + } + } - bytes32 oldHash = keccak256(bytes(_chainIdToChainSettingsMetadata[chainId])); + function _chainSettingsUpdate(uint256 chainId, string calldata metadata) private { bytes32 newHash = keccak256(bytes(metadata)); - if (_chainSettingstMetadataUniqueness[newHash]) revert MetadataNotUnique(newHash); - _chainSettingstMetadataUniqueness[newHash] = true; - _chainSettingstMetadataUniqueness[oldHash] = false; + if (_chainIdMetadataUniqueness[chainId][newHash]) revert MetadataNotUnique(newHash); + bytes32 oldHash = keccak256(bytes(_chainIdMetadata[chainId])); + _chainIdMetadataUniqueness[chainId][newHash] = true; + _chainIdMetadataUniqueness[chainId][oldHash] = false; - _chainIdToChainSettingsMetadata[chainId] = metadata; + _chainIdMetadata[chainId] = metadata; emit ChainSettingsUpdated(chainId, metadata); } + function _updateSupportedChainIds(uint256 chainId) private { + _chainIdSupported[chainId] = true; + emit ChainIdSupported(chainId); + } + + function getChainIdSettings(uint256 chainId) public view returns (string memory) { + return _chainIdMetadata[chainId]; + } + + function getSupportedChainIdsAmount() public view returns (uint256) { + return _supportedChainIdsAmount; + } + + function isChainIdSupported(uint256 chainId) public view returns (bool) { + return _chainIdSupported[chainId]; + } + /** * @notice Helper to get either msg msg.sender if not a meta transaction, signer of forwarder metatx if it is. * @inheritdoc ForwardedContext @@ -49,5 +94,14 @@ abstract contract ChainSettingsRegistryCore is BaseComponentUpgradeable { return super._msgData(); } - uint256[50] private __gap; + /** + * 50 + * - 1 _supportedChainIdsAmount; + * - 1 _chainIdSupported; + * - 1 _chainIdMetadata; + * - 1 _chainIdMetadataUniqueness; + * -------------------------- + * 46 __gap + */ + uint256[46] private __gap; } \ No newline at end of file diff --git a/scripts/deployments/platform.js b/scripts/deployments/platform.js index ded7613a..c79904ea 100644 --- a/scripts/deployments/platform.js +++ b/scripts/deployments/platform.js @@ -274,6 +274,20 @@ async function migrate(config = {}) { CACHE ); DEBUG(`[${Object.keys(contracts).length}] dispatch: ${contracts.dispatch.address}`); + + DEBUG(`Deploying Chain Setting Registry...`); + contracts.chainSettings = await contractHelpers.tryFetchProxy( + hre, + 'ChainSettingsRegistry', + 'uups', + [contracts.access.address], + { + constructorArgs: [contracts.forwarder.address], + unsafeAllow: 'delegatecall', + }, + CACHE + ); + DEBUG(`[${Object.keys(contracts).length}] chainSettings: ${contracts.dispatch.address}`); } // Roles dictionary diff --git a/test/components/chainsettings.test.js b/test/components/chainsettings.test.js new file mode 100644 index 00000000..e1c2bd48 --- /dev/null +++ b/test/components/chainsettings.test.js @@ -0,0 +1,108 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { prepare } = require('../fixture'); + +describe('Chain Settings Registry', function () { + prepare(); + + const MAX_CHAIN_IDS_PER_UPDATE = 5; + const supportedChainIds = [1, 29, 387, 4654, 53219]; + beforeEach(async function () { + await this.chainSettings.connect(this.accounts.manager).updateSupportedChains(supportedChainIds, 'Metadata1'); + }); + const unsupportedChainIds = [8, 23, 3500, 90059]; + + describe('Adding supported chains', function () { + it('should allow the amount of supported chains to be updated', async function () { + await this.chainSettings.connect(this.accounts.manager).updateSupportedChains([...unsupportedChainIds], 'Metadata1'); + + unsupportedChainIds.forEach(async (chainId) => { + expect(await this.chainSettings.connect(this.accounts.manager).isChainIdSupported(chainId)).to.be.equal(true); + }); + + expect(await this.chainSettings.connect(this.accounts.manager).getSupportedChainIdsAmount()).to.be.equal(supportedChainIds.length + unsupportedChainIds.length); + }); + + it('should not allow account that was not granted access to update supported chains', async function () { + await expect(this.chainSettings.connect(this.accounts.user3).updateSupportedChains([...unsupportedChainIds], 'Metadata1')).to.be.revertedWith( + `MissingRole("${this.roles.CHAIN_SETTINGS}", "${this.accounts.user3.address}")` + ); + }); + + it('should not allow to update supported chains when attempting to add too many chains', async function () { + const additionalUnsupportedChainIds = [37, 98, 444]; + await expect(this.chainSettings.connect(this.accounts.manager).updateSupportedChains( + [...unsupportedChainIds, ...additionalUnsupportedChainIds], + 'Metadata1' + )).to.be.revertedWith( + `ChainIdsAmountExceeded(${(unsupportedChainIds.length + additionalUnsupportedChainIds.length) - MAX_CHAIN_IDS_PER_UPDATE})` + ); + }); + + it('should not allow to add a chain to be supported that is already supported', async function () { + await expect(this.chainSettings.connect(this.accounts.manager).updateSupportedChains([supportedChainIds[1]], 'Metadata2')).to.be.revertedWith( + `ChainIdAlreadySupported(${supportedChainIds[1]})` + ); + }); + + it('should not add support for chain ids if passed chain ids contain a chain that is already supported', async function () { + await expect(this.chainSettings.connect(this.accounts.manager).updateSupportedChains( + [unsupportedChainIds[0], unsupportedChainIds[1], supportedChainIds[1], unsupportedChainIds[2]], + 'Metadata2' + )).to.be.revertedWith( + `ChainIdAlreadySupported(${supportedChainIds[1]})` + ); + + unsupportedChainIds.forEach(async (chainId) => { + expect(await this.chainSettings.connect(this.accounts.manager).isChainIdSupported(chainId)).to.be.equal(false); + }); + + expect(await this.chainSettings.connect(this.accounts.manager).getSupportedChainIdsAmount()).to.be.equal(supportedChainIds.length); + }); + }); + + describe('Updating chain settings', function () { + it('Updates the chain settings', async function () { + await this.chainSettings.connect(this.accounts.manager).updateChainSettings(supportedChainIds, 'Metadata2'); + supportedChainIds.forEach(async (chainId) => { + expect(await this.chainSettings.connect(this.accounts.manager).getChainIdSettings(chainId)).to.be.equal('Metadata2'); + }); + + await this.chainSettings.connect(this.accounts.manager).updateChainSettings(supportedChainIds, 'Metadata3'); + supportedChainIds.forEach(async (chainId) => { + expect(await this.chainSettings.connect(this.accounts.manager).getChainIdSettings(chainId)).to.be.equal('Metadata3'); + }); + }); + + it('should not allow accounts that were not granted access to update chain settings', async function () { + await expect(this.chainSettings.connect(this.accounts.user3).updateChainSettings(supportedChainIds, 'Metadata1')).to.be.revertedWith( + `MissingRole("${this.roles.CHAIN_SETTINGS}", "${this.accounts.user3.address}")` + ); + }); + + it('should not allow settings to be updated when it is the same as current settings', async function () { + supportedChainIds.forEach(async (chainId) => { + expect(await this.chainSettings.connect(this.accounts.manager).updateChainSettings([chainId], 'Metadata1')).to.be.revertedWith( + `MetadataNotUnique("${ethers.utils.id('Metadata1')}")` + ); + }); + }); + + it('should not allow to update more chains than are supported', async function () { + const additionalChainIds = [23, 37]; + // Including the supported chains, but should fail because passing too many chain ids + await expect(this.chainSettings.connect(this.accounts.manager).updateChainSettings( + [...supportedChainIds, ...additionalChainIds], + 'Metadata1' + )).to.be.revertedWith( + `ChainIdsAmountExceeded(${additionalChainIds.length})` + ); + }); + + it('should not allow to update chain settings for an unsupported chain', async function () { + await expect(this.chainSettings.connect(this.accounts.manager).updateChainSettings([unsupportedChainIds[0]], 'Metadata1')).to.be.revertedWith( + `ChainIdUnsupported(${unsupportedChainIds[0]})` + ); + }); + }); +}); \ No newline at end of file diff --git a/test/fixture.js b/test/fixture.js index 338716a1..86d53446 100644 --- a/test/fixture.js +++ b/test/fixture.js @@ -48,6 +48,7 @@ function prepare(config = {}) { this.access.connect(this.accounts.admin).grantRole(this.roles.STAKING_CONTRACT, this.contracts.staking.address), this.access.connect(this.accounts.admin).grantRole(this.roles.ALLOCATOR_CONTRACT, this.contracts.stakeAllocator.address), this.access.connect(this.accounts.admin).grantRole(this.roles.MIGRATION_EXECUTOR, this.accounts.manager.address), + this.access.connect(this.accounts.admin).grantRole(this.roles.CHAIN_SETTINGS, this.accounts.manager.address), this.token.connect(this.accounts.admin).grantRole(this.roles.MINTER, this.accounts.minter.address), this.otherToken.connect(this.accounts.admin).grantRole(this.roles.MINTER, this.accounts.minter.address), ].map((txPromise) => txPromise.then((tx) => tx.wait()).catch(() => {})) From 8a66e0851d3535ad4bceb36bd4b5e82588f6d24f Mon Sep 17 00:00:00 2001 From: Roberto Cantu Date: Tue, 11 Apr 2023 15:52:13 -0500 Subject: [PATCH 3/6] Add comments to functions --- .../ChainSettingsRegistryCore.sol | 40 ++++++++++++++++--- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/contracts/components/chain_settings/ChainSettingsRegistryCore.sol b/contracts/components/chain_settings/ChainSettingsRegistryCore.sol index ace8f1e2..332ade03 100644 --- a/contracts/components/chain_settings/ChainSettingsRegistryCore.sol +++ b/contracts/components/chain_settings/ChainSettingsRegistryCore.sol @@ -23,12 +23,16 @@ abstract contract ChainSettingsRegistryCore is BaseComponentUpgradeable { event ChainSettingsUpdated(uint256 chainId, string metadata); event ChainIdSupported(uint256 chainId); - /// @custom:oz-upgrades-unsafe-allow constructor - constructor() {} - - // Update supported chain ids, both the amount and the ids themselves + /** + * @notice Method to update which chains are supported by the network. + * @dev Method implements a cap to how many chain ids can be updated + * at once, to prevent looping through too many chain ids. + * The cap is also a lower number, since it is expected that the + * supported chains will not change often. + * @param chainIds Array of chain ids that are to be supported. + * @param metadata IPFS pointer to chain id's metadata JSON. + */ function updateSupportedChains(uint256[] calldata chainIds, string calldata metadata) external onlyRole(CHAIN_SETTINGS_ROLE) { - // Cap on how many chain ids can be updated at once if(chainIds.length > MAX_CHAIN_IDS_PER_UPDATE) revert ChainIdsAmountExceeded(chainIds.length - MAX_CHAIN_IDS_PER_UPDATE); for(uint256 i = 0; i < chainIds.length; i++) { @@ -40,7 +44,14 @@ abstract contract ChainSettingsRegistryCore is BaseComponentUpgradeable { _supportedChainIdsAmount += chainIds.length; } - // Update chain settings to be fetched later + /** + * @notice Method to update a chain's settings/metadata. + * @dev Checks to confirm there aren't more chains attempting to be updated + * than there are supported chains. Also checks to confirm that the chains + * attempting to be updated are supported. + * @param chainIds Array of chain ids that are to have their settings updated. + * @param metadata IPFS pointer to chain id's metadata JSON. + */ function updateChainSettings(uint256[] calldata chainIds, string calldata metadata) external onlyRole(CHAIN_SETTINGS_ROLE) { if(chainIds.length > _supportedChainIdsAmount) revert ChainIdsAmountExceeded(chainIds.length - _supportedChainIdsAmount); @@ -50,6 +61,12 @@ abstract contract ChainSettingsRegistryCore is BaseComponentUpgradeable { } } + /** + * @notice Logic for chain metadata update. + * @dev Checks chain id's metadata uniqueness and updates chain's metadata. + * @param chainId Chain id that is to have its settings updated. + * @param metadata IPFS pointer to chain id's metadata JSON. + */ function _chainSettingsUpdate(uint256 chainId, string calldata metadata) private { bytes32 newHash = keccak256(bytes(metadata)); if (_chainIdMetadataUniqueness[chainId][newHash]) revert MetadataNotUnique(newHash); @@ -66,14 +83,25 @@ abstract contract ChainSettingsRegistryCore is BaseComponentUpgradeable { emit ChainIdSupported(chainId); } + /** + * @notice Getter for metadata for the `chainId`. + */ function getChainIdSettings(uint256 chainId) public view returns (string memory) { return _chainIdMetadata[chainId]; } + /** + * @notice Getter for the current amount of supported chains. + */ function getSupportedChainIdsAmount() public view returns (uint256) { return _supportedChainIdsAmount; } + /** + * @notice Checks if chainId is currently supported. + * @param chainId Chain id of the specific chain. + * @return true if chain is supported, false otherwise. + */ function isChainIdSupported(uint256 chainId) public view returns (bool) { return _chainIdSupported[chainId]; } From bd64cdac20ba41f6b1d7e1a631853d6105ea0a1a Mon Sep 17 00:00:00 2001 From: Roberto Cantu Date: Tue, 11 Apr 2023 16:21:42 -0500 Subject: [PATCH 4/6] Combine chain settings contracts into single contract --- .../chain_settings/ChainSettingsRegistry.sol | 112 +++++++++++++-- .../ChainSettingsRegistryCore.sol | 135 ------------------ 2 files changed, 101 insertions(+), 146 deletions(-) delete mode 100644 contracts/components/chain_settings/ChainSettingsRegistryCore.sol diff --git a/contracts/components/chain_settings/ChainSettingsRegistry.sol b/contracts/components/chain_settings/ChainSettingsRegistry.sol index 22fcc527..f14732a1 100644 --- a/contracts/components/chain_settings/ChainSettingsRegistry.sol +++ b/contracts/components/chain_settings/ChainSettingsRegistry.sol @@ -4,10 +4,24 @@ pragma solidity ^0.8.9; import "../BaseComponentUpgradeable.sol"; -import "./ChainSettingsRegistryCore.sol"; -contract ChainSettingsRegistry is BaseComponentUpgradeable, ChainSettingsRegistryCore { +contract ChainSettingsRegistry is BaseComponentUpgradeable { string public constant version = "0.1.0"; + uint256 constant MAX_CHAIN_IDS_PER_UPDATE = 5; + + uint256 private _supportedChainIdsAmount; + mapping(uint256 => bool) private _chainIdSupported; + mapping(uint256 => string) private _chainIdMetadata; + // chainId => metadata => uniqueness + mapping(uint256 => mapping(bytes32 => bool)) private _chainIdMetadataUniqueness; + + error ChainIdsAmountExceeded(uint256 exceedingAmount); + error ChainIdAlreadySupported(uint256 chainId); + error ChainIdUnsupported(uint256 chainId); + error MetadataNotUnique(bytes32 hash); + + event ChainSettingsUpdated(uint256 chainId, string metadata); + event ChainIdSupported(uint256 chainId); /// @custom:oz-upgrades-unsafe-allow constructor constructor(address forwarder) initializer ForwardedContext(forwarder) {} @@ -23,20 +37,96 @@ contract ChainSettingsRegistry is BaseComponentUpgradeable, ChainSettingsRegistr } /** - * @notice Helper to get either msg msg.sender if not a meta transaction, signer of forwarder metatx if it is. - * @inheritdoc ForwardedContext + * @notice Method to update which chains are supported by the network. + * @dev Method implements a cap to how many chain ids can be updated + * at once, to prevent looping through too many chain ids. + * The cap is also a lower number, since it is expected that the + * supported chains will not change often. + * @param chainIds Array of chain ids that are to be supported. + * @param metadata IPFS pointer to chain id's metadata JSON. + */ + function updateSupportedChains(uint256[] calldata chainIds, string calldata metadata) external onlyRole(CHAIN_SETTINGS_ROLE) { + if(chainIds.length > MAX_CHAIN_IDS_PER_UPDATE) revert ChainIdsAmountExceeded(chainIds.length - MAX_CHAIN_IDS_PER_UPDATE); + + for(uint256 i = 0; i < chainIds.length; i++) { + if(_chainIdSupported[chainIds[i]]) revert ChainIdAlreadySupported(chainIds[i]); + _updateSupportedChainIds(chainIds[i]); + _chainSettingsUpdate(chainIds[i], metadata); + } + + _supportedChainIdsAmount += chainIds.length; + } + + /** + * @notice Method to update a chain's settings/metadata. + * @dev Checks to confirm there aren't more chains attempting to be updated + * than there are supported chains. Also checks to confirm that the chains + * attempting to be updated are supported. + * @param chainIds Array of chain ids that are to have their settings updated. + * @param metadata IPFS pointer to chain id's metadata JSON. */ - function _msgSender() internal view virtual override(BaseComponentUpgradeable, ChainSettingsRegistryCore) returns (address sender) { - return super._msgSender(); + function updateChainSettings(uint256[] calldata chainIds, string calldata metadata) external onlyRole(CHAIN_SETTINGS_ROLE) { + if(chainIds.length > _supportedChainIdsAmount) revert ChainIdsAmountExceeded(chainIds.length - _supportedChainIdsAmount); + + for(uint256 i = 0; i < chainIds.length; i++) { + if(!_chainIdSupported[chainIds[i]]) revert ChainIdUnsupported(chainIds[i]); + _chainSettingsUpdate(chainIds[i], metadata); + } } /** - * @notice Helper to get msg.data if not a meta transaction, forwarder data in metatx if it is. - * @inheritdoc ForwardedContext + * @notice Logic for chain metadata update. + * @dev Checks chain id's metadata uniqueness and updates chain's metadata. + * @param chainId Chain id that is to have its settings updated. + * @param metadata IPFS pointer to chain id's metadata JSON. */ - function _msgData() internal view virtual override(BaseComponentUpgradeable, ChainSettingsRegistryCore) returns (bytes calldata) { - return super._msgData(); + function _chainSettingsUpdate(uint256 chainId, string calldata metadata) private { + bytes32 newHash = keccak256(bytes(metadata)); + if (_chainIdMetadataUniqueness[chainId][newHash]) revert MetadataNotUnique(newHash); + bytes32 oldHash = keccak256(bytes(_chainIdMetadata[chainId])); + _chainIdMetadataUniqueness[chainId][newHash] = true; + _chainIdMetadataUniqueness[chainId][oldHash] = false; + + _chainIdMetadata[chainId] = metadata; + emit ChainSettingsUpdated(chainId, metadata); } - uint256[50] private __gap; + function _updateSupportedChainIds(uint256 chainId) private { + _chainIdSupported[chainId] = true; + emit ChainIdSupported(chainId); + } + + /** + * @notice Getter for metadata for the `chainId`. + */ + function getChainIdSettings(uint256 chainId) public view returns (string memory) { + return _chainIdMetadata[chainId]; + } + + /** + * @notice Getter for the current amount of supported chains. + */ + function getSupportedChainIdsAmount() public view returns (uint256) { + return _supportedChainIdsAmount; + } + + /** + * @notice Checks if chainId is currently supported. + * @param chainId Chain id of the specific chain. + * @return true if chain is supported, false otherwise. + */ + function isChainIdSupported(uint256 chainId) public view returns (bool) { + return _chainIdSupported[chainId]; + } + + /** + * 50 + * - 1 _supportedChainIdsAmount; + * - 1 _chainIdSupported; + * - 1 _chainIdMetadata; + * - 1 _chainIdMetadataUniqueness; + * -------------------------- + * 46 __gap + */ + uint256[46] private __gap; } diff --git a/contracts/components/chain_settings/ChainSettingsRegistryCore.sol b/contracts/components/chain_settings/ChainSettingsRegistryCore.sol deleted file mode 100644 index 332ade03..00000000 --- a/contracts/components/chain_settings/ChainSettingsRegistryCore.sol +++ /dev/null @@ -1,135 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -// See Forta Network License: https://github.com/forta-network/forta-contracts/blob/master/LICENSE.md - -pragma solidity ^0.8.9; - -import "../BaseComponentUpgradeable.sol"; - -abstract contract ChainSettingsRegistryCore is BaseComponentUpgradeable { - - uint256 constant MAX_CHAIN_IDS_PER_UPDATE = 5; - - uint256 private _supportedChainIdsAmount; - mapping(uint256 => bool) private _chainIdSupported; - mapping(uint256 => string) private _chainIdMetadata; - // chainId => metadata => uniqueness - mapping(uint256 => mapping(bytes32 => bool)) private _chainIdMetadataUniqueness; - - error ChainIdsAmountExceeded(uint256 exceedingAmount); - error ChainIdAlreadySupported(uint256 chainId); - error ChainIdUnsupported(uint256 chainId); - error MetadataNotUnique(bytes32 hash); - - event ChainSettingsUpdated(uint256 chainId, string metadata); - event ChainIdSupported(uint256 chainId); - - /** - * @notice Method to update which chains are supported by the network. - * @dev Method implements a cap to how many chain ids can be updated - * at once, to prevent looping through too many chain ids. - * The cap is also a lower number, since it is expected that the - * supported chains will not change often. - * @param chainIds Array of chain ids that are to be supported. - * @param metadata IPFS pointer to chain id's metadata JSON. - */ - function updateSupportedChains(uint256[] calldata chainIds, string calldata metadata) external onlyRole(CHAIN_SETTINGS_ROLE) { - if(chainIds.length > MAX_CHAIN_IDS_PER_UPDATE) revert ChainIdsAmountExceeded(chainIds.length - MAX_CHAIN_IDS_PER_UPDATE); - - for(uint256 i = 0; i < chainIds.length; i++) { - if(_chainIdSupported[chainIds[i]]) revert ChainIdAlreadySupported(chainIds[i]); - _updateSupportedChainIds(chainIds[i]); - _chainSettingsUpdate(chainIds[i], metadata); - } - - _supportedChainIdsAmount += chainIds.length; - } - - /** - * @notice Method to update a chain's settings/metadata. - * @dev Checks to confirm there aren't more chains attempting to be updated - * than there are supported chains. Also checks to confirm that the chains - * attempting to be updated are supported. - * @param chainIds Array of chain ids that are to have their settings updated. - * @param metadata IPFS pointer to chain id's metadata JSON. - */ - function updateChainSettings(uint256[] calldata chainIds, string calldata metadata) external onlyRole(CHAIN_SETTINGS_ROLE) { - if(chainIds.length > _supportedChainIdsAmount) revert ChainIdsAmountExceeded(chainIds.length - _supportedChainIdsAmount); - - for(uint256 i = 0; i < chainIds.length; i++) { - if(!_chainIdSupported[chainIds[i]]) revert ChainIdUnsupported(chainIds[i]); - _chainSettingsUpdate(chainIds[i], metadata); - } - } - - /** - * @notice Logic for chain metadata update. - * @dev Checks chain id's metadata uniqueness and updates chain's metadata. - * @param chainId Chain id that is to have its settings updated. - * @param metadata IPFS pointer to chain id's metadata JSON. - */ - function _chainSettingsUpdate(uint256 chainId, string calldata metadata) private { - bytes32 newHash = keccak256(bytes(metadata)); - if (_chainIdMetadataUniqueness[chainId][newHash]) revert MetadataNotUnique(newHash); - bytes32 oldHash = keccak256(bytes(_chainIdMetadata[chainId])); - _chainIdMetadataUniqueness[chainId][newHash] = true; - _chainIdMetadataUniqueness[chainId][oldHash] = false; - - _chainIdMetadata[chainId] = metadata; - emit ChainSettingsUpdated(chainId, metadata); - } - - function _updateSupportedChainIds(uint256 chainId) private { - _chainIdSupported[chainId] = true; - emit ChainIdSupported(chainId); - } - - /** - * @notice Getter for metadata for the `chainId`. - */ - function getChainIdSettings(uint256 chainId) public view returns (string memory) { - return _chainIdMetadata[chainId]; - } - - /** - * @notice Getter for the current amount of supported chains. - */ - function getSupportedChainIdsAmount() public view returns (uint256) { - return _supportedChainIdsAmount; - } - - /** - * @notice Checks if chainId is currently supported. - * @param chainId Chain id of the specific chain. - * @return true if chain is supported, false otherwise. - */ - function isChainIdSupported(uint256 chainId) public view returns (bool) { - return _chainIdSupported[chainId]; - } - - /** - * @notice Helper to get either msg msg.sender if not a meta transaction, signer of forwarder metatx if it is. - * @inheritdoc ForwardedContext - */ - function _msgSender() internal view virtual override(BaseComponentUpgradeable) returns (address sender) { - return super._msgSender(); - } - - /** - * @notice Helper to get msg.data if not a meta transaction, forwarder data in metatx if it is. - * @inheritdoc ForwardedContext - */ - function _msgData() internal view virtual override(BaseComponentUpgradeable) returns (bytes calldata) { - return super._msgData(); - } - - /** - * 50 - * - 1 _supportedChainIdsAmount; - * - 1 _chainIdSupported; - * - 1 _chainIdMetadata; - * - 1 _chainIdMetadataUniqueness; - * -------------------------- - * 46 __gap - */ - uint256[46] private __gap; -} \ No newline at end of file From 0f916f7012d2980d732990f7953f2c8f474539db Mon Sep 17 00:00:00 2001 From: Roberto Cantu Date: Tue, 11 Apr 2023 17:41:41 -0500 Subject: [PATCH 5/6] Add test case for passing empty chain ids array --- .../chain_settings/ChainSettingsRegistry.sol | 11 +++++++---- test/components/chainsettings.test.js | 12 ++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/contracts/components/chain_settings/ChainSettingsRegistry.sol b/contracts/components/chain_settings/ChainSettingsRegistry.sol index f14732a1..acf00016 100644 --- a/contracts/components/chain_settings/ChainSettingsRegistry.sol +++ b/contracts/components/chain_settings/ChainSettingsRegistry.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.9; import "../BaseComponentUpgradeable.sol"; +import "../../errors/GeneralErrors.sol"; contract ChainSettingsRegistry is BaseComponentUpgradeable { string public constant version = "0.1.0"; @@ -46,10 +47,11 @@ contract ChainSettingsRegistry is BaseComponentUpgradeable { * @param metadata IPFS pointer to chain id's metadata JSON. */ function updateSupportedChains(uint256[] calldata chainIds, string calldata metadata) external onlyRole(CHAIN_SETTINGS_ROLE) { - if(chainIds.length > MAX_CHAIN_IDS_PER_UPDATE) revert ChainIdsAmountExceeded(chainIds.length - MAX_CHAIN_IDS_PER_UPDATE); + if (chainIds.length == 0) revert EmptyArray("chainIds"); + if (chainIds.length > MAX_CHAIN_IDS_PER_UPDATE) revert ChainIdsAmountExceeded(chainIds.length - MAX_CHAIN_IDS_PER_UPDATE); for(uint256 i = 0; i < chainIds.length; i++) { - if(_chainIdSupported[chainIds[i]]) revert ChainIdAlreadySupported(chainIds[i]); + if (_chainIdSupported[chainIds[i]]) revert ChainIdAlreadySupported(chainIds[i]); _updateSupportedChainIds(chainIds[i]); _chainSettingsUpdate(chainIds[i], metadata); } @@ -66,10 +68,11 @@ contract ChainSettingsRegistry is BaseComponentUpgradeable { * @param metadata IPFS pointer to chain id's metadata JSON. */ function updateChainSettings(uint256[] calldata chainIds, string calldata metadata) external onlyRole(CHAIN_SETTINGS_ROLE) { - if(chainIds.length > _supportedChainIdsAmount) revert ChainIdsAmountExceeded(chainIds.length - _supportedChainIdsAmount); + if (chainIds.length == 0) revert EmptyArray("chainIds"); + if (chainIds.length > _supportedChainIdsAmount) revert ChainIdsAmountExceeded(chainIds.length - _supportedChainIdsAmount); for(uint256 i = 0; i < chainIds.length; i++) { - if(!_chainIdSupported[chainIds[i]]) revert ChainIdUnsupported(chainIds[i]); + if (!_chainIdSupported[chainIds[i]]) revert ChainIdUnsupported(chainIds[i]); _chainSettingsUpdate(chainIds[i], metadata); } } diff --git a/test/components/chainsettings.test.js b/test/components/chainsettings.test.js index e1c2bd48..31b117b7 100644 --- a/test/components/chainsettings.test.js +++ b/test/components/chainsettings.test.js @@ -59,6 +59,12 @@ describe('Chain Settings Registry', function () { expect(await this.chainSettings.connect(this.accounts.manager).getSupportedChainIdsAmount()).to.be.equal(supportedChainIds.length); }); + + it('should not allow to pass an empty array of chains ids to be supported', async function () { + await expect(this.chainSettings.connect(this.accounts.manager).updateSupportedChains([], 'Metadata1')).to.be.revertedWith( + `EmptyArray("chainIds")` + ); + }); }); describe('Updating chain settings', function () { @@ -104,5 +110,11 @@ describe('Chain Settings Registry', function () { `ChainIdUnsupported(${unsupportedChainIds[0]})` ); }); + + it('should not allow to pass an empty array of chains ids to update chain settings', async function () { + await expect(this.chainSettings.connect(this.accounts.manager).updateChainSettings([], 'Metadata2')).to.be.revertedWith( + `EmptyArray("chainIds")` + ); + }); }); }); \ No newline at end of file From ae312c6a33788ca889ff9611b0be1062da5a71db Mon Sep 17 00:00:00 2001 From: Roberto Cantu Date: Wed, 12 Apr 2023 09:08:46 -0500 Subject: [PATCH 6/6] Make event params indexed --- .../components/chain_settings/ChainSettingsRegistry.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/components/chain_settings/ChainSettingsRegistry.sol b/contracts/components/chain_settings/ChainSettingsRegistry.sol index acf00016..2315e255 100644 --- a/contracts/components/chain_settings/ChainSettingsRegistry.sol +++ b/contracts/components/chain_settings/ChainSettingsRegistry.sol @@ -8,7 +8,7 @@ import "../../errors/GeneralErrors.sol"; contract ChainSettingsRegistry is BaseComponentUpgradeable { string public constant version = "0.1.0"; - uint256 constant MAX_CHAIN_IDS_PER_UPDATE = 5; + uint8 constant MAX_CHAIN_IDS_PER_UPDATE = 5; uint256 private _supportedChainIdsAmount; mapping(uint256 => bool) private _chainIdSupported; @@ -21,8 +21,8 @@ contract ChainSettingsRegistry is BaseComponentUpgradeable { error ChainIdUnsupported(uint256 chainId); error MetadataNotUnique(bytes32 hash); - event ChainSettingsUpdated(uint256 chainId, string metadata); - event ChainIdSupported(uint256 chainId); + event ChainSettingsUpdated(uint256 indexed chainId, string metadata); + event ChainIdSupported(uint256 indexed chainId); /// @custom:oz-upgrades-unsafe-allow constructor constructor(address forwarder) initializer ForwardedContext(forwarder) {}