From 6d963351e9c9c722f5203bf20c483256eb72f270 Mon Sep 17 00:00:00 2001 From: mike Date: Mon, 11 Nov 2024 10:05:54 +0100 Subject: [PATCH] Make NodeDriverAuth upgradable --- contracts/sfc/ConstantsManager.sol | 4 ++-- contracts/sfc/NetworkInitializer.sol | 2 +- contracts/sfc/NodeDriver.sol | 12 +---------- contracts/sfc/NodeDriverAuth.sol | 14 ++++++------ contracts/sfc/SFC.sol | 3 +-- contracts/test/UnitTestSFC.sol | 2 +- test/NodeDriver.ts | 32 ++++++---------------------- test/SFC.ts | 8 +++++-- 8 files changed, 26 insertions(+), 51 deletions(-) diff --git a/contracts/sfc/ConstantsManager.sol b/contracts/sfc/ConstantsManager.sol index 56be7aa..8df41e3 100644 --- a/contracts/sfc/ConstantsManager.sol +++ b/contracts/sfc/ConstantsManager.sol @@ -47,8 +47,8 @@ contract ConstantsManager is OwnableUpgradeable { */ error ValueTooLarge(); - function initialize() external initializer { - __Ownable_init(msg.sender); + function initialize(address owner) external initializer { + __Ownable_init(owner); } function updateMinSelfStake(uint256 v) external virtual onlyOwner { diff --git a/contracts/sfc/NetworkInitializer.sol b/contracts/sfc/NetworkInitializer.sol index c6333ab..a070531 100644 --- a/contracts/sfc/NetworkInitializer.sol +++ b/contracts/sfc/NetworkInitializer.sol @@ -25,7 +25,7 @@ contract NetworkInitializer { NodeDriverAuth(_auth).initialize(_sfc, _driver, _owner); ConstantsManager consts = new ConstantsManager(); - consts.initialize(); + consts.initialize(address(this)); consts.updateMinSelfStake(500000 * 1e18); consts.updateMaxDelegatedRatio(16 * Decimal.unit()); consts.updateValidatorCommission((15 * Decimal.unit()) / 100); diff --git a/contracts/sfc/NodeDriver.sol b/contracts/sfc/NodeDriver.sol index 845578c..bc1287d 100644 --- a/contracts/sfc/NodeDriver.sol +++ b/contracts/sfc/NodeDriver.sol @@ -2,7 +2,6 @@ pragma solidity 0.8.27; import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; -import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; import {NodeDriverAuth} from "./NodeDriverAuth.sol"; import {IEVMWriter} from "../interfaces/IEVMWriter.sol"; @@ -14,21 +13,13 @@ import {INodeDriver} from "../interfaces/INodeDriver.sol"; * @dev Methods with onlyNode modifier are called by Sonic internal txs during epoch sealing. * @custom:security-contact security@fantom.foundation */ -contract NodeDriver is Initializable, OwnableUpgradeable, UUPSUpgradeable, INodeDriver { +contract NodeDriver is OwnableUpgradeable, UUPSUpgradeable, INodeDriver { NodeDriverAuth internal backend; IEVMWriter internal evmWriter; error NotNode(); error NotBackend(); - event UpdatedBackend(address indexed backend); - - /// NodeDriverAuth can replace itself - function setBackend(address _backend) external onlyBackend { - emit UpdatedBackend(_backend); - backend = NodeDriverAuth(_backend); - } - /// Callable only by NodeDriverAuth (which mediates calls from SFC and from admins) modifier onlyBackend() { if (msg.sender != address(backend)) { @@ -50,7 +41,6 @@ contract NodeDriver is Initializable, OwnableUpgradeable, UUPSUpgradeable, INode __Ownable_init(_owner); __UUPSUpgradeable_init(); backend = NodeDriverAuth(_backend); - emit UpdatedBackend(_backend); evmWriter = IEVMWriter(_evmWriterAddress); } diff --git a/contracts/sfc/NodeDriverAuth.sol b/contracts/sfc/NodeDriverAuth.sol index 6d3ed6f..961dda6 100644 --- a/contracts/sfc/NodeDriverAuth.sol +++ b/contracts/sfc/NodeDriverAuth.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.27; -import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; import {ISFC} from "../interfaces/ISFC.sol"; import {NodeDriver} from "./NodeDriver.sol"; @@ -10,7 +10,7 @@ import {INodeDriverExecutable} from "../interfaces/INodeDriverExecutable.sol"; /** * @custom:security-contact security@fantom.foundation */ -contract NodeDriverAuth is Initializable, OwnableUpgradeable { +contract NodeDriverAuth is OwnableUpgradeable, UUPSUpgradeable { ISFC internal sfc; NodeDriver internal driver; @@ -24,10 +24,15 @@ contract NodeDriverAuth is Initializable, OwnableUpgradeable { // Initialize NodeDriverAuth, NodeDriver and SFC in one call to allow fewer genesis transactions function initialize(address payable _sfc, address _driver, address _owner) external initializer { __Ownable_init(_owner); + __UUPSUpgradeable_init(); driver = NodeDriver(_driver); sfc = ISFC(_sfc); } + /// Override the upgrade authorization check to allow upgrades only from the owner. + // solhint-disable-next-line no-empty-blocks + function _authorizeUpgrade(address) internal override onlyOwner {} + /// Callable only by SFC contract. modifier onlySFC() { if (msg.sender != address(sfc)) { @@ -44,11 +49,6 @@ contract NodeDriverAuth is Initializable, OwnableUpgradeable { _; } - /// Change NodeDriverAuth used by NodeDriver. Callable by network admin. - function migrateTo(address newDriverAuth) external onlyOwner { - driver.setBackend(newDriverAuth); - } - function _execute(address executable, address newOwner, bytes32 selfCodeHash, bytes32 driverCodeHash) internal { _transferOwnership(executable); INodeDriverExecutable(executable).execute(); diff --git a/contracts/sfc/SFC.sol b/contracts/sfc/SFC.sol index 27ad8c3..ba6db45 100644 --- a/contracts/sfc/SFC.sol +++ b/contracts/sfc/SFC.sol @@ -3,7 +3,6 @@ pragma solidity 0.8.27; import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; -import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import {Decimal} from "../common/Decimal.sol"; import {NodeDriverAuth} from "./NodeDriverAuth.sol"; import {ConstantsManager} from "./ConstantsManager.sol"; @@ -14,7 +13,7 @@ import {Version} from "../version/Version.sol"; * @notice The SFC maintains a list of validators and delegators and distributes rewards to them. * @custom:security-contact security@fantom.foundation */ -contract SFC is Initializable, OwnableUpgradeable, UUPSUpgradeable, Version { +contract SFC is OwnableUpgradeable, UUPSUpgradeable, Version { uint256 internal constant OK_STATUS = 0; uint256 internal constant WITHDRAWN_BIT = 1; uint256 internal constant OFFLINE_BIT = 1 << 3; diff --git a/contracts/test/UnitTestSFC.sol b/contracts/test/UnitTestSFC.sol index d935252..1293827 100644 --- a/contracts/test/UnitTestSFC.sol +++ b/contracts/test/UnitTestSFC.sol @@ -62,7 +62,7 @@ contract UnitTestNetworkInitializer { NodeDriverAuth(_auth).initialize(_sfc, _driver, _owner); UnitTestConstantsManager consts = new UnitTestConstantsManager(); - consts.initialize(); + consts.initialize(address(this)); consts.updateMinSelfStake(0.3175000 * 1e18); consts.updateMaxDelegatedRatio(16 * Decimal.unit()); consts.updateValidatorCommission((15 * Decimal.unit()) / 100); diff --git a/test/NodeDriver.ts b/test/NodeDriver.ts index ccad1aa..aec7e35 100644 --- a/test/NodeDriver.ts +++ b/test/NodeDriver.ts @@ -1,7 +1,7 @@ import { ethers, upgrades } from 'hardhat'; import { expect } from 'chai'; import { loadFixture } from '@nomicfoundation/hardhat-network-helpers'; -import { IEVMWriter, NetworkInitializer, NodeDriverAuth } from '../typechain-types'; +import { IEVMWriter, NetworkInitializer } from '../typechain-types'; describe('NodeDriver', () => { const fixture = async () => { @@ -14,9 +14,13 @@ describe('NodeDriver', () => { kind: 'uups', initializer: false, }); - const evmWriter: IEVMWriter = await ethers.deployContract('StubEvmWriter'); - const nodeDriverAuth: NodeDriverAuth = await ethers.deployContract('NodeDriverAuth'); + const nodeDriverAuth = await upgrades.deployProxy(await ethers.getContractFactory('NodeDriverAuth'), { + kind: 'uups', + initializer: false, + }); + const initializer: NetworkInitializer = await ethers.deployContract('NetworkInitializer'); + const evmWriter: IEVMWriter = await ethers.deployContract('StubEvmWriter'); await initializer.initializeAll(12, 0, sfc, nodeDriverAuth, nodeDriver, evmWriter, owner); @@ -34,21 +38,6 @@ describe('NodeDriver', () => { Object.assign(this, await loadFixture(fixture)); }); - describe('Migrate', () => { - it('Should succeed and migrate to a new address', async function () { - const account = ethers.Wallet.createRandom(); - await this.nodeDriverAuth.migrateTo(account); - }); - - it('Should revert when not owner', async function () { - const account = ethers.Wallet.createRandom(); - await expect(this.nodeDriverAuth.connect(this.nonOwner).migrateTo(account)).to.be.revertedWithCustomError( - this.nodeDriverAuth, - 'OwnableUnauthorizedAccount', - ); - }); - }); - describe('Copy code', () => { it('Should succeed and copy code', async function () { const account = ethers.Wallet.createRandom(); @@ -103,13 +92,6 @@ describe('NodeDriver', () => { }); }); - describe('Set backend', () => { - it('Should revert when not backend', async function () { - const account = ethers.Wallet.createRandom(); - await expect(this.nodeDriver.setBackend(account)).to.be.revertedWithCustomError(this.nodeDriver, 'NotBackend'); - }); - }); - describe('Swap code', () => { it('Should revert when not backend', async function () { const account = ethers.Wallet.createRandom(); diff --git a/test/SFC.ts b/test/SFC.ts index 917ca28..87bbe3e 100644 --- a/test/SFC.ts +++ b/test/SFC.ts @@ -1,7 +1,7 @@ import { ethers, upgrades } from 'hardhat'; import { expect } from 'chai'; import { loadFixture } from '@nomicfoundation/hardhat-network-helpers'; -import { IEVMWriter, NodeDriverAuth, UnitTestConstantsManager, UnitTestNetworkInitializer } from '../typechain-types'; +import { IEVMWriter, UnitTestConstantsManager, UnitTestNetworkInitializer } from '../typechain-types'; import { beforeEach, Context } from 'mocha'; import { BlockchainNode, ValidatorMetrics } from './helpers/BlockchainNode'; @@ -16,8 +16,12 @@ describe('SFC', () => { kind: 'uups', initializer: false, }); + const nodeDriverAuth = await upgrades.deployProxy(await ethers.getContractFactory('NodeDriverAuth'), { + kind: 'uups', + initializer: false, + }); + const evmWriter: IEVMWriter = await ethers.deployContract('StubEvmWriter'); - const nodeDriverAuth: NodeDriverAuth = await ethers.deployContract('NodeDriverAuth'); const initializer: UnitTestNetworkInitializer = await ethers.deployContract('UnitTestNetworkInitializer'); await initializer.initializeAll(0, 0, sfc, nodeDriverAuth, nodeDriver, evmWriter, owner);