From 46e6f178e0675b2c1280058adb76d06e89412bb6 Mon Sep 17 00:00:00 2001 From: koloz Date: Thu, 9 Nov 2023 08:46:50 -0500 Subject: [PATCH 1/3] changed modifier to onlyGovernor and added tests --- ethereum/contracts/zksync/facets/Admin.sol | 2 +- .../unit/concrete/Admin/Authorization.t.sol | 21 +++++ .../unit/concrete/Admin/_Admin_Shared.t.sol | 84 +++++++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 ethereum/test/foundry/unit/concrete/Admin/Authorization.t.sol create mode 100644 ethereum/test/foundry/unit/concrete/Admin/_Admin_Shared.t.sol diff --git a/ethereum/contracts/zksync/facets/Admin.sol b/ethereum/contracts/zksync/facets/Admin.sol index 9177afb71..0990a13cf 100644 --- a/ethereum/contracts/zksync/facets/Admin.sol +++ b/ethereum/contracts/zksync/facets/Admin.sol @@ -40,7 +40,7 @@ contract AdminFacet is Base, IAdmin { /// @notice Starts the transfer of admin rights. Only the current governor or admin can propose a new pending one. /// @notice New admin can accept admin rights by calling `acceptAdmin` function. /// @param _newPendingAdmin Address of the new admin - function setPendingAdmin(address _newPendingAdmin) external onlyGovernorOrAdmin { + function setPendingAdmin(address _newPendingAdmin) external onlyGovernor { // Save previous value into the stack to put it into the event later address oldPendingAdmin = s.pendingAdmin; // Change pending admin diff --git a/ethereum/test/foundry/unit/concrete/Admin/Authorization.t.sol b/ethereum/test/foundry/unit/concrete/Admin/Authorization.t.sol new file mode 100644 index 000000000..025ef3345 --- /dev/null +++ b/ethereum/test/foundry/unit/concrete/Admin/Authorization.t.sol @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.20; + +import {AdminTest} from "./_Admin_Shared.t.sol"; + +contract AuthorizationTest is AdminTest { + function test_SetPendingAdmin_RevertWhen() public { + address newAdmin = address(0x1337); + vm.prank(owner); + vm.expectRevert(bytes.concat("1g")); + proxyAsAdmin.setPendingAdmin(newAdmin); + } + + function test_SetPendingAdmin_RevertWhen_AdminNotGovernanceOwner() public { + address newAdmin = address(0x1337); + vm.prank(owner); + vm.expectRevert(bytes.concat("1g")); + proxyAsAdmin.setPendingAdmin(newAdmin); + } +} diff --git a/ethereum/test/foundry/unit/concrete/Admin/_Admin_Shared.t.sol b/ethereum/test/foundry/unit/concrete/Admin/_Admin_Shared.t.sol new file mode 100644 index 000000000..8c098b00f --- /dev/null +++ b/ethereum/test/foundry/unit/concrete/Admin/_Admin_Shared.t.sol @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.20; + +import {Test} from "forge-std/Test.sol"; +import {DiamondProxy} from "solpp/zksync/DiamondProxy.sol"; +import {DiamondInit} from "solpp/zksync/DiamondInit.sol"; +import {VerifierParams} from "solpp/zksync/Storage.sol"; +import {Diamond} from "solpp/zksync/libraries/Diamond.sol"; +import {AdminFacet} from "solpp/zksync/facets/Admin.sol"; +import {Governance} from "solpp/governance/Governance.sol"; + +contract AdminTest is Test { + DiamondProxy internal diamondProxy; + address internal owner; + address internal securityCouncil; + Governance internal governor; + AdminFacet internal adminFacet; + AdminFacet internal proxyAsAdmin; + + function getAdminSelectors() private view returns (bytes4[] memory) { + bytes4[] memory dcSelectors = new bytes4[](10); + dcSelectors[0] = adminFacet.setPendingGovernor.selector; + dcSelectors[1] = adminFacet.acceptGovernor.selector; + dcSelectors[2] = adminFacet.setPendingAdmin.selector; + dcSelectors[3] = adminFacet.acceptAdmin.selector; + dcSelectors[4] = adminFacet.setValidator.selector; + dcSelectors[5] = adminFacet.setPorterAvailability.selector; + dcSelectors[6] = adminFacet.setPriorityTxMaxGasLimit.selector; + dcSelectors[7] = adminFacet.executeUpgrade.selector; + dcSelectors[8] = adminFacet.freezeDiamond.selector; + dcSelectors[9] = adminFacet.unfreezeDiamond.selector; + return dcSelectors; + } + + function setUp() public { + owner = makeAddr("owner"); + securityCouncil = makeAddr("securityCouncil"); + governor = new Governance(owner, securityCouncil, 10_000); + DiamondInit diamondInit = new DiamondInit(); + + VerifierParams memory dummyVerifierParams = VerifierParams({ + recursionNodeLevelVkHash: 0, + recursionLeafLevelVkHash: 0, + recursionCircuitsSetVksHash: 0 + }); + + adminFacet = new AdminFacet(); + + bytes memory diamondInitCalldata = abi.encodeWithSelector( + diamondInit.initialize.selector, + 0x03752D8252d67f99888E741E3fB642803B29B155, + governor, + owner, + 0x02c775f0a90abf7a0e8043f2fdc38f0580ca9f9996a895d05a501bfeaa3b2e21, + 0, + 0x0000000000000000000000000000000000000000000000000000000000000000, + 0x70a0F165d6f8054d0d0CF8dFd4DD2005f0AF6B55, + dummyVerifierParams, + false, + 0x0100000000000000000000000000000000000000000000000000000000000000, + 0x0100000000000000000000000000000000000000000000000000000000000000, + 500000 // priority tx max L2 gas limit + ); + + Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](1); + facetCuts[0] = Diamond.FacetCut({ + facet: address(adminFacet), + action: Diamond.Action.Add, + isFreezable: false, + selectors: getAdminSelectors() + }); + + emit log_uint(facetCuts[0].selectors.length); + + Diamond.DiamondCutData memory diamondCutData = Diamond.DiamondCutData({ + facetCuts: facetCuts, + initAddress: address(diamondInit), + initCalldata: diamondInitCalldata + }); + + diamondProxy = new DiamondProxy(block.chainid, diamondCutData); + proxyAsAdmin = AdminFacet(address(diamondProxy)); + } +} From dc7998cd378f6491a25dd3e075c674dbd9982c3c Mon Sep 17 00:00:00 2001 From: koloz Date: Thu, 9 Nov 2023 09:08:18 -0500 Subject: [PATCH 2/3] removed redundant test --- .../test/foundry/unit/concrete/Admin/Authorization.t.sol | 7 ------- 1 file changed, 7 deletions(-) diff --git a/ethereum/test/foundry/unit/concrete/Admin/Authorization.t.sol b/ethereum/test/foundry/unit/concrete/Admin/Authorization.t.sol index 025ef3345..f390a62d4 100644 --- a/ethereum/test/foundry/unit/concrete/Admin/Authorization.t.sol +++ b/ethereum/test/foundry/unit/concrete/Admin/Authorization.t.sol @@ -5,13 +5,6 @@ pragma solidity 0.8.20; import {AdminTest} from "./_Admin_Shared.t.sol"; contract AuthorizationTest is AdminTest { - function test_SetPendingAdmin_RevertWhen() public { - address newAdmin = address(0x1337); - vm.prank(owner); - vm.expectRevert(bytes.concat("1g")); - proxyAsAdmin.setPendingAdmin(newAdmin); - } - function test_SetPendingAdmin_RevertWhen_AdminNotGovernanceOwner() public { address newAdmin = address(0x1337); vm.prank(owner); From e42aa54efb367e62b97c5cbe5c028a29de270ec0 Mon Sep 17 00:00:00 2001 From: koloz Date: Thu, 9 Nov 2023 09:34:40 -0500 Subject: [PATCH 3/3] updated test param --- .../test/foundry/unit/concrete/Admin/_Admin_Shared.t.sol | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/ethereum/test/foundry/unit/concrete/Admin/_Admin_Shared.t.sol b/ethereum/test/foundry/unit/concrete/Admin/_Admin_Shared.t.sol index 8c098b00f..a632f77b6 100644 --- a/ethereum/test/foundry/unit/concrete/Admin/_Admin_Shared.t.sol +++ b/ethereum/test/foundry/unit/concrete/Admin/_Admin_Shared.t.sol @@ -13,7 +13,7 @@ contract AdminTest is Test { DiamondProxy internal diamondProxy; address internal owner; address internal securityCouncil; - Governance internal governor; + address internal governor; AdminFacet internal adminFacet; AdminFacet internal proxyAsAdmin; @@ -35,7 +35,7 @@ contract AdminTest is Test { function setUp() public { owner = makeAddr("owner"); securityCouncil = makeAddr("securityCouncil"); - governor = new Governance(owner, securityCouncil, 10_000); + governor = makeAddr("governor"); DiamondInit diamondInit = new DiamondInit(); VerifierParams memory dummyVerifierParams = VerifierParams({ @@ -59,7 +59,8 @@ contract AdminTest is Test { false, 0x0100000000000000000000000000000000000000000000000000000000000000, 0x0100000000000000000000000000000000000000000000000000000000000000, - 500000 // priority tx max L2 gas limit + 500000, // priority tx max L2 gas limit + 0 ); Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](1); @@ -70,8 +71,6 @@ contract AdminTest is Test { selectors: getAdminSelectors() }); - emit log_uint(facetCuts[0].selectors.length); - Diamond.DiamondCutData memory diamondCutData = Diamond.DiamondCutData({ facetCuts: facetCuts, initAddress: address(diamondInit),