Skip to content

Commit

Permalink
remove admin and use governance owner as admin instead
Browse files Browse the repository at this point in the history
  • Loading branch information
koloz193 committed Nov 3, 2023
1 parent 5772ecd commit 36fe0ca
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 55 deletions.
4 changes: 0 additions & 4 deletions ethereum/contracts/zksync/DiamondInit.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ contract DiamondInit is Base {
/// @dev We use struct instead of raw parameters in `initialize` function to prevent "Stack too deep" error
/// @param _verifier address of Verifier contract
/// @param _governor address who can manage critical updates in the contract
/// @param _admin address who can manage non-critical updates in the contract
/// @param _genesisBatchHash Batch hash of the genesis (initial) batch
/// @param _genesisIndexRepeatedStorageChanges The serial number of the shortcut storage key for genesis batch
/// @param _genesisBatchCommitment The zk-proof commitment for the genesis batch
Expand All @@ -34,7 +33,6 @@ contract DiamondInit is Base {
struct InitializeData {
IVerifier verifier;
address governor;
address admin;
bytes32 genesisBatchHash;
uint64 genesisIndexRepeatedStorageChanges;
bytes32 genesisBatchCommitment;
Expand All @@ -55,12 +53,10 @@ contract DiamondInit is Base {
function initialize(InitializeData calldata _initalizeData) external reentrancyGuardInitializer returns (bytes32) {
require(address(_initalizeData.verifier) != address(0), "vt");
require(_initalizeData.governor != address(0), "vy");
require(_initalizeData.admin != address(0), "hc");
require(_initalizeData.priorityTxMaxGasLimit <= L2_TX_MAX_GAS_LIMIT, "vu");

s.verifier = _initalizeData.verifier;
s.governor = _initalizeData.governor;
s.admin = _initalizeData.admin;

// We need to initialize the state hash because it is used in the commitment of the next batch
IExecutor.StoredBatchInfo memory storedBatchZero = IExecutor.StoredBatchInfo(
Expand Down
2 changes: 1 addition & 1 deletion ethereum/contracts/zksync/Storage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ struct AppStorage {
/// yet.
uint256 l2SystemContractsUpgradeBatchNumber;
/// @dev Address which will exercise non-critical changes to the Diamond Proxy (changing validator set & unfreezing)
address admin;
address __DEPRECATED_admin;
/// @notice Address that the governor or admin proposed as one that will replace admin role
address pendingAdmin;
}
25 changes: 1 addition & 24 deletions ethereum/contracts/zksync/facets/Admin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import "../interfaces/IAdmin.sol";
import "../libraries/Diamond.sol";
import {L2_TX_MAX_GAS_LIMIT} from "../Config.sol";
import "./Base.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";

/// @title Admin Contract controls access rights for contract management.
/// @author Matter Labs
Expand Down Expand Up @@ -37,30 +38,6 @@ contract AdminFacet is Base, IAdmin {
emit NewGovernor(previousGovernor, pendingGovernor);
}

/// @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 {
// Save previous value into the stack to put it into the event later
address oldPendingAdmin = s.pendingAdmin;
// Change pending admin
s.pendingAdmin = _newPendingAdmin;
emit NewPendingAdmin(oldPendingAdmin, _newPendingAdmin);
}

/// @notice Accepts transfer of admin rights. Only pending admin can accept the role.
function acceptAdmin() external {
address pendingAdmin = s.pendingAdmin;
require(msg.sender == pendingAdmin, "n4"); // Only proposed by current admin address can claim the admin rights

address previousAdmin = s.admin;
s.admin = pendingAdmin;
delete s.pendingAdmin;

emit NewPendingAdmin(pendingAdmin, address(0));
emit NewAdmin(previousAdmin, pendingAdmin);
}

/// @notice Change validator status (active or not active)
/// @param _validator Validator address
/// @param _active Active flag
Expand Down
10 changes: 9 additions & 1 deletion ethereum/contracts/zksync/facets/Base.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,15 @@ contract Base is ReentrancyGuard, AllowListed {

/// @notice Checks that the message sender is an active governor or admin
modifier onlyGovernorOrAdmin() {
require(msg.sender == s.governor || msg.sender == s.admin, "Only by governor or admin");
if (s.governor.code.length > 0) {
try Ownable(s.governor).owner() returns (address admin) {
require(msg.sender == s.governor || msg.sender == admin, "Only by governor or admin");
} catch {
require(msg.sender == s.governor, "Only by governor or admin");
}
} else {
require(msg.sender == s.governor, "Only by governor or admin");
}
_;
}

Expand Down
4 changes: 0 additions & 4 deletions ethereum/contracts/zksync/interfaces/IAdmin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ interface IAdmin is IBase {

function acceptGovernor() external;

function setPendingAdmin(address _newPendingAdmin) external;

function acceptAdmin() external;

function setValidator(address _validator, bool _active) external;

function setPorterAvailability(bool _zkPorterIsAvailable) external;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ contract L1WethBridgeTest is Test {
diamondInit.initialize.selector,
dummyAddress,
owner,
owner,
0,
0,
0,
Expand Down
17 changes: 7 additions & 10 deletions ethereum/test/foundry/unit/concrete/DiamondCut/UpgradeLogic.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,15 @@ contract UpgradeLogicTest is DiamondCutTest {
address private randomSigner;

function getAdminSelectors() private view returns (bytes4[] memory) {
bytes4[] memory dcSelectors = new bytes4[](10);
bytes4[] memory dcSelectors = new bytes4[](8);
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;
dcSelectors[2] = adminFacet.setValidator.selector;
dcSelectors[3] = adminFacet.setPorterAvailability.selector;
dcSelectors[4] = adminFacet.setPriorityTxMaxGasLimit.selector;
dcSelectors[5] = adminFacet.executeUpgrade.selector;
dcSelectors[6] = adminFacet.freezeDiamond.selector;
dcSelectors[7] = adminFacet.unfreezeDiamond.selector;
return dcSelectors;
}

Expand Down Expand Up @@ -72,7 +70,6 @@ contract UpgradeLogicTest is DiamondCutTest {
diamondInit.initialize.selector,
0x03752D8252d67f99888E741E3fB642803B29B155,
governor,
governor,
0x02c775f0a90abf7a0e8043f2fdc38f0580ca9f9996a895d05a501bfeaa3b2e21,
0,
0x0000000000000000000000000000000000000000000000000000000000000000,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,15 @@ contract ExecutorTest is Test {
IExecutor.ProofInput internal proofInput;

function getAdminSelectors() private view returns (bytes4[] memory) {
bytes4[] memory selectors = new bytes4[](10);
bytes4[] memory selectors = new bytes4[](8);
selectors[0] = admin.setPendingGovernor.selector;
selectors[1] = admin.acceptGovernor.selector;
selectors[2] = admin.setPendingAdmin.selector;
selectors[3] = admin.acceptAdmin.selector;
selectors[4] = admin.setValidator.selector;
selectors[5] = admin.setPorterAvailability.selector;
selectors[6] = admin.setPriorityTxMaxGasLimit.selector;
selectors[7] = admin.executeUpgrade.selector;
selectors[8] = admin.freezeDiamond.selector;
selectors[9] = admin.unfreezeDiamond.selector;
selectors[2] = admin.setValidator.selector;
selectors[3] = admin.setPorterAvailability.selector;
selectors[4] = admin.setPriorityTxMaxGasLimit.selector;
selectors[5] = admin.executeUpgrade.selector;
selectors[6] = admin.freezeDiamond.selector;
selectors[7] = admin.unfreezeDiamond.selector;
return selectors;
}

Expand Down Expand Up @@ -123,7 +121,6 @@ contract ExecutorTest is Test {
diamondInit.initialize.selector,
dummyAddress, //verifier
owner,
owner,
0,
0,
0,
Expand Down
1 change: 1 addition & 0 deletions ethereum/test/unit_tests/governance_test.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe("Admin facet tests", function () {
const revertReason = await getCallRevertReason(
adminFacetTest.connect(randomSigner).setValidator(validatorAddress, true)
);
console.log(revertReason);
expect(revertReason).equal("Only by governor or admin");
});

Expand Down

0 comments on commit 36fe0ca

Please sign in to comment.