From 3e5fa9a13a540ca088bc267a4aa29340cd5e77ed Mon Sep 17 00:00:00 2001 From: Ignasi Date: Fri, 11 Oct 2024 16:19:58 +0200 Subject: [PATCH] Internal audit fixes + new coverage --- .../IBasePolygonZkEVMGlobalExitRoot.sol | 4 +- contracts/v2/PolygonZkEVMBridgeV2.sol | 4 +- .../interfaces/IBridgeL2SovereignChains.sol | 21 ++++ .../BridgeL2SovereignChain.sol | 63 ++++++++-- .../GlobalExitRootManagerL2SovereignChain.sol | 40 ++++--- hardhat.config.ts | 2 +- ...dgeL2GasTokenMappedSovereignChains.test.ts | 26 ---- .../BridgeL2GasTokensSovereignChains.test.ts | 112 ++++++++---------- .../BridgeL2SovereignChain.test.ts | 103 +++++----------- 9 files changed, 179 insertions(+), 196 deletions(-) diff --git a/contracts/interfaces/IBasePolygonZkEVMGlobalExitRoot.sol b/contracts/interfaces/IBasePolygonZkEVMGlobalExitRoot.sol index 383b58ff..d8e149fd 100644 --- a/contracts/interfaces/IBasePolygonZkEVMGlobalExitRoot.sol +++ b/contracts/interfaces/IBasePolygonZkEVMGlobalExitRoot.sol @@ -9,9 +9,9 @@ interface IBasePolygonZkEVMGlobalExitRoot { error OnlyAllowedContracts(); /** - * @dev Thrown when the caller is not the coinbase + * @dev Thrown when the caller is not the coinbase neither the globalExitRootUpdater */ - error OnlyAggOracleOrCoinbase(); + error OnlyGlobalExitRootUpdater(); /** * @dev Thrown when trying to insert a global exit root that is already set diff --git a/contracts/v2/PolygonZkEVMBridgeV2.sol b/contracts/v2/PolygonZkEVMBridgeV2.sol index 4b8eea59..4a705c31 100644 --- a/contracts/v2/PolygonZkEVMBridgeV2.sol +++ b/contracts/v2/PolygonZkEVMBridgeV2.sol @@ -740,7 +740,7 @@ contract PolygonZkEVMBridgeV2 is * @notice Function to activate the emergency state " Only can be called by the Polygon ZK-EVM in extreme situations */ - function activateEmergencyState() external onlyRollupManager { + function activateEmergencyState() external virtual onlyRollupManager { _activateEmergencyState(); } @@ -748,7 +748,7 @@ contract PolygonZkEVMBridgeV2 is * @notice Function to deactivate the emergency state " Only can be called by the Polygon ZK-EVM */ - function deactivateEmergencyState() external onlyRollupManager { + function deactivateEmergencyState() external virtual onlyRollupManager { _deactivateEmergencyState(); } diff --git a/contracts/v2/interfaces/IBridgeL2SovereignChains.sol b/contracts/v2/interfaces/IBridgeL2SovereignChains.sol index 195746aa..6948e5fb 100644 --- a/contracts/v2/interfaces/IBridgeL2SovereignChains.sol +++ b/contracts/v2/interfaces/IBridgeL2SovereignChains.sol @@ -46,6 +46,27 @@ interface IBridgeL2SovereignChains is IPolygonZkEVMBridgeV2 { */ error InvalidInitializeFunction(); + /** + * @dev Thrown when initializing calling a function with invalid arrays length + */ + error InputArraysLengthMismatch(); + + /** + * @dev Thrown when trying to map a token that is already mapped + */ + error TokenAlreadyMapped(); + + /** + * @dev Thrown when trying to remove a legacy mapped token that has nor previously been remapped + */ + error TokenNotRemapped(); + + /** + * @dev Thrown when trying to set a custom wrapper for weth on a gas token network + */ + error WETHRemappingNotSupportedOnGasTokenNetworks(); + + function initialize( uint32 _networkID, address _gasTokenAddress, diff --git a/contracts/v2/sovereignChains/BridgeL2SovereignChain.sol b/contracts/v2/sovereignChains/BridgeL2SovereignChain.sol index 686c4936..214c9b9c 100644 --- a/contracts/v2/sovereignChains/BridgeL2SovereignChain.sol +++ b/contracts/v2/sovereignChains/BridgeL2SovereignChain.sol @@ -52,7 +52,7 @@ contract BridgeL2SovereignChain is /** * @dev Emitted when a remapped token is removed from mapping */ - event RemoveSovereignTokenAddress(address sovereignTokenAddress); + event RemoveLegacySovereignTokenAddress(address sovereignTokenAddress); /** * @dev Emitted when a WETH address is remapped by a sovereign WETH address @@ -118,6 +118,7 @@ contract BridgeL2SovereignChain is gasTokenAddress = _gasTokenAddress; gasTokenNetwork = _gasTokenNetwork; gasTokenMetadata = _gasTokenMetadata; + // Set sovereign weth token or create new if not provided if (_sovereignWETHAddress == address(0)) { // Create a wrapped token for WETH, with salt == 0 @@ -185,12 +186,14 @@ contract BridgeL2SovereignChain is address[] memory sovereignTokenAddresses, bool[] memory isNotMintable ) external onlyBridgeManager { - require( - originNetworks.length == originTokenAddresses.length && - originTokenAddresses.length == sovereignTokenAddresses.length && - sovereignTokenAddresses.length == isNotMintable.length, - "Input array lengths mismatch" - ); + if ( + originNetworks.length != originTokenAddresses.length || + originNetworks.length != sovereignTokenAddresses.length || + originNetworks.length != isNotMintable.length + ) { + revert InputArraysLengthMismatch(); + } + // Make multiple calls to setSovereignTokenAddress for (uint256 i = 0; i < sovereignTokenAddresses.length; i++) { _setSovereignTokenAddress( @@ -229,7 +232,16 @@ contract BridgeL2SovereignChain is } /** - * @notice Function to remap sovereign address + * @notice Remap a wrapped token to a new sovereign token address + * @dev This function is used to allow any existing token to be mapped with + * origin token. + * @notice If this function is called multiple times for the same existingTokenAddress, + * this will override the previous calls and only keep the last sovereignTokenAddress. + * @notice The tokenInfoToWrappedToken mapping value is replaced by the new sovereign address but it's not the case for the wrappedTokenToTokenInfo map where the value is added, this way user will always be able to withdraw their tokens + * @param originNetwork Origin network + * @param originTokenAddress Origin token address, 0 address is reserved for ether + * @param sovereignTokenAddress Address of the sovereign wrapped token + * @param isNotMintable Flag to indicate if the wrapped token is not mintable */ function _setSovereignTokenAddress( uint32 originNetwork, @@ -248,6 +260,14 @@ contract BridgeL2SovereignChain is if (originNetwork == networkID) { revert OriginNetworkInvalid(); } + // Check if the token is already mapped + if ( + wrappedTokenToTokenInfo[sovereignTokenAddress].originTokenAddress != + address(0) + ) { + revert TokenAlreadyMapped(); + } + // Compute token info hash bytes32 tokenInfoHash = keccak256( abi.encodePacked(originNetwork, originTokenAddress) @@ -255,6 +275,7 @@ contract BridgeL2SovereignChain is // Set the address of the wrapper tokenInfoToWrappedToken[tokenInfoHash] = sovereignTokenAddress; // Set the token info mapping + // @note wrappedTokenToTokenInfo mapping is not overwritten while tokenInfoToWrappedToken it is wrappedTokenToTokenInfo[sovereignTokenAddress] = TokenInformation( originNetwork, originTokenAddress @@ -274,10 +295,10 @@ contract BridgeL2SovereignChain is * @notice Although the token is removed from the mapping, the user will still be able to withdraw their tokens using tokenInfoToWrappedToken mapping * @param sovereignTokenAddress Address of the sovereign wrapped token */ - function removeSovereignTokenAddress( + function removeLegacySovereignTokenAddress( address sovereignTokenAddress ) external onlyBridgeManager { - // Only allow to remove already mapped tokens + // Only allow to remove already remapped tokens TokenInformation memory tokenInfo = wrappedTokenToTokenInfo[ sovereignTokenAddress ]; @@ -292,11 +313,11 @@ contract BridgeL2SovereignChain is tokenInfoToWrappedToken[tokenInfoHash] == address(0) || tokenInfoToWrappedToken[tokenInfoHash] == sovereignTokenAddress ) { - revert TokenNotMapped(); + revert TokenNotRemapped(); } delete wrappedTokenToTokenInfo[sovereignTokenAddress]; delete wrappedAddressIsNotMintable[sovereignTokenAddress]; - emit RemoveSovereignTokenAddress(sovereignTokenAddress); + emit RemoveLegacySovereignTokenAddress(sovereignTokenAddress); } /** @@ -309,6 +330,9 @@ contract BridgeL2SovereignChain is address sovereignWETHTokenAddress, bool isNotMintable ) external onlyBridgeManager { + if (gasTokenAddress == address(0)) { + revert WETHRemappingNotSupportedOnGasTokenNetworks(); + } WETHToken = TokenWrapped(sovereignWETHTokenAddress); wrappedAddressIsNotMintable[sovereignWETHTokenAddress] = isNotMintable; emit SetSovereignWETHAddress(sovereignWETHTokenAddress, isNotMintable); @@ -411,4 +435,19 @@ contract BridgeL2SovereignChain is tokenWrapped.mint(destinationAddress, amount); } } + + // @note This function is not used in the current implementation. We overwrite it to improve deployed bytecode size + function activateEmergencyState() + external + override(IPolygonZkEVMBridgeV2, PolygonZkEVMBridgeV2) + { + revert NotValidBridgeManager(); + } + + function deactivateEmergencyState() + external + override(IPolygonZkEVMBridgeV2, PolygonZkEVMBridgeV2) + { + revert NotValidBridgeManager(); + } } diff --git a/contracts/v2/sovereignChains/GlobalExitRootManagerL2SovereignChain.sol b/contracts/v2/sovereignChains/GlobalExitRootManagerL2SovereignChain.sol index 5fb69e96..d92e0625 100644 --- a/contracts/v2/sovereignChains/GlobalExitRootManagerL2SovereignChain.sol +++ b/contracts/v2/sovereignChains/GlobalExitRootManagerL2SovereignChain.sol @@ -13,8 +13,8 @@ contract GlobalExitRootManagerL2SovereignChain is PolygonZkEVMGlobalExitRootL2, Initializable { - // aggOracleAddress address - address public aggOracleAddress; + // globalExitRootUpdater address + address public globalExitRootUpdater; // Inserted GER counter uint256 public insertedGERCount; @@ -29,13 +29,16 @@ contract GlobalExitRootManagerL2SovereignChain is */ event RemoveLastGlobalExitRoot(bytes32 indexed removedGlobalExitRoot); - modifier onlyAggOracleOrCoinbase() { - // Only allowed to be called by aggOracle or coinbase if aggOracle is zero - if ( - (aggOracleAddress == address(0) && block.coinbase != msg.sender) || - (aggOracleAddress != address(0) && aggOracleAddress != msg.sender) - ) { - revert OnlyAggOracleOrCoinbase(); + modifier onlyGlobalExitRootUpdater() { + // Only allowed to be called by GlobalExitRootUpdater or coinbase if GlobalExitRootUpdater is zero + if (globalExitRootUpdater == address(0)) { + if (block.coinbase != msg.sender) { + revert OnlyGlobalExitRootUpdater(); + } + } else { + if (globalExitRootUpdater != msg.sender) { + revert OnlyGlobalExitRootUpdater(); + } } _; } @@ -50,13 +53,13 @@ contract GlobalExitRootManagerL2SovereignChain is } /** - * @notice Initialize contract setting the aggOracleAddress + * @notice Initialize contract setting the globalExitRootUpdater */ function initialize( - address _aggOracleAddress + address _globalExitRootUpdater ) external virtual initializer { - // set aggOracleAddress - aggOracleAddress = _aggOracleAddress; + // set globalExitRootUpdater + globalExitRootUpdater = _globalExitRootUpdater; } /** @@ -65,7 +68,7 @@ contract GlobalExitRootManagerL2SovereignChain is */ function insertGlobalExitRoot( bytes32 _newRoot - ) external onlyAggOracleOrCoinbase { + ) external onlyGlobalExitRootUpdater { // do not insert GER if already set if (globalExitRootMap[_newRoot] == 0) { globalExitRootMap[_newRoot] = ++insertedGERCount; @@ -81,9 +84,10 @@ contract GlobalExitRootManagerL2SovereignChain is */ function removeLastGlobalExitRoots( bytes32[] calldata gersToRemove - ) external onlyAggOracleOrCoinbase { + ) external onlyGlobalExitRootUpdater { + uint256 insertedGERCountCache = insertedGERCount; // Can't remove if not enough roots have been inserted - if (gersToRemove.length > insertedGERCount) { + if (gersToRemove.length > insertedGERCountCache) { revert NotEnoughGlobalExitRootsInserted(); } // Iterate through the array of roots to remove them one by one @@ -92,12 +96,12 @@ contract GlobalExitRootManagerL2SovereignChain is // Check that the root to remove is the last inserted uint256 lastInsertedIndex = globalExitRootMap[rootToRemove]; - if (lastInsertedIndex != insertedGERCount) { + if (lastInsertedIndex != insertedGERCountCache) { revert NotLastInsertedGlobalExitRoot(); } // Remove from the mapping - delete globalExitRootMap[rootToRemove]; // Delete from the mapping + delete globalExitRootMap[rootToRemove]; // Decrement the counter insertedGERCount--; diff --git a/hardhat.config.ts b/hardhat.config.ts index c41a38d4..9c4bb60c 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -80,7 +80,7 @@ const config: HardhatUserConfig = { settings: { optimizer: { enabled: true, - runs: 0, + runs: 20, }, evmVersion: "shanghai", }, // try yul optimizer diff --git a/test/contractsv2/BridgeL2GasTokenMappedSovereignChains.test.ts b/test/contractsv2/BridgeL2GasTokenMappedSovereignChains.test.ts index 1013ec9f..8f4bc6ab 100644 --- a/test/contractsv2/BridgeL2GasTokenMappedSovereignChains.test.ts +++ b/test/contractsv2/BridgeL2GasTokenMappedSovereignChains.test.ts @@ -263,32 +263,6 @@ describe("SovereignChainBridge Gas tokens tests", () => { expect(await sovereignChainBridgeContract.gasTokenMetadata()).to.be.equal(gasTokenMetadata); }); - it("should check the emergency state", async () => { - expect(await sovereignChainBridgeContract.isEmergencyState()).to.be.equal(false); - - await expect(sovereignChainBridgeContract.activateEmergencyState()).to.be.revertedWithCustomError( - sovereignChainBridgeContract, - "OnlyRollupManager" - ); - await expect(sovereignChainBridgeContract.connect(rollupManager).activateEmergencyState()).to.emit( - sovereignChainBridgeContract, - "EmergencyStateActivated" - ); - - expect(await sovereignChainBridgeContract.isEmergencyState()).to.be.equal(true); - - await expect( - sovereignChainBridgeContract.connect(deployer).deactivateEmergencyState() - ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "OnlyRollupManager"); - - await expect(sovereignChainBridgeContract.connect(rollupManager).deactivateEmergencyState()).to.emit( - sovereignChainBridgeContract, - "EmergencyStateDeactivated" - ); - - expect(await sovereignChainBridgeContract.isEmergencyState()).to.be.equal(false); - }); - it("should SovereignChain bridge asset and verify merkle proof", async () => { const depositCount = await sovereignChainBridgeContract.depositCount(); const originNetwork = networkIDRollup2; diff --git a/test/contractsv2/BridgeL2GasTokensSovereignChains.test.ts b/test/contractsv2/BridgeL2GasTokensSovereignChains.test.ts index ee44f4d3..96b83cec 100644 --- a/test/contractsv2/BridgeL2GasTokensSovereignChains.test.ts +++ b/test/contractsv2/BridgeL2GasTokensSovereignChains.test.ts @@ -31,7 +31,7 @@ describe("SovereignChainBridge Gas tokens tests", () => { let sovereignChainBridgeContract: BridgeL2SovereignChain; let polTokenContract: ERC20PermitMock; - let sovereignChainGlobalExitRoot: GlobalExitRootManagerL2SovereignChain; + let sovereignChainGlobalExitRootContract: GlobalExitRootManagerL2SovereignChain; let deployer: any; let rollupManager: any; @@ -72,12 +72,18 @@ describe("SovereignChainBridge Gas tokens tests", () => { })) as unknown as BridgeL2SovereignChain; // deploy global exit root manager - const SovereignChainGlobalExitRootFactory = await ethers.getContractFactory( + const GlobalExitRootManagerL2SovereignChainFactory = await ethers.getContractFactory( "GlobalExitRootManagerL2SovereignChain" ); - sovereignChainGlobalExitRoot = await SovereignChainGlobalExitRootFactory.deploy( - sovereignChainBridgeContract.target - ); + sovereignChainGlobalExitRootContract = (await upgrades.deployProxy( + GlobalExitRootManagerL2SovereignChainFactory, + [deployer.address], // Initializer params + { + initializer: "initialize", // initializer function name + constructorArgs: [sovereignChainBridgeContract.target], // Constructor arguments + unsafeAllow: ["constructor", "state-variable-immutable"], + } + )) as unknown as GlobalExitRootManagerL2SovereignChain; // deploy token const maticTokenFactory = await ethers.getContractFactory("ERC20PermitMock"); @@ -96,7 +102,7 @@ describe("SovereignChainBridge Gas tokens tests", () => { networkIDRollup2, polTokenContract.target, // zero for ether 0, // zero for ether - sovereignChainGlobalExitRoot.target, + sovereignChainGlobalExitRootContract.target, rollupManager.address, metadataToken, ethers.Typed.address(bridgeManager.address), @@ -172,20 +178,26 @@ describe("SovereignChainBridge Gas tokens tests", () => { // add rollup Merkle root await ethers.provider.send("hardhat_impersonateAccount", [sovereignChainBridgeContract.target]); const bridgeMock = await ethers.getSigner(sovereignChainBridgeContract.target as any); - await sovereignChainGlobalExitRoot.connect(bridgeMock).updateExitRoot(rollupRoot, {gasPrice: 0}); + await sovereignChainGlobalExitRootContract.connect(bridgeMock).updateExitRoot(rollupRoot, {gasPrice: 0}); // check roots - const rollupExitRootSC = await sovereignChainGlobalExitRoot.lastRollupExitRoot(); + const rollupExitRootSC = await sovereignChainGlobalExitRootContract.lastRollupExitRoot(); expect(rollupExitRootSC).to.be.equal(rollupRoot); const computedGlobalExitRoot = calculateGlobalExitRoot(mainnetExitRoot, rollupExitRootSC); + + // Try to insert global exit root with non coinbase + await expect( + sovereignChainGlobalExitRootContract.connect(acc1).insertGlobalExitRoot(computedGlobalExitRoot) + ).to.be.revertedWithCustomError(sovereignChainGlobalExitRootContract, "OnlyGlobalExitRootUpdater"); + // Insert global exit root - expect(await sovereignChainGlobalExitRoot.insertGlobalExitRoot(computedGlobalExitRoot)) - .to.emit(sovereignChainGlobalExitRoot, "InsertGlobalExitRoot") + expect(await sovereignChainGlobalExitRootContract.insertGlobalExitRoot(computedGlobalExitRoot)) + .to.emit(sovereignChainGlobalExitRootContract, "InsertGlobalExitRoot") .withArgs(computedGlobalExitRoot); // Check GER has value in mapping - expect(await sovereignChainGlobalExitRoot.globalExitRootMap(computedGlobalExitRoot)).to.not.be.eq(0); + expect(await sovereignChainGlobalExitRootContract.globalExitRootMap(computedGlobalExitRoot)).to.not.be.eq(0); // check merkle proof const index = 0; const proofLocal = merkleTree.getProofTreeByIndex(0); @@ -266,7 +278,7 @@ describe("SovereignChainBridge Gas tokens tests", () => { it("should check the constructor parameters", async () => { expect(await sovereignChainBridgeContract.globalExitRootManager()).to.be.equal( - sovereignChainGlobalExitRoot.target + sovereignChainGlobalExitRootContract.target ); expect(await sovereignChainBridgeContract.networkID()).to.be.equal(networkIDRollup2); expect(await sovereignChainBridgeContract.polygonRollupManager()).to.be.equal(rollupManager.address); @@ -276,32 +288,6 @@ describe("SovereignChainBridge Gas tokens tests", () => { expect(await sovereignChainBridgeContract.gasTokenMetadata()).to.be.equal(gasTokenMetadata); }); - it("should check the emergency state", async () => { - expect(await sovereignChainBridgeContract.isEmergencyState()).to.be.equal(false); - - await expect(sovereignChainBridgeContract.activateEmergencyState()).to.be.revertedWithCustomError( - sovereignChainBridgeContract, - "OnlyRollupManager" - ); - await expect(sovereignChainBridgeContract.connect(rollupManager).activateEmergencyState()).to.emit( - sovereignChainBridgeContract, - "EmergencyStateActivated" - ); - - expect(await sovereignChainBridgeContract.isEmergencyState()).to.be.equal(true); - - await expect( - sovereignChainBridgeContract.connect(deployer).deactivateEmergencyState() - ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "OnlyRollupManager"); - - await expect(sovereignChainBridgeContract.connect(rollupManager).deactivateEmergencyState()).to.emit( - sovereignChainBridgeContract, - "EmergencyStateDeactivated" - ); - - expect(await sovereignChainBridgeContract.isEmergencyState()).to.be.equal(false); - }); - it("should SovereignChain bridge asset and verify merkle proof", async () => { const depositCount = await sovereignChainBridgeContract.depositCount(); const originNetwork = networkIDRollup2; @@ -699,8 +685,8 @@ describe("SovereignChainBridge Gas tokens tests", () => { } const rootRollup = merkleTreeRollup.getRoot(); // check only rollup account with update rollup exit root - await expect(sovereignChainGlobalExitRoot.updateExitRoot(rootRollup)).to.be.revertedWithCustomError( - sovereignChainGlobalExitRoot, + await expect(sovereignChainGlobalExitRootContract.updateExitRoot(rootRollup)).to.be.revertedWithCustomError( + sovereignChainGlobalExitRootContract, "OnlyAllowedContracts" ); @@ -708,10 +694,10 @@ describe("SovereignChainBridge Gas tokens tests", () => { await ethers.provider.send("hardhat_impersonateAccount", [sovereignChainBridgeContract.target]); const bridgeMock = await ethers.getSigner(sovereignChainBridgeContract.target as any); - await sovereignChainGlobalExitRoot.connect(bridgeMock).updateExitRoot(rootRollup, {gasPrice: 0}); + await sovereignChainGlobalExitRootContract.connect(bridgeMock).updateExitRoot(rootRollup, {gasPrice: 0}); // check roots - const sovereignChainExitRootSC = await sovereignChainGlobalExitRoot.lastRollupExitRoot(); + const sovereignChainExitRootSC = await sovereignChainGlobalExitRootContract.lastRollupExitRoot(); expect(sovereignChainExitRootSC).to.be.equal(rootRollup); const mainnetExitRootSC = ethers.ZeroHash; expect(mainnetExitRootSC).to.be.equal(mainnetExitRoot); @@ -719,8 +705,8 @@ describe("SovereignChainBridge Gas tokens tests", () => { const computedGlobalExitRoot = calculateGlobalExitRoot(mainnetExitRoot, rootRollup); // Insert global exit root - expect(await sovereignChainGlobalExitRoot.insertGlobalExitRoot(computedGlobalExitRoot)) - .to.emit(sovereignChainGlobalExitRoot, "InsertGlobalExitRoot") + expect(await sovereignChainGlobalExitRootContract.insertGlobalExitRoot(computedGlobalExitRoot)) + .to.emit(sovereignChainGlobalExitRootContract, "InsertGlobalExitRoot") .withArgs(computedGlobalExitRoot); // check merkle proof @@ -844,25 +830,25 @@ describe("SovereignChainBridge Gas tokens tests", () => { const rootRollup = merkleTreeRollup.getRoot(); // check only rollup account with update rollup exit root - await expect(sovereignChainGlobalExitRoot.updateExitRoot(rootRollup)).to.be.revertedWithCustomError( - sovereignChainGlobalExitRoot, + await expect(sovereignChainGlobalExitRootContract.updateExitRoot(rootRollup)).to.be.revertedWithCustomError( + sovereignChainGlobalExitRootContract, "OnlyAllowedContracts" ); // add rollup Merkle root await ethers.provider.send("hardhat_impersonateAccount", [sovereignChainBridgeContract.target]); const bridgeMock = await ethers.getSigner(sovereignChainBridgeContract.target as any); - await sovereignChainGlobalExitRoot.connect(bridgeMock).updateExitRoot(rootRollup, {gasPrice: 0}); + await sovereignChainGlobalExitRootContract.connect(bridgeMock).updateExitRoot(rootRollup, {gasPrice: 0}); // check roots - const rollupExitRootSC = await sovereignChainGlobalExitRoot.lastRollupExitRoot(); + const rollupExitRootSC = await sovereignChainGlobalExitRootContract.lastRollupExitRoot(); expect(rollupExitRootSC).to.be.equal(rootRollup); const computedGlobalExitRoot = calculateGlobalExitRoot(mainnetExitRoot, rollupExitRootSC); // Insert global exit root - expect(await sovereignChainGlobalExitRoot.insertGlobalExitRoot(computedGlobalExitRoot)) - .to.emit(sovereignChainGlobalExitRoot, "InsertGlobalExitRoot") + expect(await sovereignChainGlobalExitRootContract.insertGlobalExitRoot(computedGlobalExitRoot)) + .to.emit(sovereignChainGlobalExitRootContract, "InsertGlobalExitRoot") .withArgs(computedGlobalExitRoot); // check merkle proof @@ -1001,7 +987,7 @@ describe("SovereignChainBridge Gas tokens tests", () => { const wrappedTokenAddress = newWrappedToken.target; const newDestinationNetwork = networkIDRollup; - const rollupExitRoot = await sovereignChainGlobalExitRoot.lastRollupExitRoot(); + const rollupExitRoot = await sovereignChainGlobalExitRootContract.lastRollupExitRoot(); // create a new deposit await expect(newWrappedToken.approve(sovereignChainBridgeContract.target, amount)) @@ -1244,16 +1230,16 @@ describe("SovereignChainBridge Gas tokens tests", () => { // add rollup Merkle root await ethers.provider.send("hardhat_impersonateAccount", [sovereignChainBridgeContract.target]); const bridgeMock = await ethers.getSigner(sovereignChainBridgeContract.target as any); - await sovereignChainGlobalExitRoot.connect(bridgeMock).updateExitRoot(rollupRoot, {gasPrice: 0}); + await sovereignChainGlobalExitRootContract.connect(bridgeMock).updateExitRoot(rollupRoot, {gasPrice: 0}); // check roots - const rollupExitRootSC = await sovereignChainGlobalExitRoot.lastRollupExitRoot(); + const rollupExitRootSC = await sovereignChainGlobalExitRootContract.lastRollupExitRoot(); expect(rollupExitRootSC).to.be.equal(rollupRoot); const computedGlobalExitRoot = calculateGlobalExitRoot(mainnetExitRoot, rollupExitRootSC); // Insert global exit root - expect(await sovereignChainGlobalExitRoot.insertGlobalExitRoot(computedGlobalExitRoot)) - .to.emit(sovereignChainGlobalExitRoot, "InsertGlobalExitRoot") + expect(await sovereignChainGlobalExitRootContract.insertGlobalExitRoot(computedGlobalExitRoot)) + .to.emit(sovereignChainGlobalExitRootContract, "InsertGlobalExitRoot") .withArgs(computedGlobalExitRoot); // check merkle proof @@ -1393,16 +1379,16 @@ describe("SovereignChainBridge Gas tokens tests", () => { // add rollup Merkle root await ethers.provider.send("hardhat_impersonateAccount", [sovereignChainBridgeContract.target]); const bridgeMock = await ethers.getSigner(sovereignChainBridgeContract.target as any); - await sovereignChainGlobalExitRoot.connect(bridgeMock).updateExitRoot(rollupRoot, {gasPrice: 0}); + await sovereignChainGlobalExitRootContract.connect(bridgeMock).updateExitRoot(rollupRoot, {gasPrice: 0}); // check roots - const rollupExitRootSC = await sovereignChainGlobalExitRoot.lastRollupExitRoot(); + const rollupExitRootSC = await sovereignChainGlobalExitRootContract.lastRollupExitRoot(); expect(rollupExitRootSC).to.be.equal(rollupRoot); const computedGlobalExitRoot = calculateGlobalExitRoot(mainnetExitRoot, rollupExitRootSC); // Insert global exit root - expect(await sovereignChainGlobalExitRoot.insertGlobalExitRoot(computedGlobalExitRoot)) - .to.emit(sovereignChainGlobalExitRoot, "InsertGlobalExitRoot") + expect(await sovereignChainGlobalExitRootContract.insertGlobalExitRoot(computedGlobalExitRoot)) + .to.emit(sovereignChainGlobalExitRootContract, "InsertGlobalExitRoot") .withArgs(computedGlobalExitRoot); // check merkle proof @@ -1496,17 +1482,17 @@ describe("SovereignChainBridge Gas tokens tests", () => { // add rollup Merkle root await ethers.provider.send("hardhat_impersonateAccount", [sovereignChainBridgeContract.target]); const bridgeMock = await ethers.getSigner(sovereignChainBridgeContract.target as any); - await sovereignChainGlobalExitRoot.connect(bridgeMock).updateExitRoot(rollupRoot, {gasPrice: 0}); + await sovereignChainGlobalExitRootContract.connect(bridgeMock).updateExitRoot(rollupRoot, {gasPrice: 0}); // check roots - const rollupExitRootSC = await sovereignChainGlobalExitRoot.lastRollupExitRoot(); + const rollupExitRootSC = await sovereignChainGlobalExitRootContract.lastRollupExitRoot(); expect(rollupExitRootSC).to.be.equal(rollupRoot); const computedGlobalExitRoot = calculateGlobalExitRoot(mainnetExitRoot, rollupExitRootSC); // Insert global exit root - expect(await sovereignChainGlobalExitRoot.insertGlobalExitRoot(computedGlobalExitRoot)) - .to.emit(sovereignChainGlobalExitRoot, "InsertGlobalExitRoot") + expect(await sovereignChainGlobalExitRootContract.insertGlobalExitRoot(computedGlobalExitRoot)) + .to.emit(sovereignChainGlobalExitRootContract, "InsertGlobalExitRoot") .withArgs(computedGlobalExitRoot); // check merkle proof const index = 0; diff --git a/test/contractsv2/BridgeL2SovereignChain.test.ts b/test/contractsv2/BridgeL2SovereignChain.test.ts index fcf46b73..c2167dac 100644 --- a/test/contractsv2/BridgeL2SovereignChain.test.ts +++ b/test/contractsv2/BridgeL2SovereignChain.test.ts @@ -63,12 +63,12 @@ describe("BridgeL2SovereignChain Contract", () => { })) as unknown as BridgeL2SovereignChain; // deploy global exit root manager - const PolygonZkEVMGlobalExitRootFactory = await ethers.getContractFactory( + const GlobalExitRootManagerL2SovereignChainFactory = await ethers.getContractFactory( "GlobalExitRootManagerL2SovereignChain" ); sovereignChainGlobalExitRootContract = (await upgrades.deployProxy( - PolygonZkEVMGlobalExitRootFactory, - [deployer.address], // Initializer params + GlobalExitRootManagerL2SovereignChainFactory, + [ethers.ZeroAddress], // Initializer params { initializer: "initialize", // initializer function name constructorArgs: [sovereignChainBridgeContract.target], // Constructor arguments @@ -185,6 +185,11 @@ describe("BridgeL2SovereignChain Contract", () => { await sovereignChainBridgeContract .connect(bridgeManager) .setSovereignTokenAddress(networkIDRollup1, polTokenContract.target, legacyToken.target, true); + await expect( + sovereignChainBridgeContract + .connect(bridgeManager) + .setSovereignTokenAddress(networkIDRollup1, polTokenContract.target, legacyToken.target, true) + ).to.revertedWithCustomError(sovereignChainBridgeContract, "TokenAlreadyMapped"); // Deploy token 2 const updatedToken = await tokenFactory.deploy(tokenName, tokenSymbol, deployer.address, iBalance); // Send legacy tokens to bridge @@ -339,7 +344,7 @@ describe("BridgeL2SovereignChain Contract", () => { // Try to insert global exit root with non coinbase await expect( sovereignChainGlobalExitRootContract.connect(acc1).insertGlobalExitRoot(computedGlobalExitRoot) - ).to.be.revertedWithCustomError(sovereignChainGlobalExitRootContract, "OnlyAggOracleOrCoinbase"); + ).to.be.revertedWithCustomError(sovereignChainGlobalExitRootContract, "OnlyGlobalExitRootUpdater"); // Insert global exit root expect(await sovereignChainGlobalExitRootContract.insertGlobalExitRoot(computedGlobalExitRoot)) @@ -384,12 +389,12 @@ describe("BridgeL2SovereignChain Contract", () => { // Remove unmapped sovereign token address, should revert await expect( - sovereignChainBridgeContract.connect(bridgeManager).removeSovereignTokenAddress(tokenAddress) - ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "TokenNotMapped"); + sovereignChainBridgeContract.connect(bridgeManager).removeLegacySovereignTokenAddress(tokenAddress) + ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "TokenNotRemapped"); // Remove not updated sovereign token address, should revert await expect( - sovereignChainBridgeContract.connect(bridgeManager).removeSovereignTokenAddress(sovereignToken.target) - ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "TokenNotMapped"); + sovereignChainBridgeContract.connect(bridgeManager).removeLegacySovereignTokenAddress(sovereignToken.target) + ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "TokenNotRemapped"); // Remove updated sovereign token address // Remap token a second time to support removal function @@ -401,9 +406,9 @@ describe("BridgeL2SovereignChain Contract", () => { .to.emit(sovereignChainBridgeContract, "SetSovereignTokenAddress") .withArgs(networkIDRollup, tokenAddress, sovereignToken2.target, true); await expect( - sovereignChainBridgeContract.connect(bridgeManager).removeSovereignTokenAddress(sovereignToken.target) + sovereignChainBridgeContract.connect(bridgeManager).removeLegacySovereignTokenAddress(sovereignToken.target) ) - .to.emit(sovereignChainBridgeContract, "RemoveSovereignTokenAddress") + .to.emit(sovereignChainBridgeContract, "RemoveLegacySovereignTokenAddress") .withArgs(sovereignToken.target); // Remap sovereign address with multiCall const originNetworksArray = [networkIDRollup, networkIDRollup]; @@ -424,6 +429,15 @@ describe("BridgeL2SovereignChain Contract", () => { .withArgs(networkIDRollup, tokenAddress, sovereignToken3.target, true) .to.emit(sovereignChainBridgeContract, "SetSovereignTokenAddress") .withArgs(networkIDRollup, tokenAddress2.target, sovereignToken4.target, false); + + await expect( + sovereignChainBridgeContract.connect(bridgeManager).setMultipleSovereignTokenAddress( + originNetworksArray, + [], // Different length + sovereignTokenAddressesArray, + isNotMintableArray + ) + ).to.revertedWithCustomError(sovereignChainBridgeContract, "InputArraysLengthMismatch"); }); it("should Sovereign Chain bridge a remapped asset mintable and verify merkle proof", async () => { @@ -586,6 +600,10 @@ describe("BridgeL2SovereignChain Contract", () => { await expect( sovereignChainBridgeContract.bridgeMessageWETH(networkIDMainnet, deployer.address, 0, true, "0x") ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "NativeTokenIsEther"); + + await expect( + sovereignChainBridgeContract.connect(bridgeManager).setSovereignWETHAddress(deployer.address, true) + ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "WETHRemappingNotSupportedOnGasTokenNetworks"); }); it("should Sovereign Chain bridge asset and verify merkle proof", async () => { @@ -2131,71 +2149,12 @@ describe("BridgeL2SovereignChain Contract", () => { it("should test emergency state", async () => { await expect(sovereignChainBridgeContract.activateEmergencyState()).to.be.revertedWithCustomError( sovereignChainBridgeContract, - "OnlyRollupManager" + "NotValidBridgeManager" ); - await expect(sovereignChainBridgeContract.connect(rollupManager).activateEmergencyState()).to.emit( + await expect(sovereignChainBridgeContract.deactivateEmergencyState()).to.be.revertedWithCustomError( sovereignChainBridgeContract, - "EmergencyStateActivated" + "NotValidBridgeManager" ); - - const tokenAddress = polTokenContract.target; - const amount = ethers.parseEther("10"); - const destinationNetwork = networkIDRollup; - const destinationAddress = deployer.address; - - const metadata = metadataToken; - - await expect( - sovereignChainBridgeContract.bridgeAsset( - destinationNetwork, - destinationAddress, - amount, - tokenAddress, - true, - "0x" - ) - ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "OnlyNotEmergencyState"); - - await expect( - sovereignChainBridgeContract.bridgeMessage(destinationNetwork, destinationAddress, true, "0x") - ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "OnlyNotEmergencyState"); - - await expect( - sovereignChainBridgeContract.bridgeMessageWETH(destinationNetwork, destinationAddress, amount, true, "0x") - ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "OnlyNotEmergencyState"); - - const mockMerkleProof = new Array(32).fill(ethers.ZeroHash) as any; - await expect( - sovereignChainBridgeContract.claimAsset( - mockMerkleProof, - mockMerkleProof, - ethers.ZeroHash, - ethers.ZeroHash, - ethers.ZeroHash, - 0, - tokenAddress, - destinationNetwork, - destinationAddress, - amount, - metadata - ) - ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "OnlyNotEmergencyState"); - - await expect( - sovereignChainBridgeContract.claimMessage( - mockMerkleProof, - mockMerkleProof, - ethers.ZeroHash, - ethers.ZeroHash, - ethers.ZeroHash, - 0, - tokenAddress, - destinationNetwork, - destinationAddress, - amount, - metadata - ) - ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "OnlyNotEmergencyState"); }); });