From d3a9cd55c0dfc55c08b0e0b5aac38250cbae2855 Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Tue, 17 Dec 2024 16:28:25 +0100 Subject: [PATCH 01/15] feat: split VaultFactory into 2 contracts: Beacon and Factory --- .vscode/settings.json | 19 +++- contracts/0.8.25/vaults/StakingVault.sol | 28 ++++-- contracts/0.8.25/vaults/VaultFactory.sol | 48 ++++++---- contracts/0.8.25/vaults/VaultHub.sol | 17 +++- .../0.8.25/vaults/interfaces/IBeaconProxy.sol | 2 +- .../vaults/interfaces/IStakingVault.sol | 4 +- lib/proxy.ts | 8 +- .../StakingVault__HarnessForTestUpgrade.sol | 49 +++++----- .../VaultFactory__MockForDashboard.sol | 2 +- .../0.8.25/vaults/dashboard/dashboard.test.ts | 10 +- .../vaults/delegation/delegation.test.ts | 14 +-- .../VaultFactory__MockForStakingVault.sol | 2 +- .../staking-vault/staking-vault.test.ts | 14 +-- test/0.8.25/vaults/vaultFactory.test.ts | 91 +++++++++++++------ 14 files changed, 195 insertions(+), 113 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 833034955..ab5a80b11 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -4,5 +4,22 @@ "source.fixAll.eslint": "always" }, "solidity.defaultCompiler": "remote", - "cSpell.words": ["IETHRegistrarController", "sealables", "streccak", "TmplAppInstalled", "TmplDAOAndTokenDeployed"] + "cSpell.words": [ + "IETHRegistrarController", + "sealables", + "streccak", + "TmplAppInstalled", + "TmplDAOAndTokenDeployed" + ], + "wake.compiler.solc.remappings": [ + "@aragon/=node_modules/@aragon/", + "@openzeppelin/=node_modules/@openzeppelin/", + "ens/=node_modules/@aragon/os/contracts/lib/ens/", + "eth-gas-reporter/=node_modules/eth-gas-reporter/", + "hardhat/=node_modules/hardhat/", + "math/=node_modules/@aragon/os/contracts/lib/math/", + "misc/=node_modules/@aragon/os/contracts/lib/misc/", + "openzeppelin-solidity/=node_modules/openzeppelin-solidity/", + "token/=node_modules/@aragon/os/contracts/lib/token/" + ] } diff --git a/contracts/0.8.25/vaults/StakingVault.sol b/contracts/0.8.25/vaults/StakingVault.sol index 93c6e518f..faa3d28ac 100644 --- a/contracts/0.8.25/vaults/StakingVault.sol +++ b/contracts/0.8.25/vaults/StakingVault.sol @@ -84,6 +84,7 @@ contract StakingVault is IStakingVault, IBeaconProxy, BeaconChainDepositLogistic IStakingVault.Report report; uint128 locked; int128 inOutDelta; + address factory; address operator; } @@ -105,21 +106,21 @@ contract StakingVault is IStakingVault, IBeaconProxy, BeaconChainDepositLogistic _disableInitializers(); } - modifier onlyBeacon() { - if (msg.sender != getBeacon()) revert SenderNotBeacon(msg.sender, getBeacon()); - _; - } - /// @notice Initialize the contract storage explicitly. - /// The initialize function selector is not changed. For upgrades use `_params` variable + /// The initialize function selector is not changed. For upgrades use `_params` variable. + /// To /// + /// @param _factory the contract from which the vault was created /// @param _owner vault owner address /// @param _operator address of the account that can make deposits to the beacon chain /// @param _params the calldata for initialize contract after upgrades // solhint-disable-next-line no-unused-vars - function initialize(address _owner, address _operator, bytes calldata _params) external onlyBeacon initializer { + function initialize(address _factory, address _owner, address _operator, bytes calldata _params) external initializer { + VaultStorage storage $ = _getVaultStorage(); + __Ownable_init(_owner); - _getVaultStorage().operator = _operator; + $.operator = _operator; + $.factory = _factory; } /** @@ -170,10 +171,18 @@ contract StakingVault is IStakingVault, IBeaconProxy, BeaconChainDepositLogistic * @notice Returns the beacon proxy address that controls this contract's implementation * @return address The beacon proxy address */ - function getBeacon() public view returns (address) { + function beacon() public view returns (address) { return ERC1967Utils.getBeacon(); } + /** + * @notice Returns the factory proxy address + * @return address The factory address + */ + function factory() public view returns (address) { + return _getVaultStorage().factory; + } + /** * @notice Returns the valuation of the vault * @return uint256 total valuation in ETH @@ -389,5 +398,4 @@ contract StakingVault is IStakingVault, IBeaconProxy, BeaconChainDepositLogistic error Unbalanced(); error NotAuthorized(string operation, address sender); error LockedCannotDecreaseOutsideOfReport(uint256 currentlyLocked, uint256 attemptedLocked); - error SenderNotBeacon(address sender, address beacon); } diff --git a/contracts/0.8.25/vaults/VaultFactory.sol b/contracts/0.8.25/vaults/VaultFactory.sol index 568dc540a..788eccfc9 100644 --- a/contracts/0.8.25/vaults/VaultFactory.sol +++ b/contracts/0.8.25/vaults/VaultFactory.sol @@ -1,18 +1,20 @@ // SPDX-FileCopyrightText: 2024 Lido // SPDX-License-Identifier: GPL-3.0 -import {UpgradeableBeacon} from "@openzeppelin/contracts-v5.0.2/proxy/beacon/UpgradeableBeacon.sol"; +// See contracts/COMPILERS.md +pragma solidity 0.8.25; + import {BeaconProxy} from "@openzeppelin/contracts-v5.0.2/proxy/beacon/BeaconProxy.sol"; import {Clones} from "@openzeppelin/contracts-v5.0.2/proxy/Clones.sol"; import {IStakingVault} from "./interfaces/IStakingVault.sol"; -pragma solidity 0.8.25; - +/// @notice This interface is strictly intended for connecting to a specific Delegation interface and specific parameters interface IDelegation { struct InitialState { - uint256 managementFee; - uint256 performanceFee; + uint256 managementFeeBP; + uint256 performanceFeeBP; + address defaultAdmin; address manager; address operator; } @@ -34,51 +36,57 @@ interface IDelegation { function revokeRole(bytes32 role, address account) external; } -contract VaultFactory is UpgradeableBeacon { - address public immutable delegationImpl; +contract VaultFactory { + address public immutable BEACON; + address public immutable DELEGATION_IMPL; - /// @param _owner The address of the VaultFactory owner - /// @param _stakingVaultImpl The address of the StakingVault implementation + /// @param _beacon The address of the beacon contract /// @param _delegationImpl The address of the Delegation implementation constructor( - address _owner, - address _stakingVaultImpl, + address _beacon, address _delegationImpl - ) UpgradeableBeacon(_stakingVaultImpl, _owner) { + ) { + if (_beacon == address(0)) revert ZeroArgument("_beacon"); if (_delegationImpl == address(0)) revert ZeroArgument("_delegation"); - delegationImpl = _delegationImpl; + BEACON = _beacon; + DELEGATION_IMPL = _delegationImpl; } /// @notice Creates a new StakingVault and Delegation contracts /// @param _delegationInitialState The params of vault initialization /// @param _stakingVaultInitializerExtraParams The params of vault initialization - function createVault( + function createVaultWithDelegation( IDelegation.InitialState calldata _delegationInitialState, bytes calldata _stakingVaultInitializerExtraParams ) external returns (IStakingVault vault, IDelegation delegation) { if (_delegationInitialState.manager == address(0)) revert ZeroArgument("manager"); - vault = IStakingVault(address(new BeaconProxy(address(this), ""))); - delegation = IDelegation(Clones.clone(delegationImpl)); + vault = IStakingVault(address(new BeaconProxy(BEACON, ""))); + delegation = IDelegation(Clones.clone(DELEGATION_IMPL)); delegation.initialize(address(vault)); - delegation.grantRole(delegation.DEFAULT_ADMIN_ROLE(), msg.sender); + delegation.grantRole(delegation.DEFAULT_ADMIN_ROLE(), _delegationInitialState.defaultAdmin); delegation.grantRole(delegation.MANAGER_ROLE(), _delegationInitialState.manager); delegation.grantRole(delegation.OPERATOR_ROLE(), _delegationInitialState.operator); delegation.grantRole(delegation.MANAGER_ROLE(), address(this)); delegation.grantRole(delegation.OPERATOR_ROLE(), address(this)); - delegation.setManagementFee(_delegationInitialState.managementFee); - delegation.setPerformanceFee(_delegationInitialState.performanceFee); + delegation.setManagementFee(_delegationInitialState.managementFeeBP); + delegation.setPerformanceFee(_delegationInitialState.performanceFeeBP); //revoke roles from factory delegation.revokeRole(delegation.MANAGER_ROLE(), address(this)); delegation.revokeRole(delegation.OPERATOR_ROLE(), address(this)); delegation.revokeRole(delegation.DEFAULT_ADMIN_ROLE(), address(this)); - vault.initialize(address(delegation), _delegationInitialState.operator, _stakingVaultInitializerExtraParams); + vault.initialize( + address(this), + address(delegation), + _delegationInitialState.operator, + _stakingVaultInitializerExtraParams + ); emit VaultCreated(address(delegation), address(vault)); emit DelegationCreated(msg.sender, address(delegation)); diff --git a/contracts/0.8.25/vaults/VaultHub.sol b/contracts/0.8.25/vaults/VaultHub.sol index 1d6e82c02..67875a434 100644 --- a/contracts/0.8.25/vaults/VaultHub.sol +++ b/contracts/0.8.25/vaults/VaultHub.sol @@ -10,6 +10,8 @@ import {Math256} from "contracts/common/lib/Math256.sol"; import {ILido as StETH} from "contracts/0.8.25/interfaces/ILido.sol"; import {IBeacon} from "@openzeppelin/contracts-v5.0.2/proxy/beacon/IBeacon.sol"; import {IBeaconProxy} from "./interfaces/IBeaconProxy.sol"; +import {IStakingVault} from "./interfaces/IStakingVault.sol"; +import {VaultFactory} from "./VaultFactory.sol"; // TODO: rebalance gas compensation // TODO: unstructured storag and upgradability @@ -145,11 +147,17 @@ abstract contract VaultHub is AccessControlEnumerableUpgradeable { VaultHubStorage storage $ = _getVaultHubStorage(); - address factory = IBeaconProxy(address (_vault)).getBeacon(); - if (!$.vaultFactories[factory]) revert FactoryNotAllowed(factory); + { + address factory = IStakingVault(address (_vault)).factory(); + if (!$.vaultFactories[factory]) revert FactoryNotAllowed(factory); - address impl = IBeacon(factory).implementation(); - if (!$.vaultImpl[impl]) revert ImplNotAllowed(impl); + address vaultBeacon = IBeaconProxy(address (_vault)).beacon(); + address factoryBeacon = VaultFactory(factory).BEACON(); + if (factoryBeacon != vaultBeacon) revert BeaconNotAllowed(factoryBeacon, vaultBeacon); + + address impl = IBeacon(vaultBeacon).implementation(); + if (!$.vaultImpl[impl]) revert ImplNotAllowed(impl); + } if ($.vaultIndex[_vault] != 0) revert AlreadyConnected(address(_vault), $.vaultIndex[_vault]); if (vaultsCount() == MAX_VAULTS_COUNT) revert TooManyVaults(); @@ -485,4 +493,5 @@ abstract contract VaultHub is AccessControlEnumerableUpgradeable { error AlreadyExists(address addr); error FactoryNotAllowed(address beacon); error ImplNotAllowed(address impl); + error BeaconNotAllowed(address factoryBeacon, address vaultBeacon); } diff --git a/contracts/0.8.25/vaults/interfaces/IBeaconProxy.sol b/contracts/0.8.25/vaults/interfaces/IBeaconProxy.sol index a99ecde57..c49bf63c4 100644 --- a/contracts/0.8.25/vaults/interfaces/IBeaconProxy.sol +++ b/contracts/0.8.25/vaults/interfaces/IBeaconProxy.sol @@ -5,6 +5,6 @@ pragma solidity 0.8.25; interface IBeaconProxy { - function getBeacon() external view returns (address); + function beacon() external view returns (address); function version() external pure returns(uint64); } diff --git a/contracts/0.8.25/vaults/interfaces/IStakingVault.sol b/contracts/0.8.25/vaults/interfaces/IStakingVault.sol index 7378cd324..929d3d2e3 100644 --- a/contracts/0.8.25/vaults/interfaces/IStakingVault.sol +++ b/contracts/0.8.25/vaults/interfaces/IStakingVault.sol @@ -10,10 +10,12 @@ interface IStakingVault { int128 inOutDelta; } - function initialize(address owner, address operator, bytes calldata params) external; + function initialize(address factory, address owner, address operator, bytes calldata params) external; function vaultHub() external view returns (address); + function factory() external view returns (address); + function operator() external view returns (address); function latestReport() external view returns (Report memory); diff --git a/lib/proxy.ts b/lib/proxy.ts index 035d3b511..25b83fa48 100644 --- a/lib/proxy.ts +++ b/lib/proxy.ts @@ -49,18 +49,20 @@ interface CreateVaultResponse { export async function createVaultProxy( vaultFactory: VaultFactory, + _admin: HardhatEthersSigner, _owner: HardhatEthersSigner, _operator: HardhatEthersSigner, ): Promise { // Define the parameters for the struct const initializationParams: DelegationInitializationParamsStruct = { - managementFee: 100n, - performanceFee: 200n, + managementFeeBP: 100n, + performanceFeeBP: 200n, + defaultAdmin: await _admin.getAddress(), manager: await _owner.getAddress(), operator: await _operator.getAddress(), }; - const tx = await vaultFactory.connect(_owner).createVault(initializationParams, "0x"); + const tx = await vaultFactory.connect(_owner).createVaultWithDelegation(initializationParams, "0x"); // Get the receipt manually const receipt = (await tx.wait())!; diff --git a/test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol b/test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol index 27159f7d4..33294c2b2 100644 --- a/test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol +++ b/test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol @@ -17,12 +17,10 @@ import {BeaconChainDepositLogistics} from "contracts/0.8.25/vaults/BeaconChainDe contract StakingVault__HarnessForTestUpgrade is IBeaconProxy, BeaconChainDepositLogistics, OwnableUpgradeable { /// @custom:storage-location erc7201:StakingVault.Vault struct VaultStorage { - uint128 reportValuation; - int128 reportInOutDelta; - - uint256 locked; - int256 inOutDelta; - + IStakingVault.Report report; + uint128 locked; + int128 inOutDelta; + address factory; address operator; } @@ -42,22 +40,21 @@ contract StakingVault__HarnessForTestUpgrade is IBeaconProxy, BeaconChainDeposit vaultHub = VaultHub(_vaultHub); } - modifier onlyBeacon() { - if (msg.sender != getBeacon()) revert UnauthorizedSender(msg.sender); - _; - } - - /// @notice Initialize the contract storage explicitly. - /// @param _owner owner address that can TBD + /// @notice Initialize the contract storage explicitly. Only new contracts can be initialized here. + /// @param _factory the contract from which the vault was created + /// @param _owner owner address + /// @param _operator address of the account that can make deposits to the beacon chain /// @param _params the calldata for initialize contract after upgrades - function initialize(address _owner, address _operator, bytes calldata _params) external onlyBeacon reinitializer(_version) { + function initialize(address _factory, address _owner, address _operator, bytes calldata _params) external reinitializer(_version) { + VaultStorage storage $ = _getVaultStorage(); + if ($.factory != address(0)) { + revert VaultAlreadyInitialized(); + } + __StakingVault_init_v2(); __Ownable_init(_owner); - _getVaultStorage().operator = _operator; - } - - function operator() external view returns (address) { - return _getVaultStorage().operator; + $.factory = _factory; + $.operator = _operator; } function finalizeUpgrade_v2() public reinitializer(_version) { @@ -65,7 +62,7 @@ contract StakingVault__HarnessForTestUpgrade is IBeaconProxy, BeaconChainDeposit } event InitializedV2(); - function __StakingVault_init_v2() internal { + function __StakingVault_init_v2() internal onlyInitializing { emit InitializedV2(); } @@ -77,15 +74,19 @@ contract StakingVault__HarnessForTestUpgrade is IBeaconProxy, BeaconChainDeposit return _version; } - function getBeacon() public view returns (address) { + function beacon() public view returns (address) { return ERC1967Utils.getBeacon(); } + function factory() public view returns (address) { + return _getVaultStorage().factory; + } + function latestReport() external view returns (IStakingVault.Report memory) { VaultStorage storage $ = _getVaultStorage(); return IStakingVault.Report({ - valuation: $.reportValuation, - inOutDelta: $.reportInOutDelta + valuation: $.report.valuation, + inOutDelta: $.report.inOutDelta }); } @@ -96,5 +97,5 @@ contract StakingVault__HarnessForTestUpgrade is IBeaconProxy, BeaconChainDeposit } error ZeroArgument(string name); - error UnauthorizedSender(address sender); + error VaultAlreadyInitialized(); } diff --git a/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol b/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol index bdc9997d5..5eba3a01d 100644 --- a/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol +++ b/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol @@ -31,7 +31,7 @@ contract VaultFactory__MockForDashboard is UpgradeableBeacon { dashboard.grantRole(dashboard.DEFAULT_ADMIN_ROLE(), msg.sender); dashboard.revokeRole(dashboard.DEFAULT_ADMIN_ROLE(), address(this)); - vault.initialize(address(dashboard), _operator, ""); + vault.initialize(address(this), address(dashboard), _operator, ""); emit VaultCreated(address(dashboard), address(vault)); emit DashboardCreated(msg.sender, address(dashboard)); diff --git a/test/0.8.25/vaults/dashboard/dashboard.test.ts b/test/0.8.25/vaults/dashboard/dashboard.test.ts index 8faeb599a..999e81cdb 100644 --- a/test/0.8.25/vaults/dashboard/dashboard.test.ts +++ b/test/0.8.25/vaults/dashboard/dashboard.test.ts @@ -1,10 +1,10 @@ -import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers"; import { expect } from "chai"; import { randomBytes } from "crypto"; import { ZeroAddress } from "ethers"; import { ethers } from "hardhat"; -import { certainAddress, ether, findEvents } from "lib"; -import { Snapshot } from "test/suite"; + +import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers"; + import { Dashboard, DepositContract__MockForStakingVault, @@ -14,6 +14,10 @@ import { VaultHub__MockForDashboard, } from "typechain-types"; +import { certainAddress, ether, findEvents } from "lib"; + +import { Snapshot } from "test/suite"; + describe("Dashboard", () => { let factoryOwner: HardhatEthersSigner; let vaultOwner: HardhatEthersSigner; diff --git a/test/0.8.25/vaults/delegation/delegation.test.ts b/test/0.8.25/vaults/delegation/delegation.test.ts index ebd15dce6..3f99aaf54 100644 --- a/test/0.8.25/vaults/delegation/delegation.test.ts +++ b/test/0.8.25/vaults/delegation/delegation.test.ts @@ -1,9 +1,9 @@ -import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers"; import { expect } from "chai"; import { keccak256 } from "ethers"; import { ethers } from "hardhat"; -import { advanceChainTime, days, ether, findEvents, getNextBlockTimestamp, impersonate, streccak } from "lib"; -import { Snapshot } from "test/suite"; + +import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers"; + import { Delegation, DepositContract__MockForStakingVault, @@ -13,17 +13,17 @@ import { VaultHub__MockForDelegation, } from "typechain-types"; +import { advanceChainTime, days, ether, findEvents, getNextBlockTimestamp, impersonate } from "lib"; + +import { Snapshot } from "test/suite"; + const BP_BASE = 10000n; const MAX_FEE = BP_BASE; describe("Delegation", () => { - let deployer: HardhatEthersSigner; let vaultOwner: HardhatEthersSigner; let manager: HardhatEthersSigner; let operator: HardhatEthersSigner; - let staker: HardhatEthersSigner; - let keyMaster: HardhatEthersSigner; - let tokenMaster: HardhatEthersSigner; let stranger: HardhatEthersSigner; let factoryOwner: HardhatEthersSigner; let hubSigner: HardhatEthersSigner; diff --git a/test/0.8.25/vaults/staking-vault/contracts/VaultFactory__MockForStakingVault.sol b/test/0.8.25/vaults/staking-vault/contracts/VaultFactory__MockForStakingVault.sol index ad0796280..35b4e6768 100644 --- a/test/0.8.25/vaults/staking-vault/contracts/VaultFactory__MockForStakingVault.sol +++ b/test/0.8.25/vaults/staking-vault/contracts/VaultFactory__MockForStakingVault.sol @@ -14,7 +14,7 @@ contract VaultFactory__MockForStakingVault is UpgradeableBeacon { function createVault(address _owner, address _operator) external { IStakingVault vault = IStakingVault(address(new BeaconProxy(address(this), ""))); - vault.initialize(_owner, _operator, ""); + vault.initialize(address(this), _owner, _operator, ""); emit VaultCreated(address(vault)); } diff --git a/test/0.8.25/vaults/staking-vault/staking-vault.test.ts b/test/0.8.25/vaults/staking-vault/staking-vault.test.ts index c46d3adb6..582d0881b 100644 --- a/test/0.8.25/vaults/staking-vault/staking-vault.test.ts +++ b/test/0.8.25/vaults/staking-vault/staking-vault.test.ts @@ -3,7 +3,7 @@ import { ZeroAddress } from "ethers"; import { ethers } from "hardhat"; import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers"; -import { getStorageAt, setBalance } from "@nomicfoundation/hardhat-network-helpers"; +import { setBalance } from "@nomicfoundation/hardhat-network-helpers"; import { DepositContract__MockForStakingVault, @@ -59,7 +59,7 @@ describe("StakingVault", () => { stakingVaultAddress = await stakingVault.getAddress(); vaultHubAddress = await vaultHub.getAddress(); depositContractAddress = await depositContract.getAddress(); - beaconAddress = await stakingVaultImplementation.getBeacon(); + beaconAddress = await stakingVaultImplementation.beacon(); vaultFactoryAddress = await vaultFactory.getAddress(); ethRejectorAddress = await ethRejector.getAddress(); @@ -104,15 +104,9 @@ describe("StakingVault", () => { it("reverts on initialization", async () => { await expect( - stakingVaultImplementation.connect(beaconSigner).initialize(vaultOwner, operator, "0x"), + stakingVaultImplementation.connect(beaconSigner).initialize(vaultFactoryAddress, vaultOwner, operator, "0x"), ).to.be.revertedWithCustomError(stakingVaultImplementation, "InvalidInitialization"); }); - - it("reverts on initialization if the caller is not the beacon", async () => { - await expect(stakingVaultImplementation.connect(stranger).initialize(vaultOwner, operator, "0x")) - .to.be.revertedWithCustomError(stakingVaultImplementation, "SenderNotBeacon") - .withArgs(stranger, await stakingVaultImplementation.getBeacon()); - }); }); context("initial state", () => { @@ -122,7 +116,7 @@ describe("StakingVault", () => { expect(await stakingVault.VAULT_HUB()).to.equal(vaultHubAddress); expect(await stakingVault.vaultHub()).to.equal(vaultHubAddress); expect(await stakingVault.DEPOSIT_CONTRACT()).to.equal(depositContractAddress); - expect(await stakingVault.getBeacon()).to.equal(vaultFactoryAddress); + expect(await stakingVault.beacon()).to.equal(vaultFactoryAddress); expect(await stakingVault.owner()).to.equal(await vaultOwner.getAddress()); expect(await stakingVault.operator()).to.equal(operator); diff --git a/test/0.8.25/vaults/vaultFactory.test.ts b/test/0.8.25/vaults/vaultFactory.test.ts index f2441fca6..1be0e584a 100644 --- a/test/0.8.25/vaults/vaultFactory.test.ts +++ b/test/0.8.25/vaults/vaultFactory.test.ts @@ -13,6 +13,7 @@ import { StakingVault, StakingVault__HarnessForTestUpgrade, StETH__HarnessForVaultHub, + UpgradeableBeacon, VaultFactory, } from "typechain-types"; @@ -32,6 +33,7 @@ describe("VaultFactory.sol", () => { let depositContract: DepositContract__MockForBeaconChainDepositor; let proxy: OssifiableProxy; + let beacon: UpgradeableBeacon; let accountingImpl: Accounting; let accounting: Accounting; let implOld: StakingVault; @@ -63,51 +65,72 @@ describe("VaultFactory.sol", () => { accounting = await ethers.getContractAt("Accounting", proxy, deployer); await accounting.initialize(admin); + //vault implementation implOld = await ethers.deployContract("StakingVault", [accounting, depositContract], { from: deployer }); implNew = await ethers.deployContract("StakingVault__HarnessForTestUpgrade", [accounting, depositContract], { from: deployer, }); + + //beacon + beacon = await ethers.deployContract("UpgradeableBeacon", [implOld, admin]); + delegation = await ethers.deployContract("Delegation", [steth], { from: deployer }); - vaultFactory = await ethers.deployContract("VaultFactory", [admin, implOld, delegation], { from: deployer }); + vaultFactory = await ethers.deployContract("VaultFactory", [beacon, delegation], { from: deployer }); //add VAULT_MASTER_ROLE role to allow admin to connect the Vaults to the vault Hub await accounting.connect(admin).grantRole(await accounting.VAULT_MASTER_ROLE(), admin); //add VAULT_REGISTRY_ROLE role to allow admin to add factory and vault implementation to the hub await accounting.connect(admin).grantRole(await accounting.VAULT_REGISTRY_ROLE(), admin); + console.log({ + beaconAddress: await beacon.getAddress(), + delegationAddress: await delegation.getAddress(), + factoryAddress: await vaultFactory.getAddress(), + }); + //the initialize() function cannot be called on a contract - await expect(implOld.initialize(stranger, operator, "0x")).to.revertedWithCustomError(implOld, "SenderNotBeacon"); + await expect(implOld.initialize(admin, stranger, operator, "0x")).to.revertedWithCustomError( + implOld, + "InvalidInitialization", + ); }); beforeEach(async () => (originalState = await Snapshot.take())); afterEach(async () => await Snapshot.restore(originalState)); + context("beacon.constructor", () => {}); + context("constructor", () => { it("reverts if `_owner` is zero address", async () => { - await expect(ethers.deployContract("VaultFactory", [ZeroAddress, implOld, steth], { from: deployer })) - .to.be.revertedWithCustomError(vaultFactory, "OwnableInvalidOwner") + await expect(ethers.deployContract("UpgradeableBeacon", [ZeroAddress, admin], { from: deployer })) + .to.be.revertedWithCustomError(beacon, "BeaconInvalidImplementation") + .withArgs(ZeroAddress); + }); + it("reverts if `_owner` is zero address", async () => { + await expect(ethers.deployContract("UpgradeableBeacon", [implOld, ZeroAddress], { from: deployer })) + .to.be.revertedWithCustomError(beacon, "OwnableInvalidOwner") .withArgs(ZeroAddress); }); it("reverts if `_implementation` is zero address", async () => { - await expect(ethers.deployContract("VaultFactory", [admin, ZeroAddress, steth], { from: deployer })) - .to.be.revertedWithCustomError(vaultFactory, "BeaconInvalidImplementation") - .withArgs(ZeroAddress); + await expect(ethers.deployContract("VaultFactory", [ZeroAddress, steth], { from: deployer })) + .to.be.revertedWithCustomError(vaultFactory, "ZeroArgument") + .withArgs("_beacon"); }); it("reverts if `_delegation` is zero address", async () => { - await expect(ethers.deployContract("VaultFactory", [admin, implOld, ZeroAddress], { from: deployer })) + await expect(ethers.deployContract("VaultFactory", [beacon, ZeroAddress], { from: deployer })) .to.be.revertedWithCustomError(vaultFactory, "ZeroArgument") .withArgs("_delegation"); }); it("works and emit `OwnershipTransferred`, `Upgraded` events", async () => { - const beacon = await ethers.deployContract( - "VaultFactory", - [await admin.getAddress(), await implOld.getAddress(), await steth.getAddress()], - { from: deployer }, - ); + // const beacon = await ethers.deployContract( + // "VaultFactory", + // [await implOld.getAddress(), await steth.getAddress()], + // { from: deployer }, + // ); const tx = beacon.deploymentTransaction(); @@ -122,7 +145,7 @@ describe("VaultFactory.sol", () => { context("createVault", () => { it("works with empty `params`", async () => { - const { tx, vault, delegation: delegation_ } = await createVaultProxy(vaultFactory, vaultOwner1, operator); + const { tx, vault, delegation: delegation_ } = await createVaultProxy(vaultFactory, admin, vaultOwner1, operator); await expect(tx) .to.emit(vaultFactory, "VaultCreated") @@ -133,11 +156,12 @@ describe("VaultFactory.sol", () => { .withArgs(await vaultOwner1.getAddress(), await delegation_.getAddress()); expect(await delegation_.getAddress()).to.eq(await vault.owner()); - expect(await vault.getBeacon()).to.eq(await vaultFactory.getAddress()); + expect(await vault.beacon()).to.eq(await beacon.getAddress()); + expect(await vault.factory()).to.eq(await vaultFactory.getAddress()); }); it("check `version()`", async () => { - const { vault } = await createVaultProxy(vaultFactory, vaultOwner1, operator); + const { vault } = await createVaultProxy(vaultFactory, admin, vaultOwner1, operator); expect(await vault.version()).to.eq(1); }); @@ -145,6 +169,7 @@ describe("VaultFactory.sol", () => { }); context("connect", () => { + it("create vault ", async () => {}); it("connect ", async () => { const vaultsBefore = await accounting.vaultsCount(); expect(vaultsBefore).to.eq(0); @@ -163,14 +188,24 @@ describe("VaultFactory.sol", () => { }; //create vault - const { vault: vault1, delegation: delegator1 } = await createVaultProxy(vaultFactory, vaultOwner1, operator); - const { vault: vault2, delegation: delegator2 } = await createVaultProxy(vaultFactory, vaultOwner2, operator); + const { vault: vault1, delegation: delegator1 } = await createVaultProxy( + vaultFactory, + admin, + vaultOwner1, + operator, + ); + const { vault: vault2, delegation: delegator2 } = await createVaultProxy( + vaultFactory, + admin, + vaultOwner2, + operator, + ); //owner of vault is delegator expect(await delegator1.getAddress()).to.eq(await vault1.owner()); expect(await delegator2.getAddress()).to.eq(await vault2.owner()); - //try to connect vault without, factory not allowed + //try to connect vault without factory not allowed await expect( accounting .connect(admin) @@ -228,17 +263,17 @@ describe("VaultFactory.sol", () => { const version1Before = await vault1.version(); const version2Before = await vault2.version(); - const implBefore = await vaultFactory.implementation(); + const implBefore = await beacon.implementation(); expect(implBefore).to.eq(await implOld.getAddress()); //upgrade beacon to new implementation - await vaultFactory.connect(admin).upgradeTo(implNew); + await beacon.connect(admin).upgradeTo(implNew); - const implAfter = await vaultFactory.implementation(); + const implAfter = await beacon.implementation(); expect(implAfter).to.eq(await implNew.getAddress()); //create new vault with new implementation - const { vault: vault3 } = await createVaultProxy(vaultFactory, vaultOwner1, operator); + const { vault: vault3 } = await createVaultProxy(vaultFactory, admin, vaultOwner1, operator); //we upgrade implementation and do not add it to whitelist await expect( @@ -260,6 +295,12 @@ describe("VaultFactory.sol", () => { //finalize first vault await vault1WithNewImpl.finalizeUpgrade_v2(); + //try to initialize the second vault + await expect(vault2WithNewImpl.initialize(ZeroAddress, admin, operator, "0x")).to.revertedWithCustomError( + vault2WithNewImpl, + "VaultAlreadyInitialized", + ); + const version1After = await vault1WithNewImpl.version(); const version2After = await vault2WithNewImpl.version(); const version3After = await vault3WithNewImpl.version(); @@ -281,10 +322,6 @@ describe("VaultFactory.sol", () => { const v3 = { version: version3After, getInitializedVersion: version3AfterV2 }; console.table([v1, v2, v3]); - - // await vault1.initialize(stranger, "0x") - // await vault2.initialize(stranger, "0x") - // await vault3.initialize(stranger, "0x") }); }); }); From 3227ef62d7c9517cb5eb83df226aed25a549fc70 Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Tue, 17 Dec 2024 16:30:42 +0100 Subject: [PATCH 02/15] feat: remove vscode --- .vscode/settings.json | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index ab5a80b11..864f600b0 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -3,23 +3,5 @@ "editor.codeActionsOnSave": { "source.fixAll.eslint": "always" }, - "solidity.defaultCompiler": "remote", - "cSpell.words": [ - "IETHRegistrarController", - "sealables", - "streccak", - "TmplAppInstalled", - "TmplDAOAndTokenDeployed" - ], - "wake.compiler.solc.remappings": [ - "@aragon/=node_modules/@aragon/", - "@openzeppelin/=node_modules/@openzeppelin/", - "ens/=node_modules/@aragon/os/contracts/lib/ens/", - "eth-gas-reporter/=node_modules/eth-gas-reporter/", - "hardhat/=node_modules/hardhat/", - "math/=node_modules/@aragon/os/contracts/lib/math/", - "misc/=node_modules/@aragon/os/contracts/lib/misc/", - "openzeppelin-solidity/=node_modules/openzeppelin-solidity/", - "token/=node_modules/@aragon/os/contracts/lib/token/" - ] + "solidity.defaultCompiler": "remote" } From ad52a5a7af1b5c3868a1777cb0a657638ed0fb03 Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Tue, 17 Dec 2024 16:32:56 +0100 Subject: [PATCH 03/15] feat: remove vscode --- .vscode/settings.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 864f600b0..833034955 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -3,5 +3,6 @@ "editor.codeActionsOnSave": { "source.fixAll.eslint": "always" }, - "solidity.defaultCompiler": "remote" + "solidity.defaultCompiler": "remote", + "cSpell.words": ["IETHRegistrarController", "sealables", "streccak", "TmplAppInstalled", "TmplDAOAndTokenDeployed"] } From 07da614bd9a699643fc52b728673eb69eea9cb28 Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Tue, 17 Dec 2024 17:34:09 +0100 Subject: [PATCH 04/15] fix: fix delegation tests --- .../vaults/delegation/delegation.test.ts | 28 +++++++++++-------- test/0.8.25/vaults/vaultFactory.test.ts | 6 ---- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/test/0.8.25/vaults/delegation/delegation.test.ts b/test/0.8.25/vaults/delegation/delegation.test.ts index 3f99aaf54..d3db3fb7d 100644 --- a/test/0.8.25/vaults/delegation/delegation.test.ts +++ b/test/0.8.25/vaults/delegation/delegation.test.ts @@ -9,6 +9,7 @@ import { DepositContract__MockForStakingVault, StakingVault, StETH__MockForDelegation, + UpgradeableBeacon, VaultFactory, VaultHub__MockForDelegation, } from "typechain-types"; @@ -25,7 +26,7 @@ describe("Delegation", () => { let manager: HardhatEthersSigner; let operator: HardhatEthersSigner; let stranger: HardhatEthersSigner; - let factoryOwner: HardhatEthersSigner; + let beaconOwner: HardhatEthersSigner; let hubSigner: HardhatEthersSigner; let steth: StETH__MockForDelegation; @@ -36,12 +37,12 @@ describe("Delegation", () => { let factory: VaultFactory; let vault: StakingVault; let delegation: Delegation; + let beacon: UpgradeableBeacon; let originalState: string; before(async () => { - [deployer, vaultOwner, manager, staker, operator, keyMaster, tokenMaster, stranger, factoryOwner] = - await ethers.getSigners(); + [vaultOwner, manager, operator, stranger, beaconOwner] = await ethers.getSigners(); steth = await ethers.deployContract("StETH__MockForDelegation"); delegationImpl = await ethers.deployContract("Delegation", [steth]); @@ -52,17 +53,19 @@ describe("Delegation", () => { vaultImpl = await ethers.deployContract("StakingVault", [hub, depositContract]); expect(await vaultImpl.vaultHub()).to.equal(hub); - factory = await ethers.deployContract("VaultFactory", [ - factoryOwner, - vaultImpl.getAddress(), - delegationImpl.getAddress(), - ]); - expect(await factory.implementation()).to.equal(vaultImpl); - expect(await factory.delegationImpl()).to.equal(delegationImpl); + beacon = await ethers.deployContract("UpgradeableBeacon", [vaultImpl, beaconOwner]); + + factory = await ethers.deployContract("VaultFactory", [beacon.getAddress(), delegationImpl.getAddress()]); + expect(await beacon.implementation()).to.equal(vaultImpl); + expect(await factory.BEACON()).to.equal(beacon); + expect(await factory.DELEGATION_IMPL()).to.equal(delegationImpl); const vaultCreationTx = await factory .connect(vaultOwner) - .createVault({ managementFee: 0n, performanceFee: 0n, manager, operator }, "0x"); + .createVaultWithDelegation( + { managementFeeBP: 0n, performanceFeeBP: 0n, defaultAdmin: vaultOwner, manager, operator }, + "0x", + ); const vaultCreationReceipt = await vaultCreationTx.wait(); if (!vaultCreationReceipt) throw new Error("Vault creation receipt not found"); @@ -70,7 +73,8 @@ describe("Delegation", () => { expect(vaultCreatedEvents.length).to.equal(1); const stakingVaultAddress = vaultCreatedEvents[0].args.vault; vault = await ethers.getContractAt("StakingVault", stakingVaultAddress, vaultOwner); - expect(await vault.getBeacon()).to.equal(factory); + expect(await vault.beacon()).to.equal(beacon); + expect(await vault.factory()).to.equal(factory); const delegationCreatedEvents = findEvents(vaultCreationReceipt, "DelegationCreated"); expect(delegationCreatedEvents.length).to.equal(1); diff --git a/test/0.8.25/vaults/vaultFactory.test.ts b/test/0.8.25/vaults/vaultFactory.test.ts index 1be0e584a..3be52e22f 100644 --- a/test/0.8.25/vaults/vaultFactory.test.ts +++ b/test/0.8.25/vaults/vaultFactory.test.ts @@ -82,12 +82,6 @@ describe("VaultFactory.sol", () => { //add VAULT_REGISTRY_ROLE role to allow admin to add factory and vault implementation to the hub await accounting.connect(admin).grantRole(await accounting.VAULT_REGISTRY_ROLE(), admin); - console.log({ - beaconAddress: await beacon.getAddress(), - delegationAddress: await delegation.getAddress(), - factoryAddress: await vaultFactory.getAddress(), - }); - //the initialize() function cannot be called on a contract await expect(implOld.initialize(admin, stranger, operator, "0x")).to.revertedWithCustomError( implOld, From 6399ce0d95a73f803e78a468c300f0523d74fa8c Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Tue, 17 Dec 2024 17:50:16 +0100 Subject: [PATCH 05/15] feat: fix scratch deploy --- lib/state-file.ts | 1 + scripts/scratch/steps/0145-deploy-vaults.ts | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/state-file.ts b/lib/state-file.ts index 2618ce3d7..eb487d6d2 100644 --- a/lib/state-file.ts +++ b/lib/state-file.ts @@ -91,6 +91,7 @@ export enum Sk { stakingVaultImpl = "stakingVaultImpl", stakingVaultFactory = "stakingVaultFactory", delegationImpl = "delegationImpl", + stakingVaultBeacon = "stakingVaultBeacon", } export function getAddress(contractKey: Sk, state: DeploymentState): string { diff --git a/scripts/scratch/steps/0145-deploy-vaults.ts b/scripts/scratch/steps/0145-deploy-vaults.ts index 2e7715307..1b6622f54 100644 --- a/scripts/scratch/steps/0145-deploy-vaults.ts +++ b/scripts/scratch/steps/0145-deploy-vaults.ts @@ -26,10 +26,13 @@ export async function main() { const room = await deployWithoutProxy(Sk.delegationImpl, "Delegation", deployer, [lidoAddress]); const roomAddress = await room.getAddress(); + // Deploy Delegation implementation contract + const beacon = await deployWithoutProxy(Sk.stakingVaultBeacon, "UpgradeableBeacon", deployer, [impAddress, deployer]); + const beaconAddress = await beacon.getAddress(); + // Deploy VaultFactory contract const factory = await deployWithoutProxy(Sk.stakingVaultFactory, "VaultFactory", deployer, [ - deployer, - impAddress, + beaconAddress, roomAddress, ]); const factoryAddress = await factory.getAddress(); From 60192c664c23e81b5a3a4784dd395b03434f9792 Mon Sep 17 00:00:00 2001 From: Yuri Tkachenko Date: Tue, 17 Dec 2024 17:52:17 +0000 Subject: [PATCH 06/15] chore: try to invalidate cache --- test/0.8.25/vaults/dashboard/dashboard.test.ts | 2 +- test/0.8.25/vaults/delegation/delegation.test.ts | 2 +- test/0.8.25/vaults/staking-vault/staking-vault.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/0.8.25/vaults/dashboard/dashboard.test.ts b/test/0.8.25/vaults/dashboard/dashboard.test.ts index 999e81cdb..e3582d4ca 100644 --- a/test/0.8.25/vaults/dashboard/dashboard.test.ts +++ b/test/0.8.25/vaults/dashboard/dashboard.test.ts @@ -18,7 +18,7 @@ import { certainAddress, ether, findEvents } from "lib"; import { Snapshot } from "test/suite"; -describe("Dashboard", () => { +describe("Dashboard.sol", () => { let factoryOwner: HardhatEthersSigner; let vaultOwner: HardhatEthersSigner; let operator: HardhatEthersSigner; diff --git a/test/0.8.25/vaults/delegation/delegation.test.ts b/test/0.8.25/vaults/delegation/delegation.test.ts index d3db3fb7d..deeed8132 100644 --- a/test/0.8.25/vaults/delegation/delegation.test.ts +++ b/test/0.8.25/vaults/delegation/delegation.test.ts @@ -21,7 +21,7 @@ import { Snapshot } from "test/suite"; const BP_BASE = 10000n; const MAX_FEE = BP_BASE; -describe("Delegation", () => { +describe("Delegation.sol", () => { let vaultOwner: HardhatEthersSigner; let manager: HardhatEthersSigner; let operator: HardhatEthersSigner; diff --git a/test/0.8.25/vaults/staking-vault/staking-vault.test.ts b/test/0.8.25/vaults/staking-vault/staking-vault.test.ts index 582d0881b..4cd6fa3e5 100644 --- a/test/0.8.25/vaults/staking-vault/staking-vault.test.ts +++ b/test/0.8.25/vaults/staking-vault/staking-vault.test.ts @@ -23,7 +23,7 @@ const MAX_INT128 = 2n ** 127n - 1n; const MAX_UINT128 = 2n ** 128n - 1n; // @TODO: test reentrancy attacks -describe("StakingVault", () => { +describe("StakingVault.sol", () => { let vaultOwner: HardhatEthersSigner; let operator: HardhatEthersSigner; let stranger: HardhatEthersSigner; From 6bb65b2a415266d394d0d3d36217ec2b7a897d32 Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Thu, 9 Jan 2025 20:08:46 +0300 Subject: [PATCH 07/15] feat: fix tests --- lib/protocol/discover.ts | 18 ++++++-------- lib/protocol/types.ts | 4 ++++ .../vaults-happy-path.integration.ts | 24 +++++++++++-------- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/lib/protocol/discover.ts b/lib/protocol/discover.ts index 2f8bac947..3032020f5 100644 --- a/lib/protocol/discover.ts +++ b/lib/protocol/discover.ts @@ -1,13 +1,6 @@ import hre from "hardhat"; -import { - AccountingOracle, - Lido, - LidoLocator, - StakingRouter, - VaultFactory, - WithdrawalQueueERC721, -} from "typechain-types"; +import { AccountingOracle, Lido, LidoLocator, StakingRouter, WithdrawalQueueERC721 } from "typechain-types"; import { batch, log } from "lib"; @@ -22,6 +15,7 @@ import { ProtocolContracts, ProtocolSigners, StakingModuleContracts, + VaultsContracts, WstETHContracts, } from "./types"; @@ -164,10 +158,11 @@ const getWstEthContract = async ( /** * Load all required vaults contracts. */ -const getVaultsContracts = async (locator: LoadedContract, config: ProtocolNetworkConfig) => { +const getVaultsContracts = async (config: ProtocolNetworkConfig) => { return (await batch({ stakingVaultFactory: loadContract("VaultFactory", config.get("stakingVaultFactory")), - })) as { stakingVaultFactory: LoadedContract }; + stakingVaultBeacon: loadContract("UpgradeableBeacon", config.get("stakingVaultBeacon")), + })) as VaultsContracts; }; export async function discover() { @@ -182,7 +177,7 @@ export async function discover() { ...(await getStakingModules(foundationContracts.stakingRouter, networkConfig)), ...(await getHashConsensusContract(foundationContracts.accountingOracle, networkConfig)), ...(await getWstEthContract(foundationContracts.withdrawalQueue, networkConfig)), - ...(await getVaultsContracts(locator, networkConfig)), + ...(await getVaultsContracts(networkConfig)), } as ProtocolContracts; log.debug("Contracts discovered", { @@ -208,6 +203,7 @@ export async function discover() { "wstETH": contracts.wstETH.address, // Vaults "Staking Vault Factory": contracts.stakingVaultFactory.address, + "Staking Vault Beacon": contracts.stakingVaultBeacon.address, }); const signers = { diff --git a/lib/protocol/types.ts b/lib/protocol/types.ts index dc49038de..f8ae8cff2 100644 --- a/lib/protocol/types.ts +++ b/lib/protocol/types.ts @@ -18,6 +18,7 @@ import { OracleDaemonConfig, OracleReportSanityChecker, StakingRouter, + UpgradeableBeacon, ValidatorsExitBusOracle, VaultFactory, WithdrawalQueueERC721, @@ -56,6 +57,7 @@ export type ProtocolNetworkItems = { hashConsensus: string; // vaults stakingVaultFactory: string; + stakingVaultBeacon: string; }; export interface ContractTypes { @@ -79,6 +81,7 @@ export interface ContractTypes { NodeOperatorsRegistry: NodeOperatorsRegistry; WstETH: WstETH; VaultFactory: VaultFactory; + UpgradeableBeacon: UpgradeableBeacon; } export type ContractName = keyof ContractTypes; @@ -129,6 +132,7 @@ export type WstETHContracts = { export type VaultsContracts = { stakingVaultFactory: LoadedContract; + stakingVaultBeacon: LoadedContract; }; export type ProtocolContracts = { locator: LoadedContract } & CoreContracts & diff --git a/test/integration/vaults-happy-path.integration.ts b/test/integration/vaults-happy-path.integration.ts index 6c524b66f..ad15a4f84 100644 --- a/test/integration/vaults-happy-path.integration.ts +++ b/test/integration/vaults-happy-path.integration.ts @@ -4,7 +4,7 @@ import { ethers } from "hardhat"; import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers"; -import { Delegation,StakingVault } from "typechain-types"; +import { Delegation, StakingVault } from "typechain-types"; import { impersonate, log, trace, updateBalance } from "lib"; import { getProtocolContext, ProtocolContext } from "lib/protocol"; @@ -136,10 +136,10 @@ describe("Scenario: Staking Vaults Happy Path", () => { }); it("Should have vaults factory deployed and adopted by DAO", async () => { - const { stakingVaultFactory } = ctx.contracts; + const { stakingVaultFactory, stakingVaultBeacon } = ctx.contracts; - const implAddress = await stakingVaultFactory.implementation(); - const adminContractImplAddress = await stakingVaultFactory.delegationImpl(); + const implAddress = await stakingVaultBeacon.implementation(); + const adminContractImplAddress = await stakingVaultFactory.DELEGATION_IMPL(); const vaultImpl = await ethers.getContractAt("StakingVault", implAddress); const vaultFactoryAdminContract = await ethers.getContractAt("Delegation", adminContractImplAddress); @@ -155,12 +155,16 @@ describe("Scenario: Staking Vaults Happy Path", () => { const { stakingVaultFactory } = ctx.contracts; // Alice can create a vault with Bob as a node operator - const deployTx = await stakingVaultFactory.connect(alice).createVault("0x", { - managementFee: VAULT_OWNER_FEE, - performanceFee: VAULT_NODE_OPERATOR_FEE, - manager: alice, - operator: bob, - }, lidoAgent); + const deployTx = await stakingVaultFactory.connect(alice).createVaultWithDelegation( + { + managementFeeBP: VAULT_OWNER_FEE, + performanceFeeBP: VAULT_NODE_OPERATOR_FEE, + defaultAdmin: lidoAgent, + manager: alice, + operator: bob, + }, + lidoAgent, + ); const createVaultTxReceipt = await trace("vaultsFactory.createVault", deployTx); const createVaultEvents = ctx.getEvents(createVaultTxReceipt, "VaultCreated"); From 29159ac073f673089fc783f74ca58adf732c875c Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Tue, 14 Jan 2025 13:41:54 +0300 Subject: [PATCH 08/15] feat: remove getBeacon() --- contracts/0.8.25/vaults/StakingVault.sol | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/contracts/0.8.25/vaults/StakingVault.sol b/contracts/0.8.25/vaults/StakingVault.sol index d4e41fda8..6c0f55762 100644 --- a/contracts/0.8.25/vaults/StakingVault.sol +++ b/contracts/0.8.25/vaults/StakingVault.sol @@ -134,10 +134,10 @@ contract StakingVault is IStakingVault, IBeaconProxy, BeaconChainDepositLogistic } /** - * @notice Returns the address of the beacon - * @return Address of the beacon + * @notice Returns the beacon proxy address that controls this contract's implementation + * @return address The beacon proxy address */ - function getBeacon() public view returns (address) { + function beacon() public view returns (address) { return ERC1967Utils.getBeacon(); } @@ -153,14 +153,6 @@ contract StakingVault is IStakingVault, IBeaconProxy, BeaconChainDepositLogistic return address(VAULT_HUB); } - /** - * @notice Returns the beacon proxy address that controls this contract's implementation - * @return address The beacon proxy address - */ - function beacon() public view returns (address) { - return ERC1967Utils.getBeacon(); - } - /** * @notice Returns the valuation of the vault * @return uint256 total valuation in ETH From 0ace794f2a3db6816e257e39fee588d79b21719d Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Tue, 14 Jan 2025 13:46:00 +0300 Subject: [PATCH 09/15] feat: remove comments --- lib/protocol/discover.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/protocol/discover.ts b/lib/protocol/discover.ts index 823dd2444..3032020f5 100644 --- a/lib/protocol/discover.ts +++ b/lib/protocol/discover.ts @@ -159,10 +159,6 @@ const getWstEthContract = async ( * Load all required vaults contracts. */ const getVaultsContracts = async (config: ProtocolNetworkConfig) => { - console.log("--------GEEEETEETETET VAAAAAAULTSSSS -----"); - - console.log(config); - return (await batch({ stakingVaultFactory: loadContract("VaultFactory", config.get("stakingVaultFactory")), stakingVaultBeacon: loadContract("UpgradeableBeacon", config.get("stakingVaultBeacon")), From 4aa0eb66aa8fac7186767b6658d148c3145d3c1f Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Tue, 14 Jan 2025 18:13:21 +0300 Subject: [PATCH 10/15] feat: add immutable args for Clones --- contracts/openzeppelin/5.2.0/proxy/Clones.sol | 0 contracts/openzeppelin/5.2.0/utils/Create2.sol | 0 contracts/openzeppelin/5.2.0/utils/Errors.sol | 0 3 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 contracts/openzeppelin/5.2.0/proxy/Clones.sol create mode 100644 contracts/openzeppelin/5.2.0/utils/Create2.sol create mode 100644 contracts/openzeppelin/5.2.0/utils/Errors.sol diff --git a/contracts/openzeppelin/5.2.0/proxy/Clones.sol b/contracts/openzeppelin/5.2.0/proxy/Clones.sol new file mode 100644 index 000000000..e69de29bb diff --git a/contracts/openzeppelin/5.2.0/utils/Create2.sol b/contracts/openzeppelin/5.2.0/utils/Create2.sol new file mode 100644 index 000000000..e69de29bb diff --git a/contracts/openzeppelin/5.2.0/utils/Errors.sol b/contracts/openzeppelin/5.2.0/utils/Errors.sol new file mode 100644 index 000000000..e69de29bb From 626556146bf4ff2eaedd2dc2b00ded0de9c16c88 Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Tue, 14 Jan 2025 18:13:55 +0300 Subject: [PATCH 11/15] feat: add immutable args for Clones --- contracts/0.8.25/vaults/Dashboard.sol | 65 +++-- contracts/0.8.25/vaults/Delegation.sol | 17 +- contracts/0.8.25/vaults/VaultFactory.sol | 9 +- contracts/openzeppelin/5.2.0/proxy/Clones.sol | 262 ++++++++++++++++++ .../openzeppelin/5.2.0/utils/Create2.sol | 92 ++++++ contracts/openzeppelin/5.2.0/utils/Errors.sol | 34 +++ .../VaultFactory__MockForDashboard.sol | 7 +- .../0.8.25/vaults/dashboard/dashboard.test.ts | 10 +- .../vaults/delegation/delegation.test.ts | 12 +- 9 files changed, 445 insertions(+), 63 deletions(-) diff --git a/contracts/0.8.25/vaults/Dashboard.sol b/contracts/0.8.25/vaults/Dashboard.sol index 3bb4c8ddf..9b8fee28c 100644 --- a/contracts/0.8.25/vaults/Dashboard.sol +++ b/contracts/0.8.25/vaults/Dashboard.sol @@ -8,11 +8,14 @@ import {AccessControlEnumerable} from "@openzeppelin/contracts-v5.0.2/access/ext import {OwnableUpgradeable} from "contracts/openzeppelin/5.0.2/upgradeable/access/OwnableUpgradeable.sol"; import {IERC20} from "@openzeppelin/contracts-v5.0.2/token/ERC20/IERC20.sol"; import {IERC20Permit} from "@openzeppelin/contracts-v5.0.2/token/ERC20/extensions/IERC20Permit.sol"; +import {Clones} from "contracts/openzeppelin/5.2.0/proxy/Clones.sol"; import {Math256} from "contracts/common/lib/Math256.sol"; + import {VaultHub} from "./VaultHub.sol"; + import {IStakingVault} from "./interfaces/IStakingVault.sol"; import {ILido as IStETH} from "../interfaces/ILido.sol"; @@ -56,9 +59,6 @@ contract Dashboard is AccessControlEnumerable { /// @notice The wrapped ether token contract IWeth public immutable WETH; - /// @notice The underlying `StakingVault` contract - IStakingVault public stakingVault; - /// @notice The `VaultHub` contract VaultHub public vaultHub; @@ -88,25 +88,22 @@ contract Dashboard is AccessControlEnumerable { } /** - * @notice Initializes the contract with the default admin and `StakingVault` address. - * @param _stakingVault Address of the `StakingVault` contract. + * @notice Initializes the contract with the default admin + * and `vaultHub` address */ - function initialize(address _stakingVault) external virtual { - _initialize(_stakingVault); + function initialize() external virtual { + _initialize(); } /** * @dev Internal initialize function. - * @param _stakingVault Address of the `StakingVault` contract. */ - function _initialize(address _stakingVault) internal { - if (_stakingVault == address(0)) revert ZeroArgument("_stakingVault"); + function _initialize() internal { if (isInitialized) revert AlreadyInitialized(); if (address(this) == _SELF) revert NonProxyCallsForbidden(); isInitialized = true; - stakingVault = IStakingVault(_stakingVault); - vaultHub = VaultHub(stakingVault.vaultHub()); + vaultHub = VaultHub(stakingVault().vaultHub()); _grantRole(DEFAULT_ADMIN_ROLE, msg.sender); emit Initialized(); @@ -119,7 +116,7 @@ contract Dashboard is AccessControlEnumerable { * @return VaultSocket struct containing vault data */ function vaultSocket() public view returns (VaultHub.VaultSocket memory) { - return vaultHub.vaultSocket(address(stakingVault)); + return vaultHub.vaultSocket(address(stakingVault())); } /** @@ -167,7 +164,7 @@ contract Dashboard is AccessControlEnumerable { * @return The valuation as a uint256. */ function valuation() external view returns (uint256) { - return stakingVault.valuation(); + return stakingVault().valuation(); } /** @@ -175,7 +172,7 @@ contract Dashboard is AccessControlEnumerable { * @return The maximum number of stETH shares as a uint256. */ function totalMintableShares() public view returns (uint256) { - return _totalMintableShares(stakingVault.valuation()); + return _totalMintableShares(stakingVault().valuation()); } /** @@ -184,7 +181,7 @@ contract Dashboard is AccessControlEnumerable { * @return the maximum number of shares that can be minted by ether */ function getMintableShares(uint256 _ether) external view returns (uint256) { - uint256 _totalShares = _totalMintableShares(stakingVault.valuation() + _ether); + uint256 _totalShares = _totalMintableShares(stakingVault().valuation() + _ether); uint256 _sharesMinted = vaultSocket().sharesMinted; if (_totalShares < _sharesMinted) return 0; @@ -196,7 +193,7 @@ contract Dashboard is AccessControlEnumerable { * @return The amount of ether that can be withdrawn. */ function getWithdrawableEther() external view returns (uint256) { - return Math256.min(address(stakingVault).balance, stakingVault.unlocked()); + return Math256.min(address(stakingVault()).balance, stakingVault().unlocked()); } // TODO: add preview view methods for minting and burning @@ -244,7 +241,7 @@ contract Dashboard is AccessControlEnumerable { WETH.withdraw(_wethAmount); // TODO: find way to use _fund() instead of stakingVault directly - stakingVault.fund{value: _wethAmount}(); + stakingVault().fund{value: _wethAmount}(); } /** @@ -324,7 +321,7 @@ contract Dashboard is AccessControlEnumerable { uint256 sharesAmount = STETH.getSharesByPooledEth(stETHAmount); - vaultHub.burnSharesBackedByVault(address(stakingVault), sharesAmount); + vaultHub.burnSharesBackedByVault(address(stakingVault()), sharesAmount); } /** @@ -398,7 +395,7 @@ contract Dashboard is AccessControlEnumerable { uint256 sharesAmount = STETH.getSharesByPooledEth(stETHAmount); - vaultHub.burnSharesBackedByVault(address(stakingVault), sharesAmount); + vaultHub.burnSharesBackedByVault(address(stakingVault()), sharesAmount); } /** @@ -426,7 +423,7 @@ contract Dashboard is AccessControlEnumerable { * @param _newOwner Address of the new owner */ function _transferStVaultOwnership(address _newOwner) internal { - OwnableUpgradeable(address(stakingVault)).transferOwnership(_newOwner); + OwnableUpgradeable(address(stakingVault())).transferOwnership(_newOwner); } /** @@ -438,14 +435,14 @@ contract Dashboard is AccessControlEnumerable { _rebalanceVault(STETH.getPooledEthBySharesRoundUp(shares)); } - vaultHub.voluntaryDisconnect(address(stakingVault)); + vaultHub.voluntaryDisconnect(address(stakingVault())); } /** * @dev Funds the staking vault with the ether sent in the transaction */ function _fund() internal { - stakingVault.fund{value: msg.value}(); + stakingVault().fund{value: msg.value}(); } /** @@ -454,7 +451,7 @@ contract Dashboard is AccessControlEnumerable { * @param _ether Amount of ether to withdraw */ function _withdraw(address _recipient, uint256 _ether) internal { - stakingVault.withdraw(_recipient, _ether); + stakingVault().withdraw(_recipient, _ether); } /** @@ -462,7 +459,7 @@ contract Dashboard is AccessControlEnumerable { * @param _validatorPublicKey Public key of the validator to exit */ function _requestValidatorExit(bytes calldata _validatorPublicKey) internal { - stakingVault.requestValidatorExit(_validatorPublicKey); + stakingVault().requestValidatorExit(_validatorPublicKey); } /** @@ -476,7 +473,7 @@ contract Dashboard is AccessControlEnumerable { bytes calldata _pubkeys, bytes calldata _signatures ) internal { - stakingVault.depositToBeaconChain(_numberOfDeposits, _pubkeys, _signatures); + stakingVault().depositToBeaconChain(_numberOfDeposits, _pubkeys, _signatures); } /** @@ -485,7 +482,7 @@ contract Dashboard is AccessControlEnumerable { * @param _amountOfShares Amount of tokens to mint */ function _mint(address _recipient, uint256 _amountOfShares) internal { - vaultHub.mintSharesBackedByVault(address(stakingVault), _recipient, _amountOfShares); + vaultHub.mintSharesBackedByVault(address(stakingVault()), _recipient, _amountOfShares); } /** @@ -494,7 +491,7 @@ contract Dashboard is AccessControlEnumerable { */ function _burn(uint256 _amountOfShares) internal { STETH.transferSharesFrom(msg.sender, address(vaultHub), _amountOfShares); - vaultHub.burnSharesBackedByVault(address(stakingVault), _amountOfShares); + vaultHub.burnSharesBackedByVault(address(stakingVault()), _amountOfShares); } /** @@ -511,7 +508,17 @@ contract Dashboard is AccessControlEnumerable { * @param _ether Amount of ether to rebalance */ function _rebalanceVault(uint256 _ether) internal { - stakingVault.rebalance(_ether); + stakingVault().rebalance(_ether); + } + + /// @notice The underlying `StakingVault` contract + function stakingVault() public view returns (IStakingVault) { + bytes memory args = Clones.fetchCloneArgs(address(this)); + address addr; + assembly { + addr := mload(add(args, 32)) + } + return IStakingVault(addr); } // ==================== Events ==================== diff --git a/contracts/0.8.25/vaults/Delegation.sol b/contracts/0.8.25/vaults/Delegation.sol index 08429de3c..de3dc8228 100644 --- a/contracts/0.8.25/vaults/Delegation.sol +++ b/contracts/0.8.25/vaults/Delegation.sol @@ -124,18 +124,17 @@ contract Delegation is Dashboard { /** * @notice Initializes the contract: - * - sets the address of StakingVault; + * - sets the vaultHub from inherit Dashboard `_initialize()` func * - sets up the roles; * - sets the vote lifetime to 7 days (can be changed later by CURATOR_ROLE and OPERATOR_ROLE). - * @param _stakingVault The address of StakingVault. * @dev The msg.sender here is VaultFactory. It is given the OPERATOR_ROLE * to be able to set initial operatorFee in VaultFactory, because only OPERATOR_ROLE * is the admin role for itself. The rest of the roles are also temporarily given to * VaultFactory to be able to set initial config in VaultFactory. * All the roles are revoked from VaultFactory at the end of the initialization. */ - function initialize(address _stakingVault) external override { - _initialize(_stakingVault); + function initialize() external override { + _initialize(); // the next line implies that the msg.sender is an operator // however, the msg.sender is the VaultFactory, and the role will be revoked @@ -184,8 +183,8 @@ contract Delegation is Dashboard { * @return uint256: the amount of unreserved ether. */ function unreserved() public view returns (uint256) { - uint256 reserved = stakingVault.locked() + curatorDue() + operatorDue(); - uint256 valuation = stakingVault.valuation(); + uint256 reserved = stakingVault().locked() + curatorDue() + operatorDue(); + uint256 valuation = stakingVault().valuation(); return reserved > valuation ? 0 : valuation - reserved; } @@ -313,7 +312,7 @@ contract Delegation is Dashboard { */ function claimCuratorDue(address _recipient) external onlyRole(CURATOR_ROLE) { uint256 due = curatorDue(); - curatorDueClaimedReport = stakingVault.latestReport(); + curatorDueClaimedReport = stakingVault().latestReport(); _claimDue(_recipient, due); } @@ -325,7 +324,7 @@ contract Delegation is Dashboard { */ function claimOperatorDue(address _recipient) external onlyRole(CLAIM_OPERATOR_DUE_ROLE) { uint256 due = operatorDue(); - operatorDueClaimedReport = stakingVault.latestReport(); + operatorDueClaimedReport = stakingVault().latestReport(); _claimDue(_recipient, due); } @@ -434,7 +433,7 @@ contract Delegation is Dashboard { uint256 _fee, IStakingVault.Report memory _lastClaimedReport ) internal view returns (uint256) { - IStakingVault.Report memory latestReport = stakingVault.latestReport(); + IStakingVault.Report memory latestReport = stakingVault().latestReport(); int128 rewardsAccrued = int128(latestReport.valuation - _lastClaimedReport.valuation) - (latestReport.inOutDelta - _lastClaimedReport.inOutDelta); diff --git a/contracts/0.8.25/vaults/VaultFactory.sol b/contracts/0.8.25/vaults/VaultFactory.sol index 19b16d5a5..a50354ea4 100644 --- a/contracts/0.8.25/vaults/VaultFactory.sol +++ b/contracts/0.8.25/vaults/VaultFactory.sol @@ -5,7 +5,7 @@ pragma solidity 0.8.25; import {BeaconProxy} from "@openzeppelin/contracts-v5.0.2/proxy/beacon/BeaconProxy.sol"; -import {Clones} from "@openzeppelin/contracts-v5.0.2/proxy/Clones.sol"; +import {Clones} from "contracts/openzeppelin/5.2.0/proxy/Clones.sol"; import {IStakingVault} from "./interfaces/IStakingVault.sol"; @@ -34,7 +34,7 @@ interface IDelegation { function CLAIM_OPERATOR_DUE_ROLE() external view returns (bytes32); - function initialize(address _stakingVault) external; + function initialize() external; function setCuratorFee(uint256 _newCuratorFee) external; @@ -74,7 +74,8 @@ contract VaultFactory { // create StakingVault vault = IStakingVault(address(new BeaconProxy(BEACON, ""))); // create Delegation - delegation = IDelegation(Clones.clone(DELEGATION_IMPL)); + bytes memory immutableArgs = abi.encode(vault); + delegation = IDelegation(Clones.cloneWithImmutableArgs(DELEGATION_IMPL, immutableArgs)); // initialize StakingVault vault.initialize( @@ -83,7 +84,7 @@ contract VaultFactory { _stakingVaultInitializerExtraParams ); // initialize Delegation - delegation.initialize(address(vault)); + delegation.initialize(); // grant roles to defaultAdmin, owner, manager, operator delegation.grantRole(delegation.DEFAULT_ADMIN_ROLE(), _delegationInitialState.defaultAdmin); diff --git a/contracts/openzeppelin/5.2.0/proxy/Clones.sol b/contracts/openzeppelin/5.2.0/proxy/Clones.sol index e69de29bb..fc66906e9 100644 --- a/contracts/openzeppelin/5.2.0/proxy/Clones.sol +++ b/contracts/openzeppelin/5.2.0/proxy/Clones.sol @@ -0,0 +1,262 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts (last updated v5.2.0) (proxy/Clones.sol) + +pragma solidity ^0.8.20; + +import {Create2} from "../utils/Create2.sol"; +import {Errors} from "../utils/Errors.sol"; + +/** + * @dev https://eips.ethereum.org/EIPS/eip-1167[ERC-1167] is a standard for + * deploying minimal proxy contracts, also known as "clones". + * + * > To simply and cheaply clone contract functionality in an immutable way, this standard specifies + * > a minimal bytecode implementation that delegates all calls to a known, fixed address. + * + * The library includes functions to deploy a proxy using either `create` (traditional deployment) or `create2` + * (salted deterministic deployment). It also includes functions to predict the addresses of clones deployed using the + * deterministic method. + */ +library Clones { + error CloneArgumentsTooLong(); + + /** + * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation`. + * + * This function uses the create opcode, which should never revert. + */ + function clone(address implementation) internal returns (address instance) { + return clone(implementation, 0); + } + + /** + * @dev Same as {xref-Clones-clone-address-}[clone], but with a `value` parameter to send native currency + * to the new contract. + * + * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory) + * to always have enough balance for new deployments. Consider exposing this function under a payable method. + */ + function clone(address implementation, uint256 value) internal returns (address instance) { + if (address(this).balance < value) { + revert Errors.InsufficientBalance(address(this).balance, value); + } + assembly ("memory-safe") { + // Cleans the upper 96 bits of the `implementation` word, then packs the first 3 bytes + // of the `implementation` address with the bytecode before the address. + mstore(0x00, or(shr(0xe8, shl(0x60, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000)) + // Packs the remaining 17 bytes of `implementation` with the bytecode after the address. + mstore(0x20, or(shl(0x78, implementation), 0x5af43d82803e903d91602b57fd5bf3)) + instance := create(value, 0x09, 0x37) + } + if (instance == address(0)) { + revert Errors.FailedDeployment(); + } + } + + /** + * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation`. + * + * This function uses the create2 opcode and a `salt` to deterministically deploy + * the clone. Using the same `implementation` and `salt` multiple times will revert, since + * the clones cannot be deployed twice at the same address. + */ + function cloneDeterministic(address implementation, bytes32 salt) internal returns (address instance) { + return cloneDeterministic(implementation, salt, 0); + } + + /** + * @dev Same as {xref-Clones-cloneDeterministic-address-bytes32-}[cloneDeterministic], but with + * a `value` parameter to send native currency to the new contract. + * + * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory) + * to always have enough balance for new deployments. Consider exposing this function under a payable method. + */ + function cloneDeterministic( + address implementation, + bytes32 salt, + uint256 value + ) internal returns (address instance) { + if (address(this).balance < value) { + revert Errors.InsufficientBalance(address(this).balance, value); + } + assembly ("memory-safe") { + // Cleans the upper 96 bits of the `implementation` word, then packs the first 3 bytes + // of the `implementation` address with the bytecode before the address. + mstore(0x00, or(shr(0xe8, shl(0x60, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000)) + // Packs the remaining 17 bytes of `implementation` with the bytecode after the address. + mstore(0x20, or(shl(0x78, implementation), 0x5af43d82803e903d91602b57fd5bf3)) + instance := create2(value, 0x09, 0x37, salt) + } + if (instance == address(0)) { + revert Errors.FailedDeployment(); + } + } + + /** + * @dev Computes the address of a clone deployed using {Clones-cloneDeterministic}. + */ + function predictDeterministicAddress( + address implementation, + bytes32 salt, + address deployer + ) internal pure returns (address predicted) { + assembly ("memory-safe") { + let ptr := mload(0x40) + mstore(add(ptr, 0x38), deployer) + mstore(add(ptr, 0x24), 0x5af43d82803e903d91602b57fd5bf3ff) + mstore(add(ptr, 0x14), implementation) + mstore(ptr, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73) + mstore(add(ptr, 0x58), salt) + mstore(add(ptr, 0x78), keccak256(add(ptr, 0x0c), 0x37)) + predicted := and(keccak256(add(ptr, 0x43), 0x55), 0xffffffffffffffffffffffffffffffffffffffff) + } + } + + /** + * @dev Computes the address of a clone deployed using {Clones-cloneDeterministic}. + */ + function predictDeterministicAddress( + address implementation, + bytes32 salt + ) internal view returns (address predicted) { + return predictDeterministicAddress(implementation, salt, address(this)); + } + + /** + * @dev Deploys and returns the address of a clone that mimics the behavior of `implementation` with custom + * immutable arguments. These are provided through `args` and cannot be changed after deployment. To + * access the arguments within the implementation, use {fetchCloneArgs}. + * + * This function uses the create opcode, which should never revert. + */ + function cloneWithImmutableArgs(address implementation, bytes memory args) internal returns (address instance) { + return cloneWithImmutableArgs(implementation, args, 0); + } + + /** + * @dev Same as {xref-Clones-cloneWithImmutableArgs-address-bytes-}[cloneWithImmutableArgs], but with a `value` + * parameter to send native currency to the new contract. + * + * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory) + * to always have enough balance for new deployments. Consider exposing this function under a payable method. + */ + function cloneWithImmutableArgs( + address implementation, + bytes memory args, + uint256 value + ) internal returns (address instance) { + if (address(this).balance < value) { + revert Errors.InsufficientBalance(address(this).balance, value); + } + bytes memory bytecode = _cloneCodeWithImmutableArgs(implementation, args); + assembly ("memory-safe") { + instance := create(value, add(bytecode, 0x20), mload(bytecode)) + } + if (instance == address(0)) { + revert Errors.FailedDeployment(); + } + } + + /** + * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation` with custom + * immutable arguments. These are provided through `args` and cannot be changed after deployment. To + * access the arguments within the implementation, use {fetchCloneArgs}. + * + * This function uses the create2 opcode and a `salt` to deterministically deploy the clone. Using the same + * `implementation`, `args` and `salt` multiple times will revert, since the clones cannot be deployed twice + * at the same address. + */ + function cloneDeterministicWithImmutableArgs( + address implementation, + bytes memory args, + bytes32 salt + ) internal returns (address instance) { + return cloneDeterministicWithImmutableArgs(implementation, args, salt, 0); + } + + /** + * @dev Same as {xref-Clones-cloneDeterministicWithImmutableArgs-address-bytes-bytes32-}[cloneDeterministicWithImmutableArgs], + * but with a `value` parameter to send native currency to the new contract. + * + * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory) + * to always have enough balance for new deployments. Consider exposing this function under a payable method. + */ + function cloneDeterministicWithImmutableArgs( + address implementation, + bytes memory args, + bytes32 salt, + uint256 value + ) internal returns (address instance) { + bytes memory bytecode = _cloneCodeWithImmutableArgs(implementation, args); + return Create2.deploy(value, salt, bytecode); + } + + /** + * @dev Computes the address of a clone deployed using {Clones-cloneDeterministicWithImmutableArgs}. + */ + function predictDeterministicAddressWithImmutableArgs( + address implementation, + bytes memory args, + bytes32 salt, + address deployer + ) internal pure returns (address predicted) { + bytes memory bytecode = _cloneCodeWithImmutableArgs(implementation, args); + return Create2.computeAddress(salt, keccak256(bytecode), deployer); + } + + /** + * @dev Computes the address of a clone deployed using {Clones-cloneDeterministicWithImmutableArgs}. + */ + function predictDeterministicAddressWithImmutableArgs( + address implementation, + bytes memory args, + bytes32 salt + ) internal view returns (address predicted) { + return predictDeterministicAddressWithImmutableArgs(implementation, args, salt, address(this)); + } + + /** + * @dev Get the immutable args attached to a clone. + * + * - If `instance` is a clone that was deployed using `clone` or `cloneDeterministic`, this + * function will return an empty array. + * - If `instance` is a clone that was deployed using `cloneWithImmutableArgs` or + * `cloneDeterministicWithImmutableArgs`, this function will return the args array used at + * creation. + * - If `instance` is NOT a clone deployed using this library, the behavior is undefined. This + * function should only be used to check addresses that are known to be clones. + */ + function fetchCloneArgs(address instance) internal view returns (bytes memory) { + bytes memory result = new bytes(instance.code.length - 45); // revert if length is too short + assembly ("memory-safe") { + extcodecopy(instance, add(result, 32), 45, mload(result)) + } + return result; + } + + /** + * @dev Helper that prepares the initcode of the proxy with immutable args. + * + * An assembly variant of this function requires copying the `args` array, which can be efficiently done using + * `mcopy`. Unfortunately, that opcode is not available before cancun. A pure solidity implementation using + * abi.encodePacked is more expensive but also more portable and easier to review. + * + * NOTE: https://eips.ethereum.org/EIPS/eip-170[EIP-170] limits the length of the contract code to 24576 bytes. + * With the proxy code taking 45 bytes, that limits the length of the immutable args to 24531 bytes. + */ + function _cloneCodeWithImmutableArgs( + address implementation, + bytes memory args + ) private pure returns (bytes memory) { + if (args.length > 24531) revert CloneArgumentsTooLong(); + return + abi.encodePacked( + hex"61", + uint16(args.length + 45), + hex"3d81600a3d39f3363d3d373d3d3d363d73", + implementation, + hex"5af43d82803e903d91602b57fd5bf3", + args + ); + } +} diff --git a/contracts/openzeppelin/5.2.0/utils/Create2.sol b/contracts/openzeppelin/5.2.0/utils/Create2.sol index e69de29bb..d61331741 100644 --- a/contracts/openzeppelin/5.2.0/utils/Create2.sol +++ b/contracts/openzeppelin/5.2.0/utils/Create2.sol @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts (last updated v5.1.0) (utils/Create2.sol) + +pragma solidity ^0.8.20; + +import {Errors} from "./Errors.sol"; + +/** + * @dev Helper to make usage of the `CREATE2` EVM opcode easier and safer. + * `CREATE2` can be used to compute in advance the address where a smart + * contract will be deployed, which allows for interesting new mechanisms known + * as 'counterfactual interactions'. + * + * See the https://eips.ethereum.org/EIPS/eip-1014#motivation[EIP] for more + * information. + */ +library Create2 { + /** + * @dev There's no code to deploy. + */ + error Create2EmptyBytecode(); + + /** + * @dev Deploys a contract using `CREATE2`. The address where the contract + * will be deployed can be known in advance via {computeAddress}. + * + * The bytecode for a contract can be obtained from Solidity with + * `type(contractName).creationCode`. + * + * Requirements: + * + * - `bytecode` must not be empty. + * - `salt` must have not been used for `bytecode` already. + * - the factory must have a balance of at least `amount`. + * - if `amount` is non-zero, `bytecode` must have a `payable` constructor. + */ + function deploy(uint256 amount, bytes32 salt, bytes memory bytecode) internal returns (address addr) { + if (address(this).balance < amount) { + revert Errors.InsufficientBalance(address(this).balance, amount); + } + if (bytecode.length == 0) { + revert Create2EmptyBytecode(); + } + assembly ("memory-safe") { + addr := create2(amount, add(bytecode, 0x20), mload(bytecode), salt) + // if no address was created, and returndata is not empty, bubble revert + if and(iszero(addr), not(iszero(returndatasize()))) { + let p := mload(0x40) + returndatacopy(p, 0, returndatasize()) + revert(p, returndatasize()) + } + } + if (addr == address(0)) { + revert Errors.FailedDeployment(); + } + } + + /** + * @dev Returns the address where a contract will be stored if deployed via {deploy}. Any change in the + * `bytecodeHash` or `salt` will result in a new destination address. + */ + function computeAddress(bytes32 salt, bytes32 bytecodeHash) internal view returns (address) { + return computeAddress(salt, bytecodeHash, address(this)); + } + + /** + * @dev Returns the address where a contract will be stored if deployed via {deploy} from a contract located at + * `deployer`. If `deployer` is this contract's address, returns the same value as {computeAddress}. + */ + function computeAddress(bytes32 salt, bytes32 bytecodeHash, address deployer) internal pure returns (address addr) { + assembly ("memory-safe") { + let ptr := mload(0x40) // Get free memory pointer + + // | | ↓ ptr ... ↓ ptr + 0x0B (start) ... ↓ ptr + 0x20 ... ↓ ptr + 0x40 ... | + // |-------------------|---------------------------------------------------------------------------| + // | bytecodeHash | CCCCCCCCCCCCC...CC | + // | salt | BBBBBBBBBBBBB...BB | + // | deployer | 000000...0000AAAAAAAAAAAAAAAAAAA...AA | + // | 0xFF | FF | + // |-------------------|---------------------------------------------------------------------------| + // | memory | 000000...00FFAAAAAAAAAAAAAAAAAAA...AABBBBBBBBBBBBB...BBCCCCCCCCCCCCC...CC | + // | keccak(start, 85) | ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑ | + + mstore(add(ptr, 0x40), bytecodeHash) + mstore(add(ptr, 0x20), salt) + mstore(ptr, deployer) // Right-aligned with 12 preceding garbage bytes + let start := add(ptr, 0x0b) // The hashed data starts at the final garbage byte which we will set to 0xff + mstore8(start, 0xff) + addr := and(keccak256(start, 85), 0xffffffffffffffffffffffffffffffffffffffff) + } + } +} diff --git a/contracts/openzeppelin/5.2.0/utils/Errors.sol b/contracts/openzeppelin/5.2.0/utils/Errors.sol index e69de29bb..442fc1892 100644 --- a/contracts/openzeppelin/5.2.0/utils/Errors.sol +++ b/contracts/openzeppelin/5.2.0/utils/Errors.sol @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts (last updated v5.1.0) (utils/Errors.sol) + +pragma solidity ^0.8.20; + +/** + * @dev Collection of common custom errors used in multiple contracts + * + * IMPORTANT: Backwards compatibility is not guaranteed in future versions of the library. + * It is recommended to avoid relying on the error API for critical functionality. + * + * _Available since v5.1._ + */ +library Errors { + /** + * @dev The ETH balance of the account is not enough to perform the operation. + */ + error InsufficientBalance(uint256 balance, uint256 needed); + + /** + * @dev A call to an address target failed. The target may have reverted. + */ + error FailedCall(); + + /** + * @dev The deployment failed. + */ + error FailedDeployment(); + + /** + * @dev A necessary precompile is missing. + */ + error MissingPrecompile(address); +} diff --git a/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol b/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol index 63a0c3d41..596a0e67a 100644 --- a/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol +++ b/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol @@ -5,7 +5,7 @@ pragma solidity 0.8.25; import {UpgradeableBeacon} from "@openzeppelin/contracts-v5.0.2/proxy/beacon/UpgradeableBeacon.sol"; import {BeaconProxy} from "@openzeppelin/contracts-v5.0.2/proxy/beacon/BeaconProxy.sol"; -import {Clones} from "@openzeppelin/contracts-v5.0.2/proxy/Clones.sol"; +import {Clones} from "contracts/openzeppelin/5.2.0/proxy/Clones.sol"; import {IStakingVault} from "contracts/0.8.25/vaults/interfaces/IStakingVault.sol"; import {Dashboard} from "contracts/0.8.25/vaults/Dashboard.sol"; @@ -25,9 +25,10 @@ contract VaultFactory__MockForDashboard is UpgradeableBeacon { function createVault(address _operator) external returns (IStakingVault vault, Dashboard dashboard) { vault = IStakingVault(address(new BeaconProxy(address(this), ""))); - dashboard = Dashboard(payable(Clones.clone(dashboardImpl))); + bytes memory immutableArgs = abi.encode(vault); + dashboard = Dashboard(payable(Clones.cloneWithImmutableArgs(dashboardImpl, immutableArgs))); - dashboard.initialize(address(vault)); + dashboard.initialize(); dashboard.grantRole(dashboard.DEFAULT_ADMIN_ROLE(), msg.sender); dashboard.revokeRole(dashboard.DEFAULT_ADMIN_ROLE(), address(this)); diff --git a/test/0.8.25/vaults/dashboard/dashboard.test.ts b/test/0.8.25/vaults/dashboard/dashboard.test.ts index 616f9f48d..5eb43cf02 100644 --- a/test/0.8.25/vaults/dashboard/dashboard.test.ts +++ b/test/0.8.25/vaults/dashboard/dashboard.test.ts @@ -119,20 +119,14 @@ describe("Dashboard.sol", () => { }); context("initialize", () => { - it("reverts if staking vault is zero address", async () => { - await expect(dashboard.initialize(ethers.ZeroAddress)) - .to.be.revertedWithCustomError(dashboard, "ZeroArgument") - .withArgs("_stakingVault"); - }); - it("reverts if already initialized", async () => { - await expect(dashboard.initialize(vault)).to.be.revertedWithCustomError(dashboard, "AlreadyInitialized"); + await expect(dashboard.initialize()).to.be.revertedWithCustomError(dashboard, "AlreadyInitialized"); }); it("reverts if called on the implementation", async () => { const dashboard_ = await ethers.deployContract("Dashboard", [steth, weth, wsteth]); - await expect(dashboard_.initialize(vault)).to.be.revertedWithCustomError(dashboard_, "NonProxyCallsForbidden"); + await expect(dashboard_.initialize()).to.be.revertedWithCustomError(dashboard_, "NonProxyCallsForbidden"); }); }); diff --git a/test/0.8.25/vaults/delegation/delegation.test.ts b/test/0.8.25/vaults/delegation/delegation.test.ts index 374b1246b..e512e7fcf 100644 --- a/test/0.8.25/vaults/delegation/delegation.test.ts +++ b/test/0.8.25/vaults/delegation/delegation.test.ts @@ -143,22 +143,14 @@ describe("Delegation.sol", () => { }); context("initialize", () => { - it("reverts if staking vault is zero address", async () => { - const delegation_ = await ethers.deployContract("Delegation", [steth, weth, wsteth]); - - await expect(delegation_.initialize(ethers.ZeroAddress)) - .to.be.revertedWithCustomError(delegation_, "ZeroArgument") - .withArgs("_stakingVault"); - }); - it("reverts if already initialized", async () => { - await expect(delegation.initialize(vault)).to.be.revertedWithCustomError(delegation, "AlreadyInitialized"); + await expect(delegation.initialize()).to.be.revertedWithCustomError(delegation, "AlreadyInitialized"); }); it("reverts if called on the implementation", async () => { const delegation_ = await ethers.deployContract("Delegation", [steth, weth, wsteth]); - await expect(delegation_.initialize(vault)).to.be.revertedWithCustomError(delegation_, "NonProxyCallsForbidden"); + await expect(delegation_.initialize()).to.be.revertedWithCustomError(delegation_, "NonProxyCallsForbidden"); }); }); From f52348ac33ede0369f9b420f9663ca9fd070dbdf Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Thu, 16 Jan 2025 17:10:05 +0300 Subject: [PATCH 12/15] feat: remove local OZ-5.2.0 --- contracts/0.8.25/vaults/Dashboard.sol | 2 +- contracts/0.8.25/vaults/VaultFactory.sol | 4 +- contracts/openzeppelin/5.2.0/proxy/Clones.sol | 262 ------------------ .../openzeppelin/5.2.0/utils/Create2.sol | 92 ------ contracts/openzeppelin/5.2.0/utils/Errors.sol | 34 --- package.json | 1 + .../VaultFactory__MockForDashboard.sol | 6 +- .../VaultFactory__MockForStakingVault.sol | 4 +- yarn.lock | 8 + 9 files changed, 17 insertions(+), 396 deletions(-) delete mode 100644 contracts/openzeppelin/5.2.0/proxy/Clones.sol delete mode 100644 contracts/openzeppelin/5.2.0/utils/Create2.sol delete mode 100644 contracts/openzeppelin/5.2.0/utils/Errors.sol diff --git a/contracts/0.8.25/vaults/Dashboard.sol b/contracts/0.8.25/vaults/Dashboard.sol index 9b8fee28c..3572c37dd 100644 --- a/contracts/0.8.25/vaults/Dashboard.sol +++ b/contracts/0.8.25/vaults/Dashboard.sol @@ -8,7 +8,7 @@ import {AccessControlEnumerable} from "@openzeppelin/contracts-v5.0.2/access/ext import {OwnableUpgradeable} from "contracts/openzeppelin/5.0.2/upgradeable/access/OwnableUpgradeable.sol"; import {IERC20} from "@openzeppelin/contracts-v5.0.2/token/ERC20/IERC20.sol"; import {IERC20Permit} from "@openzeppelin/contracts-v5.0.2/token/ERC20/extensions/IERC20Permit.sol"; -import {Clones} from "contracts/openzeppelin/5.2.0/proxy/Clones.sol"; +import {Clones} from "@openzeppelin/contracts-v5.2.0/proxy/Clones.sol"; import {Math256} from "contracts/common/lib/Math256.sol"; diff --git a/contracts/0.8.25/vaults/VaultFactory.sol b/contracts/0.8.25/vaults/VaultFactory.sol index a50354ea4..c7774eb2a 100644 --- a/contracts/0.8.25/vaults/VaultFactory.sol +++ b/contracts/0.8.25/vaults/VaultFactory.sol @@ -4,8 +4,8 @@ // See contracts/COMPILERS.md pragma solidity 0.8.25; -import {BeaconProxy} from "@openzeppelin/contracts-v5.0.2/proxy/beacon/BeaconProxy.sol"; -import {Clones} from "contracts/openzeppelin/5.2.0/proxy/Clones.sol"; +import {BeaconProxy} from "@openzeppelin/contracts-v5.2.0/proxy/beacon/BeaconProxy.sol"; +import {Clones} from "@openzeppelin/contracts-v5.2.0/proxy/Clones.sol"; import {IStakingVault} from "./interfaces/IStakingVault.sol"; diff --git a/contracts/openzeppelin/5.2.0/proxy/Clones.sol b/contracts/openzeppelin/5.2.0/proxy/Clones.sol deleted file mode 100644 index fc66906e9..000000000 --- a/contracts/openzeppelin/5.2.0/proxy/Clones.sol +++ /dev/null @@ -1,262 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts (last updated v5.2.0) (proxy/Clones.sol) - -pragma solidity ^0.8.20; - -import {Create2} from "../utils/Create2.sol"; -import {Errors} from "../utils/Errors.sol"; - -/** - * @dev https://eips.ethereum.org/EIPS/eip-1167[ERC-1167] is a standard for - * deploying minimal proxy contracts, also known as "clones". - * - * > To simply and cheaply clone contract functionality in an immutable way, this standard specifies - * > a minimal bytecode implementation that delegates all calls to a known, fixed address. - * - * The library includes functions to deploy a proxy using either `create` (traditional deployment) or `create2` - * (salted deterministic deployment). It also includes functions to predict the addresses of clones deployed using the - * deterministic method. - */ -library Clones { - error CloneArgumentsTooLong(); - - /** - * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation`. - * - * This function uses the create opcode, which should never revert. - */ - function clone(address implementation) internal returns (address instance) { - return clone(implementation, 0); - } - - /** - * @dev Same as {xref-Clones-clone-address-}[clone], but with a `value` parameter to send native currency - * to the new contract. - * - * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory) - * to always have enough balance for new deployments. Consider exposing this function under a payable method. - */ - function clone(address implementation, uint256 value) internal returns (address instance) { - if (address(this).balance < value) { - revert Errors.InsufficientBalance(address(this).balance, value); - } - assembly ("memory-safe") { - // Cleans the upper 96 bits of the `implementation` word, then packs the first 3 bytes - // of the `implementation` address with the bytecode before the address. - mstore(0x00, or(shr(0xe8, shl(0x60, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000)) - // Packs the remaining 17 bytes of `implementation` with the bytecode after the address. - mstore(0x20, or(shl(0x78, implementation), 0x5af43d82803e903d91602b57fd5bf3)) - instance := create(value, 0x09, 0x37) - } - if (instance == address(0)) { - revert Errors.FailedDeployment(); - } - } - - /** - * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation`. - * - * This function uses the create2 opcode and a `salt` to deterministically deploy - * the clone. Using the same `implementation` and `salt` multiple times will revert, since - * the clones cannot be deployed twice at the same address. - */ - function cloneDeterministic(address implementation, bytes32 salt) internal returns (address instance) { - return cloneDeterministic(implementation, salt, 0); - } - - /** - * @dev Same as {xref-Clones-cloneDeterministic-address-bytes32-}[cloneDeterministic], but with - * a `value` parameter to send native currency to the new contract. - * - * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory) - * to always have enough balance for new deployments. Consider exposing this function under a payable method. - */ - function cloneDeterministic( - address implementation, - bytes32 salt, - uint256 value - ) internal returns (address instance) { - if (address(this).balance < value) { - revert Errors.InsufficientBalance(address(this).balance, value); - } - assembly ("memory-safe") { - // Cleans the upper 96 bits of the `implementation` word, then packs the first 3 bytes - // of the `implementation` address with the bytecode before the address. - mstore(0x00, or(shr(0xe8, shl(0x60, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000)) - // Packs the remaining 17 bytes of `implementation` with the bytecode after the address. - mstore(0x20, or(shl(0x78, implementation), 0x5af43d82803e903d91602b57fd5bf3)) - instance := create2(value, 0x09, 0x37, salt) - } - if (instance == address(0)) { - revert Errors.FailedDeployment(); - } - } - - /** - * @dev Computes the address of a clone deployed using {Clones-cloneDeterministic}. - */ - function predictDeterministicAddress( - address implementation, - bytes32 salt, - address deployer - ) internal pure returns (address predicted) { - assembly ("memory-safe") { - let ptr := mload(0x40) - mstore(add(ptr, 0x38), deployer) - mstore(add(ptr, 0x24), 0x5af43d82803e903d91602b57fd5bf3ff) - mstore(add(ptr, 0x14), implementation) - mstore(ptr, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73) - mstore(add(ptr, 0x58), salt) - mstore(add(ptr, 0x78), keccak256(add(ptr, 0x0c), 0x37)) - predicted := and(keccak256(add(ptr, 0x43), 0x55), 0xffffffffffffffffffffffffffffffffffffffff) - } - } - - /** - * @dev Computes the address of a clone deployed using {Clones-cloneDeterministic}. - */ - function predictDeterministicAddress( - address implementation, - bytes32 salt - ) internal view returns (address predicted) { - return predictDeterministicAddress(implementation, salt, address(this)); - } - - /** - * @dev Deploys and returns the address of a clone that mimics the behavior of `implementation` with custom - * immutable arguments. These are provided through `args` and cannot be changed after deployment. To - * access the arguments within the implementation, use {fetchCloneArgs}. - * - * This function uses the create opcode, which should never revert. - */ - function cloneWithImmutableArgs(address implementation, bytes memory args) internal returns (address instance) { - return cloneWithImmutableArgs(implementation, args, 0); - } - - /** - * @dev Same as {xref-Clones-cloneWithImmutableArgs-address-bytes-}[cloneWithImmutableArgs], but with a `value` - * parameter to send native currency to the new contract. - * - * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory) - * to always have enough balance for new deployments. Consider exposing this function under a payable method. - */ - function cloneWithImmutableArgs( - address implementation, - bytes memory args, - uint256 value - ) internal returns (address instance) { - if (address(this).balance < value) { - revert Errors.InsufficientBalance(address(this).balance, value); - } - bytes memory bytecode = _cloneCodeWithImmutableArgs(implementation, args); - assembly ("memory-safe") { - instance := create(value, add(bytecode, 0x20), mload(bytecode)) - } - if (instance == address(0)) { - revert Errors.FailedDeployment(); - } - } - - /** - * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation` with custom - * immutable arguments. These are provided through `args` and cannot be changed after deployment. To - * access the arguments within the implementation, use {fetchCloneArgs}. - * - * This function uses the create2 opcode and a `salt` to deterministically deploy the clone. Using the same - * `implementation`, `args` and `salt` multiple times will revert, since the clones cannot be deployed twice - * at the same address. - */ - function cloneDeterministicWithImmutableArgs( - address implementation, - bytes memory args, - bytes32 salt - ) internal returns (address instance) { - return cloneDeterministicWithImmutableArgs(implementation, args, salt, 0); - } - - /** - * @dev Same as {xref-Clones-cloneDeterministicWithImmutableArgs-address-bytes-bytes32-}[cloneDeterministicWithImmutableArgs], - * but with a `value` parameter to send native currency to the new contract. - * - * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory) - * to always have enough balance for new deployments. Consider exposing this function under a payable method. - */ - function cloneDeterministicWithImmutableArgs( - address implementation, - bytes memory args, - bytes32 salt, - uint256 value - ) internal returns (address instance) { - bytes memory bytecode = _cloneCodeWithImmutableArgs(implementation, args); - return Create2.deploy(value, salt, bytecode); - } - - /** - * @dev Computes the address of a clone deployed using {Clones-cloneDeterministicWithImmutableArgs}. - */ - function predictDeterministicAddressWithImmutableArgs( - address implementation, - bytes memory args, - bytes32 salt, - address deployer - ) internal pure returns (address predicted) { - bytes memory bytecode = _cloneCodeWithImmutableArgs(implementation, args); - return Create2.computeAddress(salt, keccak256(bytecode), deployer); - } - - /** - * @dev Computes the address of a clone deployed using {Clones-cloneDeterministicWithImmutableArgs}. - */ - function predictDeterministicAddressWithImmutableArgs( - address implementation, - bytes memory args, - bytes32 salt - ) internal view returns (address predicted) { - return predictDeterministicAddressWithImmutableArgs(implementation, args, salt, address(this)); - } - - /** - * @dev Get the immutable args attached to a clone. - * - * - If `instance` is a clone that was deployed using `clone` or `cloneDeterministic`, this - * function will return an empty array. - * - If `instance` is a clone that was deployed using `cloneWithImmutableArgs` or - * `cloneDeterministicWithImmutableArgs`, this function will return the args array used at - * creation. - * - If `instance` is NOT a clone deployed using this library, the behavior is undefined. This - * function should only be used to check addresses that are known to be clones. - */ - function fetchCloneArgs(address instance) internal view returns (bytes memory) { - bytes memory result = new bytes(instance.code.length - 45); // revert if length is too short - assembly ("memory-safe") { - extcodecopy(instance, add(result, 32), 45, mload(result)) - } - return result; - } - - /** - * @dev Helper that prepares the initcode of the proxy with immutable args. - * - * An assembly variant of this function requires copying the `args` array, which can be efficiently done using - * `mcopy`. Unfortunately, that opcode is not available before cancun. A pure solidity implementation using - * abi.encodePacked is more expensive but also more portable and easier to review. - * - * NOTE: https://eips.ethereum.org/EIPS/eip-170[EIP-170] limits the length of the contract code to 24576 bytes. - * With the proxy code taking 45 bytes, that limits the length of the immutable args to 24531 bytes. - */ - function _cloneCodeWithImmutableArgs( - address implementation, - bytes memory args - ) private pure returns (bytes memory) { - if (args.length > 24531) revert CloneArgumentsTooLong(); - return - abi.encodePacked( - hex"61", - uint16(args.length + 45), - hex"3d81600a3d39f3363d3d373d3d3d363d73", - implementation, - hex"5af43d82803e903d91602b57fd5bf3", - args - ); - } -} diff --git a/contracts/openzeppelin/5.2.0/utils/Create2.sol b/contracts/openzeppelin/5.2.0/utils/Create2.sol deleted file mode 100644 index d61331741..000000000 --- a/contracts/openzeppelin/5.2.0/utils/Create2.sol +++ /dev/null @@ -1,92 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts (last updated v5.1.0) (utils/Create2.sol) - -pragma solidity ^0.8.20; - -import {Errors} from "./Errors.sol"; - -/** - * @dev Helper to make usage of the `CREATE2` EVM opcode easier and safer. - * `CREATE2` can be used to compute in advance the address where a smart - * contract will be deployed, which allows for interesting new mechanisms known - * as 'counterfactual interactions'. - * - * See the https://eips.ethereum.org/EIPS/eip-1014#motivation[EIP] for more - * information. - */ -library Create2 { - /** - * @dev There's no code to deploy. - */ - error Create2EmptyBytecode(); - - /** - * @dev Deploys a contract using `CREATE2`. The address where the contract - * will be deployed can be known in advance via {computeAddress}. - * - * The bytecode for a contract can be obtained from Solidity with - * `type(contractName).creationCode`. - * - * Requirements: - * - * - `bytecode` must not be empty. - * - `salt` must have not been used for `bytecode` already. - * - the factory must have a balance of at least `amount`. - * - if `amount` is non-zero, `bytecode` must have a `payable` constructor. - */ - function deploy(uint256 amount, bytes32 salt, bytes memory bytecode) internal returns (address addr) { - if (address(this).balance < amount) { - revert Errors.InsufficientBalance(address(this).balance, amount); - } - if (bytecode.length == 0) { - revert Create2EmptyBytecode(); - } - assembly ("memory-safe") { - addr := create2(amount, add(bytecode, 0x20), mload(bytecode), salt) - // if no address was created, and returndata is not empty, bubble revert - if and(iszero(addr), not(iszero(returndatasize()))) { - let p := mload(0x40) - returndatacopy(p, 0, returndatasize()) - revert(p, returndatasize()) - } - } - if (addr == address(0)) { - revert Errors.FailedDeployment(); - } - } - - /** - * @dev Returns the address where a contract will be stored if deployed via {deploy}. Any change in the - * `bytecodeHash` or `salt` will result in a new destination address. - */ - function computeAddress(bytes32 salt, bytes32 bytecodeHash) internal view returns (address) { - return computeAddress(salt, bytecodeHash, address(this)); - } - - /** - * @dev Returns the address where a contract will be stored if deployed via {deploy} from a contract located at - * `deployer`. If `deployer` is this contract's address, returns the same value as {computeAddress}. - */ - function computeAddress(bytes32 salt, bytes32 bytecodeHash, address deployer) internal pure returns (address addr) { - assembly ("memory-safe") { - let ptr := mload(0x40) // Get free memory pointer - - // | | ↓ ptr ... ↓ ptr + 0x0B (start) ... ↓ ptr + 0x20 ... ↓ ptr + 0x40 ... | - // |-------------------|---------------------------------------------------------------------------| - // | bytecodeHash | CCCCCCCCCCCCC...CC | - // | salt | BBBBBBBBBBBBB...BB | - // | deployer | 000000...0000AAAAAAAAAAAAAAAAAAA...AA | - // | 0xFF | FF | - // |-------------------|---------------------------------------------------------------------------| - // | memory | 000000...00FFAAAAAAAAAAAAAAAAAAA...AABBBBBBBBBBBBB...BBCCCCCCCCCCCCC...CC | - // | keccak(start, 85) | ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑ | - - mstore(add(ptr, 0x40), bytecodeHash) - mstore(add(ptr, 0x20), salt) - mstore(ptr, deployer) // Right-aligned with 12 preceding garbage bytes - let start := add(ptr, 0x0b) // The hashed data starts at the final garbage byte which we will set to 0xff - mstore8(start, 0xff) - addr := and(keccak256(start, 85), 0xffffffffffffffffffffffffffffffffffffffff) - } - } -} diff --git a/contracts/openzeppelin/5.2.0/utils/Errors.sol b/contracts/openzeppelin/5.2.0/utils/Errors.sol deleted file mode 100644 index 442fc1892..000000000 --- a/contracts/openzeppelin/5.2.0/utils/Errors.sol +++ /dev/null @@ -1,34 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts (last updated v5.1.0) (utils/Errors.sol) - -pragma solidity ^0.8.20; - -/** - * @dev Collection of common custom errors used in multiple contracts - * - * IMPORTANT: Backwards compatibility is not guaranteed in future versions of the library. - * It is recommended to avoid relying on the error API for critical functionality. - * - * _Available since v5.1._ - */ -library Errors { - /** - * @dev The ETH balance of the account is not enough to perform the operation. - */ - error InsufficientBalance(uint256 balance, uint256 needed); - - /** - * @dev A call to an address target failed. The target may have reverted. - */ - error FailedCall(); - - /** - * @dev The deployment failed. - */ - error FailedDeployment(); - - /** - * @dev A necessary precompile is missing. - */ - error MissingPrecompile(address); -} diff --git a/package.json b/package.json index a8711c17c..d1e3a7836 100644 --- a/package.json +++ b/package.json @@ -110,6 +110,7 @@ "@openzeppelin/contracts": "3.4.0", "@openzeppelin/contracts-v4.4": "npm:@openzeppelin/contracts@4.4.1", "@openzeppelin/contracts-v5.0.2": "npm:@openzeppelin/contracts@5.0.2", + "@openzeppelin/contracts-v5.2.0": "npm:@openzeppelin/contracts@5.2.0", "openzeppelin-solidity": "2.0.0" } } diff --git a/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol b/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol index 596a0e67a..f5780b015 100644 --- a/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol +++ b/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol @@ -3,9 +3,9 @@ pragma solidity 0.8.25; -import {UpgradeableBeacon} from "@openzeppelin/contracts-v5.0.2/proxy/beacon/UpgradeableBeacon.sol"; -import {BeaconProxy} from "@openzeppelin/contracts-v5.0.2/proxy/beacon/BeaconProxy.sol"; -import {Clones} from "contracts/openzeppelin/5.2.0/proxy/Clones.sol"; +import {UpgradeableBeacon} from "@openzeppelin/contracts-v5.2.0/proxy/beacon/UpgradeableBeacon.sol"; +import {BeaconProxy} from "@openzeppelin/contracts-v5.2.0/proxy/beacon/BeaconProxy.sol"; +import {Clones} from "@openzeppelin/contracts-v5.2.0/proxy/Clones.sol"; import {IStakingVault} from "contracts/0.8.25/vaults/interfaces/IStakingVault.sol"; import {Dashboard} from "contracts/0.8.25/vaults/Dashboard.sol"; diff --git a/test/0.8.25/vaults/staking-vault/contracts/VaultFactory__MockForStakingVault.sol b/test/0.8.25/vaults/staking-vault/contracts/VaultFactory__MockForStakingVault.sol index 6cb53a18f..287ea3e4d 100644 --- a/test/0.8.25/vaults/staking-vault/contracts/VaultFactory__MockForStakingVault.sol +++ b/test/0.8.25/vaults/staking-vault/contracts/VaultFactory__MockForStakingVault.sol @@ -3,8 +3,8 @@ pragma solidity ^0.8.0; -import {UpgradeableBeacon} from "@openzeppelin/contracts-v5.0.2/proxy/beacon/UpgradeableBeacon.sol"; -import {BeaconProxy} from "@openzeppelin/contracts-v5.0.2/proxy/beacon/BeaconProxy.sol"; +import {UpgradeableBeacon} from "@openzeppelin/contracts-v5.2.0/proxy/beacon/UpgradeableBeacon.sol"; +import {BeaconProxy} from "@openzeppelin/contracts-v5.2.0/proxy/beacon/BeaconProxy.sol"; import {IStakingVault} from "contracts/0.8.25/vaults/interfaces/IStakingVault.sol"; contract VaultFactory__MockForStakingVault is UpgradeableBeacon { diff --git a/yarn.lock b/yarn.lock index c910ac91b..becd6884c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1617,6 +1617,13 @@ __metadata: languageName: node linkType: hard +"@openzeppelin/contracts-v5.2.0@npm:@openzeppelin/contracts@5.2.0": + version: 5.2.0 + resolution: "@openzeppelin/contracts@npm:5.2.0" + checksum: 10c0/6e2d8c6daaeb8e111d49a82c30997a6c5d4e512338b55500db7fd4340f29c1cbf35f9dcfa0dbc672e417bc84e99f5441a105cb585cd4680ad70cbcf9a24094fc + languageName: node + linkType: hard + "@openzeppelin/contracts@npm:3.4.0": version: 3.4.0 resolution: "@openzeppelin/contracts@npm:3.4.0" @@ -8064,6 +8071,7 @@ __metadata: "@openzeppelin/contracts": "npm:3.4.0" "@openzeppelin/contracts-v4.4": "npm:@openzeppelin/contracts@4.4.1" "@openzeppelin/contracts-v5.0.2": "npm:@openzeppelin/contracts@5.0.2" + "@openzeppelin/contracts-v5.2.0": "npm:@openzeppelin/contracts@5.2.0" "@typechain/ethers-v6": "npm:0.5.1" "@typechain/hardhat": "npm:9.1.0" "@types/chai": "npm:4.3.20" From 4d3f84d3bb490dd8c8a277d2e772f862e85cf056 Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Thu, 16 Jan 2025 20:23:53 +0300 Subject: [PATCH 13/15] feat: add proxy bytecode verification, remove implementation verification --- contracts/0.8.25/vaults/Dashboard.sol | 10 ++-- contracts/0.8.25/vaults/StakingVault.sol | 13 +---- contracts/0.8.25/vaults/VaultHub.sol | 45 +++++---------- .../0.8.25/vaults/interfaces/IBeaconProxy.sol | 10 ---- .../vaults/interfaces/IStakingVault.sol | 1 + scripts/scratch/steps/0145-deploy-vaults.ts | 9 ++- .../StakingVault__HarnessForTestUpgrade.sol | 56 ++++++++++++++++--- .../0.8.25/vaults/dashboard/dashboard.test.ts | 2 +- .../vaults/delegation/delegation.test.ts | 1 - .../staking-vault/staking-vault.test.ts | 12 +--- test/0.8.25/vaults/vaultFactory.test.ts | 38 +++++-------- 11 files changed, 92 insertions(+), 105 deletions(-) delete mode 100644 contracts/0.8.25/vaults/interfaces/IBeaconProxy.sol diff --git a/contracts/0.8.25/vaults/Dashboard.sol b/contracts/0.8.25/vaults/Dashboard.sol index 3bb4c8ddf..d2b2a546a 100644 --- a/contracts/0.8.25/vaults/Dashboard.sol +++ b/contracts/0.8.25/vaults/Dashboard.sol @@ -44,9 +44,6 @@ contract Dashboard is AccessControlEnumerable { /// @notice Total basis points for fee calculations; equals to 100%. uint256 internal constant TOTAL_BASIS_POINTS = 10000; - /// @notice Indicates whether the contract has been initialized - bool public isInitialized; - /// @notice The stETH token contract IStETH public immutable STETH; @@ -56,6 +53,9 @@ contract Dashboard is AccessControlEnumerable { /// @notice The wrapped ether token contract IWeth public immutable WETH; + /// @notice Indicates whether the contract has been initialized + bool public initialized; + /// @notice The underlying `StakingVault` contract IStakingVault public stakingVault; @@ -101,10 +101,10 @@ contract Dashboard is AccessControlEnumerable { */ function _initialize(address _stakingVault) internal { if (_stakingVault == address(0)) revert ZeroArgument("_stakingVault"); - if (isInitialized) revert AlreadyInitialized(); + if (initialized) revert AlreadyInitialized(); if (address(this) == _SELF) revert NonProxyCallsForbidden(); - isInitialized = true; + initialized = true; stakingVault = IStakingVault(_stakingVault); vaultHub = VaultHub(stakingVault.vaultHub()); _grantRole(DEFAULT_ADMIN_ROLE, msg.sender); diff --git a/contracts/0.8.25/vaults/StakingVault.sol b/contracts/0.8.25/vaults/StakingVault.sol index 6c0f55762..2a63e2ffa 100644 --- a/contracts/0.8.25/vaults/StakingVault.sol +++ b/contracts/0.8.25/vaults/StakingVault.sol @@ -9,9 +9,6 @@ import {BeaconChainDepositLogistics} from "./BeaconChainDepositLogistics.sol"; import {VaultHub} from "./VaultHub.sol"; import {IStakingVault} from "./interfaces/IStakingVault.sol"; -import {IBeaconProxy} from "./interfaces/IBeaconProxy.sol"; - -import {ERC1967Utils} from "@openzeppelin/contracts-v5.0.2/proxy/ERC1967/ERC1967Utils.sol"; /** * @title StakingVault @@ -52,7 +49,7 @@ import {ERC1967Utils} from "@openzeppelin/contracts-v5.0.2/proxy/ERC1967/ERC1967 * deposit contract. * */ -contract StakingVault is IStakingVault, IBeaconProxy, BeaconChainDepositLogistics, OwnableUpgradeable { +contract StakingVault is IStakingVault, BeaconChainDepositLogistics, OwnableUpgradeable { /** * @notice ERC-7201 storage namespace for the vault * @dev ERC-7201 namespace is used to prevent upgrade collisions @@ -133,14 +130,6 @@ contract StakingVault is IStakingVault, IBeaconProxy, BeaconChainDepositLogistic return _VERSION; } - /** - * @notice Returns the beacon proxy address that controls this contract's implementation - * @return address The beacon proxy address - */ - function beacon() public view returns (address) { - return ERC1967Utils.getBeacon(); - } - // * * * * * * * * * * * * * * * * * * * * // // * * * STAKING VAULT BUSINESS LOGIC * * * // // * * * * * * * * * * * * * * * * * * * * // diff --git a/contracts/0.8.25/vaults/VaultHub.sol b/contracts/0.8.25/vaults/VaultHub.sol index 425f0b6e4..d6ec85a17 100644 --- a/contracts/0.8.25/vaults/VaultHub.sol +++ b/contracts/0.8.25/vaults/VaultHub.sol @@ -10,7 +10,6 @@ import {OwnableUpgradeable} from "contracts/openzeppelin/5.0.2/upgradeable/acces import {IStakingVault} from "./interfaces/IStakingVault.sol"; import {ILido as IStETH} from "../interfaces/ILido.sol"; -import {IBeaconProxy} from "./interfaces/IBeaconProxy.sol"; import {Math256} from "contracts/common/lib/Math256.sol"; @@ -29,9 +28,7 @@ abstract contract VaultHub is AccessControlEnumerableUpgradeable { /// @dev if vault is not connected to the hub, its index is zero mapping(address => uint256) vaultIndex; /// @notice allowed beacon addresses - mapping(address => bool) vaultBeacons; - /// @notice allowed vault implementation addresses - mapping(address => bool) vaultImpl; + mapping(bytes32 => bool) vaultProxyCodehash; } struct VaultSocket { @@ -91,26 +88,15 @@ abstract contract VaultHub is AccessControlEnumerableUpgradeable { _grantRole(DEFAULT_ADMIN_ROLE, _admin); } - /// @notice added beacon address to allowed list - /// @param beacon beacon address - function addBeacon(address beacon) public onlyRole(VAULT_REGISTRY_ROLE) { - if (beacon == address(0)) revert ZeroArgument("beacon"); + /// @notice added vault proxy codehash to allowed list + /// @param codehash vault proxy codehash + function addVaultProxyCodehash(bytes32 codehash) public onlyRole(VAULT_REGISTRY_ROLE) { + if (codehash == bytes32(0)) revert ZeroArgument("codehash"); VaultHubStorage storage $ = _getVaultHubStorage(); - if ($.vaultBeacons[beacon]) revert AlreadyExists(beacon); - $.vaultBeacons[beacon] = true; - emit VaultBeaconAdded(beacon); - } - - /// @notice added vault implementation address to allowed list - /// @param impl vault implementation address - function addVaultImpl(address impl) public onlyRole(VAULT_REGISTRY_ROLE) { - if (impl == address(0)) revert ZeroArgument("impl"); - - VaultHubStorage storage $ = _getVaultHubStorage(); - if ($.vaultImpl[impl]) revert AlreadyExists(impl); - $.vaultImpl[impl] = true; - emit VaultImplAdded(impl); + if ($.vaultProxyCodehash[codehash]) revert AlreadyExists(codehash); + $.vaultProxyCodehash[codehash] = true; + emit VaultProxyCodehashAdded(codehash); } /// @notice returns the number of vaults connected to the hub @@ -163,11 +149,8 @@ abstract contract VaultHub is AccessControlEnumerableUpgradeable { VaultHubStorage storage $ = _getVaultHubStorage(); if ($.vaultIndex[_vault] != 0) revert AlreadyConnected(_vault, $.vaultIndex[_vault]); - address vaultBeacon = IBeaconProxy(address (_vault)).beacon(); - if (!$.vaultBeacons[vaultBeacon]) revert BeaconNotAllowed(vaultBeacon); - - address impl = IBeacon(vaultBeacon).implementation(); - if (!$.vaultImpl[impl]) revert ImplNotAllowed(impl); + bytes32 vaultProxyCodehash = address(_vault).codehash; + if (!$.vaultProxyCodehash[vaultProxyCodehash]) revert VaultProxyNotAllowed(_vault); VaultSocket memory vr = VaultSocket( _vault, @@ -524,8 +507,7 @@ abstract contract VaultHub is AccessControlEnumerableUpgradeable { event MintedSharesOnVault(address indexed vault, uint256 amountOfShares); event BurnedSharesOnVault(address indexed vault, uint256 amountOfShares); event VaultRebalanced(address indexed vault, uint256 sharesBurned); - event VaultImplAdded(address indexed impl); - event VaultBeaconAdded(address indexed beacon); + event VaultProxyCodehashAdded(bytes32 indexed codehash); error StETHMintFailed(address vault); error AlreadyBalanced(address vault, uint256 mintedShares, uint256 rebalancingThresholdInShares); @@ -543,8 +525,7 @@ abstract contract VaultHub is AccessControlEnumerableUpgradeable { error TreasuryFeeTooHigh(address vault, uint256 treasuryFeeBP, uint256 maxTreasuryFeeBP); error ExternalSharesCapReached(address vault, uint256 capShares, uint256 maxMintableExternalShares); error InsufficientValuationToMint(address vault, uint256 valuation); - error AlreadyExists(address addr); - error ImplNotAllowed(address impl); + error AlreadyExists(bytes32 codehash); error NoMintedSharesShouldBeLeft(address vault, uint256 sharesMinted); - error BeaconNotAllowed(address beacon); + error VaultProxyNotAllowed(address beacon); } diff --git a/contracts/0.8.25/vaults/interfaces/IBeaconProxy.sol b/contracts/0.8.25/vaults/interfaces/IBeaconProxy.sol deleted file mode 100644 index c49bf63c4..000000000 --- a/contracts/0.8.25/vaults/interfaces/IBeaconProxy.sol +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-FileCopyrightText: 2024 Lido -// SPDX-License-Identifier: GPL-3.0 - -// See contracts/COMPILERS.md -pragma solidity 0.8.25; - -interface IBeaconProxy { - function beacon() external view returns (address); - function version() external pure returns(uint64); -} diff --git a/contracts/0.8.25/vaults/interfaces/IStakingVault.sol b/contracts/0.8.25/vaults/interfaces/IStakingVault.sol index 54d597073..e7d0df602 100644 --- a/contracts/0.8.25/vaults/interfaces/IStakingVault.sol +++ b/contracts/0.8.25/vaults/interfaces/IStakingVault.sol @@ -21,6 +21,7 @@ interface IStakingVault { } function initialize(address _owner, address _operator, bytes calldata _params) external; + function version() external pure returns(uint64); function getInitializedVersion() external view returns (uint64); function vaultHub() external view returns (address); function operator() external view returns (address); diff --git a/scripts/scratch/steps/0145-deploy-vaults.ts b/scripts/scratch/steps/0145-deploy-vaults.ts index f91233f96..ddd879311 100644 --- a/scripts/scratch/steps/0145-deploy-vaults.ts +++ b/scripts/scratch/steps/0145-deploy-vaults.ts @@ -1,3 +1,4 @@ +import { keccak256 } from "ethers"; import { ethers } from "hardhat"; import { Accounting } from "typechain-types"; @@ -36,6 +37,11 @@ export async function main() { const beacon = await deployWithoutProxy(Sk.stakingVaultBeacon, "UpgradeableBeacon", deployer, [impAddress, deployer]); const beaconAddress = await beacon.getAddress(); + // Deploy BeaconProxy to get bytecode and add it to whitelist + const vaultBeaconProxy = await ethers.deployContract("BeaconProxy", [beaconAddress, "0x"]); + const vaultBeaconProxyCode = await ethers.provider.getCode(await vaultBeaconProxy.getAddress()); + const vaultBeaconProxyCodeHash = keccak256(vaultBeaconProxyCode); + // Deploy VaultFactory contract const factory = await deployWithoutProxy(Sk.stakingVaultFactory, "VaultFactory", deployer, [ beaconAddress, @@ -53,8 +59,7 @@ export async function main() { await makeTx(accounting, "grantRole", [vaultMasterRole, deployer], { from: deployer }); await makeTx(accounting, "grantRole", [vaultRegistryRole, deployer], { from: deployer }); - await makeTx(accounting, "addBeacon", [beaconAddress], { from: deployer }); - await makeTx(accounting, "addVaultImpl", [impAddress], { from: deployer }); + await makeTx(accounting, "addVaultProxyCodehash", [vaultBeaconProxyCodeHash], { from: deployer }); await makeTx(accounting, "renounceRole", [vaultMasterRole, deployer], { from: deployer }); await makeTx(accounting, "renounceRole", [vaultRegistryRole, deployer], { from: deployer }); diff --git a/test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol b/test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol index a6b22b756..ced641a7b 100644 --- a/test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol +++ b/test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol @@ -6,13 +6,11 @@ pragma solidity 0.8.25; import {OwnableUpgradeable} from "contracts/openzeppelin/5.0.2/upgradeable/access/OwnableUpgradeable.sol"; import {SafeCast} from "@openzeppelin/contracts-v5.0.2/utils/math/SafeCast.sol"; import {IERC20} from "@openzeppelin/contracts-v5.0.2/token/ERC20/IERC20.sol"; -import {ERC1967Utils} from "@openzeppelin/contracts-v5.0.2/proxy/ERC1967/ERC1967Utils.sol"; import {VaultHub} from "contracts/0.8.25/vaults/VaultHub.sol"; import {IStakingVault} from "contracts/0.8.25/vaults/interfaces/IStakingVault.sol"; -import {IBeaconProxy} from "contracts/0.8.25/vaults/interfaces/IBeaconProxy.sol"; import {BeaconChainDepositLogistics} from "contracts/0.8.25/vaults/BeaconChainDepositLogistics.sol"; -contract StakingVault__HarnessForTestUpgrade is IBeaconProxy, BeaconChainDepositLogistics, OwnableUpgradeable { +contract StakingVault__HarnessForTestUpgrade is IStakingVault, BeaconChainDepositLogistics, OwnableUpgradeable { /// @custom:storage-location erc7201:StakingVault.Vault struct VaultStorage { IStakingVault.Report report; @@ -22,7 +20,7 @@ contract StakingVault__HarnessForTestUpgrade is IBeaconProxy, BeaconChainDeposit } uint64 private constant _version = 2; - VaultHub public immutable vaultHub; + VaultHub private immutable VAULT_HUB; /// keccak256(abi.encode(uint256(keccak256("StakingVault.Vault")) - 1)) & ~bytes32(uint256(0xff)); bytes32 private constant VAULT_STORAGE_LOCATION = @@ -34,7 +32,10 @@ contract StakingVault__HarnessForTestUpgrade is IBeaconProxy, BeaconChainDeposit ) BeaconChainDepositLogistics(_beaconChainDepositContract) { if (_vaultHub == address(0)) revert ZeroArgument("_vaultHub"); - vaultHub = VaultHub(_vaultHub); + VAULT_HUB = VaultHub(_vaultHub); + + // Prevents reinitialization of the implementation + _disableInitializers(); } /// @notice Initialize the contract storage explicitly. Only new contracts can be initialized here. @@ -68,10 +69,6 @@ contract StakingVault__HarnessForTestUpgrade is IBeaconProxy, BeaconChainDeposit return _version; } - function beacon() public view returns (address) { - return ERC1967Utils.getBeacon(); - } - function latestReport() external view returns (IStakingVault.Report memory) { VaultStorage storage $ = _getVaultStorage(); return IStakingVault.Report({valuation: $.report.valuation, inOutDelta: $.report.inOutDelta}); @@ -83,6 +80,47 @@ contract StakingVault__HarnessForTestUpgrade is IBeaconProxy, BeaconChainDeposit } } + function depositToBeaconChain( + uint256 _numberOfDeposits, + bytes calldata _pubkeys, + bytes calldata _signatures + ) external {} + function fund() external payable {} + function inOutDelta() external view returns (int256) { + return -1; + } + function isBalanced() external view returns (bool) { + return true; + } + function operator() external view returns (address) { + return _getVaultStorage().operator; + } + function rebalance(uint256 _ether) external {} + function report(uint256 _valuation, int256 _inOutDelta, uint256 _locked) external {} + function requestValidatorExit(bytes calldata _pubkeys) external {} + function lock(uint256 _locked) external {} + + function locked() external view returns (uint256) { + return 0; + } + function unlocked() external view returns (uint256) { + return 0; + } + + function valuation() external view returns (uint256) { + return 0; + } + + function vaultHub() external view returns (address) { + return address(VAULT_HUB); + } + + function withdraw(address _recipient, uint256 _ether) external {} + + function withdrawalCredentials() external view returns (bytes32) { + return bytes32((0x01 << 248) + uint160(address(this))); + } + error ZeroArgument(string name); error VaultAlreadyInitialized(); } diff --git a/test/0.8.25/vaults/dashboard/dashboard.test.ts b/test/0.8.25/vaults/dashboard/dashboard.test.ts index 616f9f48d..101883cf9 100644 --- a/test/0.8.25/vaults/dashboard/dashboard.test.ts +++ b/test/0.8.25/vaults/dashboard/dashboard.test.ts @@ -140,7 +140,7 @@ describe("Dashboard.sol", () => { it("post-initialization state is correct", async () => { expect(await vault.owner()).to.equal(dashboard); expect(await vault.operator()).to.equal(operator); - expect(await dashboard.isInitialized()).to.equal(true); + expect(await dashboard.initialized()).to.equal(true); expect(await dashboard.stakingVault()).to.equal(vault); expect(await dashboard.vaultHub()).to.equal(hub); expect(await dashboard.STETH()).to.equal(steth); diff --git a/test/0.8.25/vaults/delegation/delegation.test.ts b/test/0.8.25/vaults/delegation/delegation.test.ts index 374b1246b..7525c0069 100644 --- a/test/0.8.25/vaults/delegation/delegation.test.ts +++ b/test/0.8.25/vaults/delegation/delegation.test.ts @@ -97,7 +97,6 @@ describe("Delegation.sol", () => { const stakingVaultAddress = vaultCreatedEvents[0].args.vault; vault = await ethers.getContractAt("StakingVault", stakingVaultAddress, vaultOwner); - expect(await vault.beacon()).to.equal(beacon); const delegationCreatedEvents = findEvents(vaultCreationReceipt, "DelegationCreated"); expect(delegationCreatedEvents.length).to.equal(1); diff --git a/test/0.8.25/vaults/staking-vault/staking-vault.test.ts b/test/0.8.25/vaults/staking-vault/staking-vault.test.ts index 1d4ad2904..2df2c8e3e 100644 --- a/test/0.8.25/vaults/staking-vault/staking-vault.test.ts +++ b/test/0.8.25/vaults/staking-vault/staking-vault.test.ts @@ -26,7 +26,6 @@ describe("StakingVault.sol", () => { let vaultOwner: HardhatEthersSigner; let operator: HardhatEthersSigner; let stranger: HardhatEthersSigner; - let beaconSigner: HardhatEthersSigner; let elRewardsSender: HardhatEthersSigner; let vaultHubSigner: HardhatEthersSigner; @@ -34,21 +33,18 @@ describe("StakingVault.sol", () => { let stakingVaultImplementation: StakingVault; let depositContract: DepositContract__MockForStakingVault; let vaultHub: VaultHub__MockForStakingVault; - let vaultFactory: VaultFactory__MockForStakingVault; let ethRejector: EthRejector; let vaultOwnerAddress: string; let stakingVaultAddress: string; let vaultHubAddress: string; - let vaultFactoryAddress: string; let depositContractAddress: string; - let beaconAddress: string; let ethRejectorAddress: string; let originalState: string; before(async () => { [vaultOwner, operator, elRewardsSender, stranger] = await ethers.getSigners(); - [stakingVault, vaultHub, vaultFactory, stakingVaultImplementation, depositContract] = + [stakingVault, vaultHub /* vaultFactory */, , stakingVaultImplementation, depositContract] = await deployStakingVaultBehindBeaconProxy(); ethRejector = await ethers.deployContract("EthRejector"); @@ -56,11 +52,8 @@ describe("StakingVault.sol", () => { stakingVaultAddress = await stakingVault.getAddress(); vaultHubAddress = await vaultHub.getAddress(); depositContractAddress = await depositContract.getAddress(); - beaconAddress = await stakingVaultImplementation.beacon(); - vaultFactoryAddress = await vaultFactory.getAddress(); ethRejectorAddress = await ethRejector.getAddress(); - beaconSigner = await impersonate(beaconAddress, ether("10")); vaultHubSigner = await impersonate(vaultHubAddress, ether("10")); }); @@ -101,7 +94,7 @@ describe("StakingVault.sol", () => { it("reverts on initialization", async () => { await expect( - stakingVaultImplementation.connect(beaconSigner).initialize(vaultOwner, operator, "0x"), + stakingVaultImplementation.connect(stranger).initialize(vaultOwner, operator, "0x"), ).to.be.revertedWithCustomError(stakingVaultImplementation, "InvalidInitialization"); }); }); @@ -112,7 +105,6 @@ describe("StakingVault.sol", () => { expect(await stakingVault.getInitializedVersion()).to.equal(1n); expect(await stakingVault.vaultHub()).to.equal(vaultHubAddress); expect(await stakingVault.DEPOSIT_CONTRACT()).to.equal(depositContractAddress); - expect(await stakingVault.beacon()).to.equal(vaultFactoryAddress); expect(await stakingVault.owner()).to.equal(await vaultOwner.getAddress()); expect(await stakingVault.operator()).to.equal(operator); diff --git a/test/0.8.25/vaults/vaultFactory.test.ts b/test/0.8.25/vaults/vaultFactory.test.ts index 98233d67d..765946c65 100644 --- a/test/0.8.25/vaults/vaultFactory.test.ts +++ b/test/0.8.25/vaults/vaultFactory.test.ts @@ -1,11 +1,12 @@ import { expect } from "chai"; -import { ZeroAddress } from "ethers"; +import { keccak256, ZeroAddress } from "ethers"; import { ethers } from "hardhat"; import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers"; import { Accounting, + BeaconProxy, Delegation, DepositContract__MockForBeaconChainDepositor, LidoLocator, @@ -49,6 +50,9 @@ describe("VaultFactory.sol", () => { let locator: LidoLocator; + let vaultBeaconProxy: BeaconProxy; + let vaultBeaconProxyCode: string; + let originalState: string; before(async () => { @@ -78,6 +82,9 @@ describe("VaultFactory.sol", () => { //beacon beacon = await ethers.deployContract("UpgradeableBeacon", [implOld, admin]); + vaultBeaconProxy = await ethers.deployContract("BeaconProxy", [beacon, "0x"]); + vaultBeaconProxyCode = await ethers.provider.getCode(await vaultBeaconProxy.getAddress()); + delegation = await ethers.deployContract("Delegation", [steth, weth, wsteth], { from: deployer }); vaultFactory = await ethers.deployContract("VaultFactory", [beacon, delegation], { from: deployer }); @@ -164,7 +171,6 @@ describe("VaultFactory.sol", () => { .withArgs(await admin.getAddress(), await delegation_.getAddress()); expect(await delegation_.getAddress()).to.eq(await vault.owner()); - expect(await vault.beacon()).to.eq(await beacon.getAddress()); }); it("check `version()`", async () => { @@ -212,7 +218,7 @@ describe("VaultFactory.sol", () => { expect(await delegator1.getAddress()).to.eq(await vault1.owner()); expect(await delegator2.getAddress()).to.eq(await vault2.owner()); - //attempting to add a vault without adding a beacon to the allowed list + //attempting to add a vault without adding a proxy bytecode to the allowed list await expect( accounting .connect(admin) @@ -223,26 +229,12 @@ describe("VaultFactory.sol", () => { config1.thresholdReserveRatioBP, config1.treasuryFeeBP, ), - ).to.revertedWithCustomError(accounting, "BeaconNotAllowed"); + ).to.revertedWithCustomError(accounting, "VaultProxyNotAllowed"); - //add beacon to whitelist - await accounting.connect(admin).addBeacon(beacon); - - //attempting to add a vault without adding a implementation to the allowed list - await expect( - accounting - .connect(admin) - .connectVault( - await vault1.getAddress(), - config1.shareLimit, - config1.minReserveRatioBP, - config1.thresholdReserveRatioBP, - config1.treasuryFeeBP, - ), - ).to.revertedWithCustomError(accounting, "ImplNotAllowed"); + const vaultProxyCodeHash = keccak256(vaultBeaconProxyCode); - //add impl to whitelist - await accounting.connect(admin).addVaultImpl(implOld); + //add proxy code hash to whitelist + await accounting.connect(admin).addVaultProxyCodehash(vaultProxyCodeHash); //connect vault 1 to VaultHub await accounting @@ -273,7 +265,7 @@ describe("VaultFactory.sol", () => { //create new vault with new implementation const { vault: vault3 } = await createVaultProxy(vaultFactory, admin, vaultOwner1, operator); - //we upgrade implementation and do not add it to whitelist + //we upgrade implementation - we do not check implementation, just proxy bytecode await expect( accounting .connect(admin) @@ -284,7 +276,7 @@ describe("VaultFactory.sol", () => { config2.thresholdReserveRatioBP, config2.treasuryFeeBP, ), - ).to.revertedWithCustomError(accounting, "ImplNotAllowed"); + ).to.not.revertedWithCustomError(accounting, "VaultProxyNotAllowed"); const vault1WithNewImpl = await ethers.getContractAt("StakingVault__HarnessForTestUpgrade", vault1, deployer); const vault2WithNewImpl = await ethers.getContractAt("StakingVault__HarnessForTestUpgrade", vault2, deployer); From 444506aabf7a4f91b426f9ac421e29743349ea05 Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Mon, 20 Jan 2025 13:16:50 +0300 Subject: [PATCH 14/15] feat: change OZ version v5.0.2 -> v5.2 --- .../workflows/tests-integration-mainnet.yml | 1 - contracts/0.8.25/interfaces/ILido.sol | 4 +-- .../0.8.25/utils/PausableUntilWithRoles.sol | 2 +- contracts/0.8.25/vaults/Dashboard.sol | 10 +++--- contracts/0.8.25/vaults/StakingVault.sol | 2 +- contracts/0.8.25/vaults/VaultFactory.sol | 4 +-- contracts/0.8.25/vaults/VaultHub.sol | 4 +-- contracts/COMPILERS.md | 4 +-- .../access/AccessControlUpgradeable.sol | 15 ++++---- .../upgradeable/access/OwnableUpgradeable.sol | 0 .../AccessControlEnumerableUpgradeable.sol | 35 ++++++++++++------- .../upgradeable/proxy/utils/Initializable.sol | 0 .../upgradeable/utils/ContextUpgradeable.sol | 7 ++-- .../utils/introspection/ERC165Upgradeable.sol | 13 +++---- package.json | 3 +- .../StakingVault__HarnessForTestUpgrade.sol | 6 ++-- ...kingVault__MockForVaultDelegationLayer.sol | 2 +- .../VaultFactory__MockForDashboard.sol | 6 ++-- .../contracts/StETH__MockForDelegation.sol | 2 +- .../VaultFactory__MockForStakingVault.sol | 4 +-- yarn.lock | 12 ++----- 21 files changed, 69 insertions(+), 67 deletions(-) rename contracts/openzeppelin/{5.0.2 => 5.2}/upgradeable/access/AccessControlUpgradeable.sol (95%) rename contracts/openzeppelin/{5.0.2 => 5.2}/upgradeable/access/OwnableUpgradeable.sol (100%) rename contracts/openzeppelin/{5.0.2 => 5.2}/upgradeable/access/extensions/AccessControlEnumerableUpgradeable.sol (74%) rename contracts/openzeppelin/{5.0.2 => 5.2}/upgradeable/proxy/utils/Initializable.sol (100%) rename contracts/openzeppelin/{5.0.2 => 5.2}/upgradeable/utils/ContextUpgradeable.sol (93%) rename contracts/openzeppelin/{5.0.2 => 5.2}/upgradeable/utils/introspection/ERC165Upgradeable.sol (71%) diff --git a/.github/workflows/tests-integration-mainnet.yml b/.github/workflows/tests-integration-mainnet.yml index dcee343ea..742776c25 100644 --- a/.github/workflows/tests-integration-mainnet.yml +++ b/.github/workflows/tests-integration-mainnet.yml @@ -1,5 +1,4 @@ name: Integration Tests - #on: [push] # #jobs: diff --git a/contracts/0.8.25/interfaces/ILido.sol b/contracts/0.8.25/interfaces/ILido.sol index 14d65ec5a..faf58a415 100644 --- a/contracts/0.8.25/interfaces/ILido.sol +++ b/contracts/0.8.25/interfaces/ILido.sol @@ -4,8 +4,8 @@ // See contracts/COMPILERS.md pragma solidity 0.8.25; -import {IERC20} from "@openzeppelin/contracts-v5.0.2/token/ERC20/IERC20.sol"; -import {IERC20Permit} from "@openzeppelin/contracts-v5.0.2/token/ERC20/extensions/IERC20Permit.sol"; +import {IERC20} from "@openzeppelin/contracts-v5.2/token/ERC20/IERC20.sol"; +import {IERC20Permit} from "@openzeppelin/contracts-v5.2/token/ERC20/extensions/IERC20Permit.sol"; interface ILido is IERC20, IERC20Permit { function getSharesByPooledEth(uint256) external view returns (uint256); diff --git a/contracts/0.8.25/utils/PausableUntilWithRoles.sol b/contracts/0.8.25/utils/PausableUntilWithRoles.sol index e8c2d831b..1665e69c3 100644 --- a/contracts/0.8.25/utils/PausableUntilWithRoles.sol +++ b/contracts/0.8.25/utils/PausableUntilWithRoles.sol @@ -5,7 +5,7 @@ pragma solidity 0.8.25; import {PausableUntil} from "contracts/common/utils/PausableUntil.sol"; -import {AccessControlEnumerableUpgradeable} from "contracts/openzeppelin/5.0.2/upgradeable/access/extensions/AccessControlEnumerableUpgradeable.sol"; +import {AccessControlEnumerableUpgradeable} from "contracts/openzeppelin/5.2/upgradeable/access/extensions/AccessControlEnumerableUpgradeable.sol"; /** * @title PausableUntilWithRoles diff --git a/contracts/0.8.25/vaults/Dashboard.sol b/contracts/0.8.25/vaults/Dashboard.sol index 7edc5711c..b09cc6360 100644 --- a/contracts/0.8.25/vaults/Dashboard.sol +++ b/contracts/0.8.25/vaults/Dashboard.sol @@ -4,11 +4,11 @@ // See contracts/COMPILERS.md pragma solidity 0.8.25; -import {AccessControlEnumerable} from "@openzeppelin/contracts-v5.0.2/access/extensions/AccessControlEnumerable.sol"; -import {OwnableUpgradeable} from "contracts/openzeppelin/5.0.2/upgradeable/access/OwnableUpgradeable.sol"; -import {IERC20} from "@openzeppelin/contracts-v5.0.2/token/ERC20/IERC20.sol"; -import {IERC20Permit} from "@openzeppelin/contracts-v5.0.2/token/ERC20/extensions/IERC20Permit.sol"; -import {Clones} from "@openzeppelin/contracts-v5.2.0/proxy/Clones.sol"; +import {AccessControlEnumerable} from "@openzeppelin/contracts-v5.2/access/extensions/AccessControlEnumerable.sol"; +import {OwnableUpgradeable} from "contracts/openzeppelin/5.2/upgradeable/access/OwnableUpgradeable.sol"; +import {IERC20} from "@openzeppelin/contracts-v5.2/token/ERC20/IERC20.sol"; +import {IERC20Permit} from "@openzeppelin/contracts-v5.2/token/ERC20/extensions/IERC20Permit.sol"; +import {Clones} from "@openzeppelin/contracts-v5.2/proxy/Clones.sol"; import {Math256} from "contracts/common/lib/Math256.sol"; diff --git a/contracts/0.8.25/vaults/StakingVault.sol b/contracts/0.8.25/vaults/StakingVault.sol index 1a30edf3e..0224f7753 100644 --- a/contracts/0.8.25/vaults/StakingVault.sol +++ b/contracts/0.8.25/vaults/StakingVault.sol @@ -4,7 +4,7 @@ // See contracts/COMPILERS.md pragma solidity 0.8.25; -import {OwnableUpgradeable} from "contracts/openzeppelin/5.0.2/upgradeable/access/OwnableUpgradeable.sol"; +import {OwnableUpgradeable} from "contracts/openzeppelin/5.2/upgradeable/access/OwnableUpgradeable.sol"; import {BeaconChainDepositLogistics} from "./BeaconChainDepositLogistics.sol"; import {VaultHub} from "./VaultHub.sol"; diff --git a/contracts/0.8.25/vaults/VaultFactory.sol b/contracts/0.8.25/vaults/VaultFactory.sol index 1d36f83d9..99e92f110 100644 --- a/contracts/0.8.25/vaults/VaultFactory.sol +++ b/contracts/0.8.25/vaults/VaultFactory.sol @@ -4,8 +4,8 @@ // See contracts/COMPILERS.md pragma solidity 0.8.25; -import {BeaconProxy} from "@openzeppelin/contracts-v5.2.0/proxy/beacon/BeaconProxy.sol"; -import {Clones} from "@openzeppelin/contracts-v5.2.0/proxy/Clones.sol"; +import {BeaconProxy} from "@openzeppelin/contracts-v5.2/proxy/beacon/BeaconProxy.sol"; +import {Clones} from "@openzeppelin/contracts-v5.2/proxy/Clones.sol"; import {IStakingVault} from "./interfaces/IStakingVault.sol"; diff --git a/contracts/0.8.25/vaults/VaultHub.sol b/contracts/0.8.25/vaults/VaultHub.sol index 666a105bb..3c8d10b47 100644 --- a/contracts/0.8.25/vaults/VaultHub.sol +++ b/contracts/0.8.25/vaults/VaultHub.sol @@ -4,8 +4,8 @@ // See contracts/COMPILERS.md pragma solidity 0.8.25; -import {IBeacon} from "@openzeppelin/contracts-v5.0.2/proxy/beacon/IBeacon.sol"; -import {OwnableUpgradeable} from "contracts/openzeppelin/5.0.2/upgradeable/access/OwnableUpgradeable.sol"; +import {IBeacon} from "@openzeppelin/contracts-v5.2/proxy/beacon/IBeacon.sol"; +import {OwnableUpgradeable} from "contracts/openzeppelin/5.2/upgradeable/access/OwnableUpgradeable.sol"; import {IStakingVault} from "./interfaces/IStakingVault.sol"; import {ILido as IStETH} from "../interfaces/ILido.sol"; diff --git a/contracts/COMPILERS.md b/contracts/COMPILERS.md index ae89a8968..729afc963 100644 --- a/contracts/COMPILERS.md +++ b/contracts/COMPILERS.md @@ -11,9 +11,9 @@ For the `wstETH` contract, we use `solc 0.6.12`, as it is non-upgradeable and bo For the other contracts, newer compiler versions are used. -The 0.8.25 version of the compiler was introduced for Lido Vaults to be able to support [OpenZeppelin v5.0.2](https://github.com/OpenZeppelin/openzeppelin-contracts/tree/v5.0.2) dependencies (under the "@openzeppelin/contracts-v5.0.2" alias). +The 0.8.25 version of the compiler was introduced for Lido Vaults to be able to support [OpenZeppelin v5.2.0](https://github.com/OpenZeppelin/openzeppelin-contracts/tree/v5.2.0) dependencies (under the "@openzeppelin/contracts-v5.2" alias). -The OpenZeppelin 5.0.2 upgradeable contracts are copied locally in this repository (`contracts/openzeppelin/5.0.2`) instead of being imported from npm. This is because the original upgradeable contracts import from "@openzeppelin/contracts", but we use a custom alias "@openzeppelin/contracts-v5.0.2" to manage multiple OpenZeppelin versions. To resolve these import conflicts, we maintain local copies of the upgradeable contracts with corrected import paths that reference our aliased version. +The OpenZeppelin 5.2.0 upgradeable contracts are copied locally in this repository (`contracts/openzeppelin/5.2`) instead of being imported from npm. This is because the original upgradeable contracts import from "@openzeppelin/contracts", but we use a custom alias "@openzeppelin/contracts-v5.2" to manage multiple OpenZeppelin versions. To resolve these import conflicts, we maintain local copies of the upgradeable contracts with corrected import paths that reference our aliased version. # Compilation Instructions diff --git a/contracts/openzeppelin/5.0.2/upgradeable/access/AccessControlUpgradeable.sol b/contracts/openzeppelin/5.2/upgradeable/access/AccessControlUpgradeable.sol similarity index 95% rename from contracts/openzeppelin/5.0.2/upgradeable/access/AccessControlUpgradeable.sol rename to contracts/openzeppelin/5.2/upgradeable/access/AccessControlUpgradeable.sol index 26e403d26..3c9b67f05 100644 --- a/contracts/openzeppelin/5.0.2/upgradeable/access/AccessControlUpgradeable.sol +++ b/contracts/openzeppelin/5.2/upgradeable/access/AccessControlUpgradeable.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.20; -import {IAccessControl} from "@openzeppelin/contracts-v5.0.2/access/IAccessControl.sol"; +import {IAccessControl} from "@openzeppelin/contracts-v5.2/access/IAccessControl.sol"; import {ContextUpgradeable} from "../utils/ContextUpgradeable.sol"; import {ERC165Upgradeable} from "../utils/introspection/ERC165Upgradeable.sol"; import {Initializable} from "../proxy/utils/Initializable.sol"; @@ -55,14 +55,14 @@ abstract contract AccessControlUpgradeable is Initializable, ContextUpgradeable, bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00; + /// @custom:storage-location erc7201:openzeppelin.storage.AccessControl struct AccessControlStorage { mapping(bytes32 role => RoleData) _roles; } // keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.AccessControl")) - 1)) & ~bytes32(uint256(0xff)) - bytes32 private constant AccessControlStorageLocation = - 0x02dd7bc7dec4dceedda775e58dd541e08a116c6c53815c0bd028192f7b626800; + bytes32 private constant AccessControlStorageLocation = 0x02dd7bc7dec4dceedda775e58dd541e08a116c6c53815c0bd028192f7b626800; function _getAccessControlStorage() private pure returns (AccessControlStorage storage $) { assembly { @@ -79,10 +79,11 @@ abstract contract AccessControlUpgradeable is Initializable, ContextUpgradeable, _; } - function __AccessControl_init() internal onlyInitializing {} - - function __AccessControl_init_unchained() internal onlyInitializing {} + function __AccessControl_init() internal onlyInitializing { + } + function __AccessControl_init_unchained() internal onlyInitializing { + } /** * @dev See {IERC165-supportsInterface}. */ @@ -213,7 +214,7 @@ abstract contract AccessControlUpgradeable is Initializable, ContextUpgradeable, } /** - * @dev Attempts to revoke `role` to `account` and returns a boolean indicating if `role` was revoked. + * @dev Attempts to revoke `role` from `account` and returns a boolean indicating if `role` was revoked. * * Internal function without access restriction. * diff --git a/contracts/openzeppelin/5.0.2/upgradeable/access/OwnableUpgradeable.sol b/contracts/openzeppelin/5.2/upgradeable/access/OwnableUpgradeable.sol similarity index 100% rename from contracts/openzeppelin/5.0.2/upgradeable/access/OwnableUpgradeable.sol rename to contracts/openzeppelin/5.2/upgradeable/access/OwnableUpgradeable.sol diff --git a/contracts/openzeppelin/5.0.2/upgradeable/access/extensions/AccessControlEnumerableUpgradeable.sol b/contracts/openzeppelin/5.2/upgradeable/access/extensions/AccessControlEnumerableUpgradeable.sol similarity index 74% rename from contracts/openzeppelin/5.0.2/upgradeable/access/extensions/AccessControlEnumerableUpgradeable.sol rename to contracts/openzeppelin/5.2/upgradeable/access/extensions/AccessControlEnumerableUpgradeable.sol index 83759584b..9fbf69e08 100644 --- a/contracts/openzeppelin/5.0.2/upgradeable/access/extensions/AccessControlEnumerableUpgradeable.sol +++ b/contracts/openzeppelin/5.2/upgradeable/access/extensions/AccessControlEnumerableUpgradeable.sol @@ -1,21 +1,17 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts (last updated v5.0.0) (access/extensions/AccessControlEnumerable.sol) +// OpenZeppelin Contracts (last updated v5.1.0) (access/extensions/AccessControlEnumerable.sol) pragma solidity ^0.8.20; -import {IAccessControlEnumerable} from "@openzeppelin/contracts-v5.0.2/access/extensions/IAccessControlEnumerable.sol"; +import {IAccessControlEnumerable} from "@openzeppelin/contracts-v5.2/access/extensions/IAccessControlEnumerable.sol"; import {AccessControlUpgradeable} from "../AccessControlUpgradeable.sol"; -import {EnumerableSet} from "@openzeppelin/contracts-v5.0.2/utils/structs/EnumerableSet.sol"; +import {EnumerableSet} from "@openzeppelin/contracts-v5.2/utils/structs/EnumerableSet.sol"; import {Initializable} from "../../proxy/utils/Initializable.sol"; /** * @dev Extension of {AccessControl} that allows enumerating the members of each role. */ -abstract contract AccessControlEnumerableUpgradeable is - Initializable, - IAccessControlEnumerable, - AccessControlUpgradeable -{ +abstract contract AccessControlEnumerableUpgradeable is Initializable, IAccessControlEnumerable, AccessControlUpgradeable { using EnumerableSet for EnumerableSet.AddressSet; /// @custom:storage-location erc7201:openzeppelin.storage.AccessControlEnumerable @@ -24,8 +20,7 @@ abstract contract AccessControlEnumerableUpgradeable is } // keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.AccessControlEnumerable")) - 1)) & ~bytes32(uint256(0xff)) - bytes32 private constant AccessControlEnumerableStorageLocation = - 0xc1f6fe24621ce81ec5827caf0253cadb74709b061630e6b55e82371705932000; + bytes32 private constant AccessControlEnumerableStorageLocation = 0xc1f6fe24621ce81ec5827caf0253cadb74709b061630e6b55e82371705932000; function _getAccessControlEnumerableStorage() private pure returns (AccessControlEnumerableStorage storage $) { assembly { @@ -33,10 +28,11 @@ abstract contract AccessControlEnumerableUpgradeable is } } - function __AccessControlEnumerable_init() internal onlyInitializing {} - - function __AccessControlEnumerable_init_unchained() internal onlyInitializing {} + function __AccessControlEnumerable_init() internal onlyInitializing { + } + function __AccessControlEnumerable_init_unchained() internal onlyInitializing { + } /** * @dev See {IERC165-supportsInterface}. */ @@ -70,6 +66,19 @@ abstract contract AccessControlEnumerableUpgradeable is return $._roleMembers[role].length(); } + /** + * @dev Return all accounts that have `role` + * + * WARNING: This operation will copy the entire storage to memory, which can be quite expensive. This is designed + * to mostly be used by view accessors that are queried without any gas fees. Developers should keep in mind that + * this function has an unbounded cost, and using it as part of a state-changing function may render the function + * uncallable if the set grows to a point where copying to memory consumes too much gas to fit in a block. + */ + function getRoleMembers(bytes32 role) public view virtual returns (address[] memory) { + AccessControlEnumerableStorage storage $ = _getAccessControlEnumerableStorage(); + return $._roleMembers[role].values(); + } + /** * @dev Overload {AccessControl-_grantRole} to track enumerable memberships */ diff --git a/contracts/openzeppelin/5.0.2/upgradeable/proxy/utils/Initializable.sol b/contracts/openzeppelin/5.2/upgradeable/proxy/utils/Initializable.sol similarity index 100% rename from contracts/openzeppelin/5.0.2/upgradeable/proxy/utils/Initializable.sol rename to contracts/openzeppelin/5.2/upgradeable/proxy/utils/Initializable.sol diff --git a/contracts/openzeppelin/5.0.2/upgradeable/utils/ContextUpgradeable.sol b/contracts/openzeppelin/5.2/upgradeable/utils/ContextUpgradeable.sol similarity index 93% rename from contracts/openzeppelin/5.0.2/upgradeable/utils/ContextUpgradeable.sol rename to contracts/openzeppelin/5.2/upgradeable/utils/ContextUpgradeable.sol index 6390d7def..5aa9b48bb 100644 --- a/contracts/openzeppelin/5.0.2/upgradeable/utils/ContextUpgradeable.sol +++ b/contracts/openzeppelin/5.2/upgradeable/utils/ContextUpgradeable.sol @@ -15,10 +15,11 @@ import {Initializable} from "../proxy/utils/Initializable.sol"; * This contract is only required for intermediate, library-like contracts. */ abstract contract ContextUpgradeable is Initializable { - function __Context_init() internal onlyInitializing {} - - function __Context_init_unchained() internal onlyInitializing {} + function __Context_init() internal onlyInitializing { + } + function __Context_init_unchained() internal onlyInitializing { + } function _msgSender() internal view virtual returns (address) { return msg.sender; } diff --git a/contracts/openzeppelin/5.0.2/upgradeable/utils/introspection/ERC165Upgradeable.sol b/contracts/openzeppelin/5.2/upgradeable/utils/introspection/ERC165Upgradeable.sol similarity index 71% rename from contracts/openzeppelin/5.0.2/upgradeable/utils/introspection/ERC165Upgradeable.sol rename to contracts/openzeppelin/5.2/upgradeable/utils/introspection/ERC165Upgradeable.sol index 883a5d1a8..84f2c4a17 100644 --- a/contracts/openzeppelin/5.0.2/upgradeable/utils/introspection/ERC165Upgradeable.sol +++ b/contracts/openzeppelin/5.2/upgradeable/utils/introspection/ERC165Upgradeable.sol @@ -1,15 +1,15 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts (last updated v5.0.0) (utils/introspection/ERC165.sol) +// OpenZeppelin Contracts (last updated v5.1.0) (utils/introspection/ERC165.sol) pragma solidity ^0.8.20; -import {IERC165} from "@openzeppelin/contracts-v5.0.2/utils/introspection/IERC165.sol"; +import {IERC165} from "@openzeppelin/contracts-v5.2/utils/introspection/IERC165.sol"; import {Initializable} from "../../proxy/utils/Initializable.sol"; /** * @dev Implementation of the {IERC165} interface. * - * Contracts that want to implement ERC165 should inherit from this contract and override {supportsInterface} to check + * Contracts that want to implement ERC-165 should inherit from this contract and override {supportsInterface} to check * for the additional interface id that will be supported. For example: * * ```solidity @@ -19,10 +19,11 @@ import {Initializable} from "../../proxy/utils/Initializable.sol"; * ``` */ abstract contract ERC165Upgradeable is Initializable, IERC165 { - function __ERC165_init() internal onlyInitializing {} - - function __ERC165_init_unchained() internal onlyInitializing {} + function __ERC165_init() internal onlyInitializing { + } + function __ERC165_init_unchained() internal onlyInitializing { + } /** * @dev See {IERC165-supportsInterface}. */ diff --git a/package.json b/package.json index 920e3d687..6e73fe112 100644 --- a/package.json +++ b/package.json @@ -110,8 +110,7 @@ "@aragon/os": "4.4.0", "@openzeppelin/contracts": "3.4.0", "@openzeppelin/contracts-v4.4": "npm:@openzeppelin/contracts@4.4.1", - "@openzeppelin/contracts-v5.0.2": "npm:@openzeppelin/contracts@5.0.2", - "@openzeppelin/contracts-v5.2.0": "npm:@openzeppelin/contracts@5.2.0", + "@openzeppelin/contracts-v5.2": "npm:@openzeppelin/contracts@5.2.0", "openzeppelin-solidity": "2.0.0" } } diff --git a/test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol b/test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol index de910fe8e..7dd22ef7e 100644 --- a/test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol +++ b/test/0.8.25/vaults/contracts/StakingVault__HarnessForTestUpgrade.sol @@ -3,9 +3,9 @@ pragma solidity 0.8.25; -import {OwnableUpgradeable} from "contracts/openzeppelin/5.0.2/upgradeable/access/OwnableUpgradeable.sol"; -import {SafeCast} from "@openzeppelin/contracts-v5.0.2/utils/math/SafeCast.sol"; -import {IERC20} from "@openzeppelin/contracts-v5.0.2/token/ERC20/IERC20.sol"; +import {OwnableUpgradeable} from "contracts/openzeppelin/5.2/upgradeable/access/OwnableUpgradeable.sol"; +import {SafeCast} from "@openzeppelin/contracts-v5.2/utils/math/SafeCast.sol"; +import {IERC20} from "@openzeppelin/contracts-v5.2/token/ERC20/IERC20.sol"; import {VaultHub} from "contracts/0.8.25/vaults/VaultHub.sol"; import {IStakingVault} from "contracts/0.8.25/vaults/interfaces/IStakingVault.sol"; import {BeaconChainDepositLogistics} from "contracts/0.8.25/vaults/BeaconChainDepositLogistics.sol"; diff --git a/test/0.8.25/vaults/contracts/StakingVault__MockForVaultDelegationLayer.sol b/test/0.8.25/vaults/contracts/StakingVault__MockForVaultDelegationLayer.sol index 50fe9a7b0..550a46567 100644 --- a/test/0.8.25/vaults/contracts/StakingVault__MockForVaultDelegationLayer.sol +++ b/test/0.8.25/vaults/contracts/StakingVault__MockForVaultDelegationLayer.sol @@ -4,7 +4,7 @@ pragma solidity 0.8.25; import {IStakingVault} from "contracts/0.8.25/vaults/interfaces/IStakingVault.sol"; -import {OwnableUpgradeable} from "contracts/openzeppelin/5.0.2/upgradeable/access/OwnableUpgradeable.sol"; +import {OwnableUpgradeable} from "contracts/openzeppelin/5.2/upgradeable/access/OwnableUpgradeable.sol"; contract StakingVault__MockForVaultDelegationLayer is OwnableUpgradeable { address public constant vaultHub = address(0xABCD); diff --git a/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol b/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol index f5780b015..311034508 100644 --- a/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol +++ b/test/0.8.25/vaults/dashboard/contracts/VaultFactory__MockForDashboard.sol @@ -3,9 +3,9 @@ pragma solidity 0.8.25; -import {UpgradeableBeacon} from "@openzeppelin/contracts-v5.2.0/proxy/beacon/UpgradeableBeacon.sol"; -import {BeaconProxy} from "@openzeppelin/contracts-v5.2.0/proxy/beacon/BeaconProxy.sol"; -import {Clones} from "@openzeppelin/contracts-v5.2.0/proxy/Clones.sol"; +import {UpgradeableBeacon} from "@openzeppelin/contracts-v5.2/proxy/beacon/UpgradeableBeacon.sol"; +import {BeaconProxy} from "@openzeppelin/contracts-v5.2/proxy/beacon/BeaconProxy.sol"; +import {Clones} from "@openzeppelin/contracts-v5.2/proxy/Clones.sol"; import {IStakingVault} from "contracts/0.8.25/vaults/interfaces/IStakingVault.sol"; import {Dashboard} from "contracts/0.8.25/vaults/Dashboard.sol"; diff --git a/test/0.8.25/vaults/delegation/contracts/StETH__MockForDelegation.sol b/test/0.8.25/vaults/delegation/contracts/StETH__MockForDelegation.sol index 5a9da4183..2b3f84e6c 100644 --- a/test/0.8.25/vaults/delegation/contracts/StETH__MockForDelegation.sol +++ b/test/0.8.25/vaults/delegation/contracts/StETH__MockForDelegation.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.25; -import {ERC20} from "@openzeppelin/contracts-v5.0.2/token/ERC20/ERC20.sol"; +import {ERC20} from "@openzeppelin/contracts-v5.2/token/ERC20/ERC20.sol"; contract StETH__MockForDelegation is ERC20 { constructor() ERC20("Staked Ether", "stETH") {} diff --git a/test/0.8.25/vaults/staking-vault/contracts/VaultFactory__MockForStakingVault.sol b/test/0.8.25/vaults/staking-vault/contracts/VaultFactory__MockForStakingVault.sol index 287ea3e4d..f843c98c9 100644 --- a/test/0.8.25/vaults/staking-vault/contracts/VaultFactory__MockForStakingVault.sol +++ b/test/0.8.25/vaults/staking-vault/contracts/VaultFactory__MockForStakingVault.sol @@ -3,8 +3,8 @@ pragma solidity ^0.8.0; -import {UpgradeableBeacon} from "@openzeppelin/contracts-v5.2.0/proxy/beacon/UpgradeableBeacon.sol"; -import {BeaconProxy} from "@openzeppelin/contracts-v5.2.0/proxy/beacon/BeaconProxy.sol"; +import {UpgradeableBeacon} from "@openzeppelin/contracts-v5.2/proxy/beacon/UpgradeableBeacon.sol"; +import {BeaconProxy} from "@openzeppelin/contracts-v5.2/proxy/beacon/BeaconProxy.sol"; import {IStakingVault} from "contracts/0.8.25/vaults/interfaces/IStakingVault.sol"; contract VaultFactory__MockForStakingVault is UpgradeableBeacon { diff --git a/yarn.lock b/yarn.lock index 164c68abb..65443ee15 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1610,14 +1610,7 @@ __metadata: languageName: node linkType: hard -"@openzeppelin/contracts-v5.0.2@npm:@openzeppelin/contracts@5.0.2": - version: 5.0.2 - resolution: "@openzeppelin/contracts@npm:5.0.2" - checksum: 10c0/d042661db7bb2f3a4b9ef30bba332e86ac20907d171f2ebfccdc9255cc69b62786fead8d6904b8148a8f26946bc7c15eead91b95f75db0c193a99d52e528663e - languageName: node - linkType: hard - -"@openzeppelin/contracts-v5.2.0@npm:@openzeppelin/contracts@5.2.0": +"@openzeppelin/contracts-v5.2@npm:@openzeppelin/contracts@5.2.0": version: 5.2.0 resolution: "@openzeppelin/contracts@npm:5.2.0" checksum: 10c0/6e2d8c6daaeb8e111d49a82c30997a6c5d4e512338b55500db7fd4340f29c1cbf35f9dcfa0dbc672e417bc84e99f5441a105cb585cd4680ad70cbcf9a24094fc @@ -8070,8 +8063,7 @@ __metadata: "@nomicfoundation/ignition-core": "npm:0.15.8" "@openzeppelin/contracts": "npm:3.4.0" "@openzeppelin/contracts-v4.4": "npm:@openzeppelin/contracts@4.4.1" - "@openzeppelin/contracts-v5.0.2": "npm:@openzeppelin/contracts@5.0.2" - "@openzeppelin/contracts-v5.2.0": "npm:@openzeppelin/contracts@5.2.0" + "@openzeppelin/contracts-v5.2": "npm:@openzeppelin/contracts@5.2.0" "@typechain/ethers-v6": "npm:0.5.1" "@typechain/hardhat": "npm:9.1.0" "@types/chai": "npm:4.3.20" From 26c0a2f3cbd0404efc8377c6614f1c50e708ec5f Mon Sep 17 00:00:00 2001 From: Logachev Nikita Date: Wed, 22 Jan 2025 00:51:04 +0300 Subject: [PATCH 15/15] fix: review fixes --- .env.example | 4 ++ lib/proxy.ts | 31 +++------- test/0.8.25/vaults/vaultFactory.test.ts | 60 ++++++++++--------- .../vaults-happy-path.integration.ts | 12 ++-- 4 files changed, 52 insertions(+), 55 deletions(-) diff --git a/.env.example b/.env.example index 5dcda8f6b..6d126f4e1 100644 --- a/.env.example +++ b/.env.example @@ -21,6 +21,8 @@ LOCAL_STAKING_ROUTER_ADDRESS= LOCAL_VALIDATORS_EXIT_BUS_ORACLE_ADDRESS= LOCAL_WITHDRAWAL_QUEUE_ADDRESS= LOCAL_WITHDRAWAL_VAULT_ADDRESS= +LOCAL_STAKING_VAULT_FACTORY_ADDRESS= +LOCAL_STAKING_VAULT_BEACON_ADDRESS= # RPC URL for a separate, non Hardhat Network node (Anvil, Infura, Alchemy, etc.) MAINNET_RPC_URL=http://localhost:8545 @@ -46,6 +48,8 @@ MAINNET_STAKING_ROUTER_ADDRESS= MAINNET_VALIDATORS_EXIT_BUS_ORACLE_ADDRESS= MAINNET_WITHDRAWAL_QUEUE_ADDRESS= MAINNET_WITHDRAWAL_VAULT_ADDRESS= +MAINNET_STAKING_VAULT_FACTORY_ADDRESS= +MAINNET_STAKING_VAULT_BEACON_ADDRESS= HOLESKY_RPC_URL= SEPOLIA_RPC_URL= diff --git a/lib/proxy.ts b/lib/proxy.ts index 52a77123e..fafffca39 100644 --- a/lib/proxy.ts +++ b/lib/proxy.ts @@ -15,7 +15,6 @@ import { import { findEventsWithInterfaces } from "lib"; import { IDelegation } from "../typechain-types/contracts/0.8.25/vaults/VaultFactory.sol/VaultFactory"; -import DelegationInitializationParamsStruct = IDelegation.InitialStateStruct; interface ProxifyArgs { impl: T; @@ -48,26 +47,14 @@ interface CreateVaultResponse { } export async function createVaultProxy( + caller: HardhatEthersSigner, vaultFactory: VaultFactory, - _admin: HardhatEthersSigner, - _owner: HardhatEthersSigner, - _operator: HardhatEthersSigner, - initializationParams: Partial = {}, + delegationParams: IDelegation.InitialStateStruct, + stakingVaultInitializerExtraParams: BytesLike = "0x", ): Promise { - // Define the parameters for the struct - const defaultParams: DelegationInitializationParamsStruct = { - defaultAdmin: await _admin.getAddress(), - curator: await _owner.getAddress(), - funderWithdrawer: await _owner.getAddress(), - minterBurner: await _owner.getAddress(), - nodeOperatorManager: await _operator.getAddress(), - nodeOperatorFeeClaimer: await _owner.getAddress(), - curatorFeeBP: 100n, - nodeOperatorFeeBP: 200n, - }; - const params = { ...defaultParams, ...initializationParams }; - - const tx = await vaultFactory.connect(_owner).createVaultWithDelegation(params, "0x"); + const tx = await vaultFactory + .connect(caller) + .createVaultWithDelegation(delegationParams, stakingVaultInitializerExtraParams); // Get the receipt manually const receipt = (await tx.wait())!; @@ -84,9 +71,9 @@ export async function createVaultProxy( const { delegation: delegationAddress } = delegationEvents[0].args; - const proxy = (await ethers.getContractAt("BeaconProxy", vault, _owner)) as BeaconProxy; - const stakingVault = (await ethers.getContractAt("StakingVault", vault, _owner)) as StakingVault; - const delegation = (await ethers.getContractAt("Delegation", delegationAddress, _owner)) as Delegation; + const proxy = (await ethers.getContractAt("BeaconProxy", vault, caller)) as BeaconProxy; + const stakingVault = (await ethers.getContractAt("StakingVault", vault, caller)) as StakingVault; + const delegation = (await ethers.getContractAt("Delegation", delegationAddress, caller)) as Delegation; return { tx, diff --git a/test/0.8.25/vaults/vaultFactory.test.ts b/test/0.8.25/vaults/vaultFactory.test.ts index 765946c65..7d187d28f 100644 --- a/test/0.8.25/vaults/vaultFactory.test.ts +++ b/test/0.8.25/vaults/vaultFactory.test.ts @@ -25,6 +25,8 @@ import { createVaultProxy, ether } from "lib"; import { deployLidoLocator } from "test/deploy"; import { Snapshot } from "test/suite"; +import { IDelegation } from "../../../typechain-types/contracts/0.8.25/vaults/VaultFactory.sol/VaultFactory"; + describe("VaultFactory.sol", () => { let deployer: HardhatEthersSigner; let admin: HardhatEthersSigner; @@ -55,6 +57,8 @@ describe("VaultFactory.sol", () => { let originalState: string; + let delegationParams: IDelegation.InitialStateStruct; + before(async () => { [deployer, admin, holder, operator, stranger, vaultOwner1, vaultOwner2] = await ethers.getSigners(); @@ -98,14 +102,23 @@ describe("VaultFactory.sol", () => { implOld, "InvalidInitialization", ); + + delegationParams = { + defaultAdmin: await admin.getAddress(), + curator: await vaultOwner1.getAddress(), + minterBurner: await vaultOwner1.getAddress(), + funderWithdrawer: await vaultOwner1.getAddress(), + nodeOperatorManager: await operator.getAddress(), + nodeOperatorFeeClaimer: await vaultOwner1.getAddress(), + curatorFeeBP: 100n, + nodeOperatorFeeBP: 200n, + }; }); beforeEach(async () => (originalState = await Snapshot.take())); afterEach(async () => await Snapshot.restore(originalState)); - context("beacon.constructor", () => {}); - context("constructor", () => { it("reverts if `_owner` is zero address", async () => { await expect(ethers.deployContract("UpgradeableBeacon", [ZeroAddress, admin], { from: deployer })) @@ -131,12 +144,6 @@ describe("VaultFactory.sol", () => { }); it("works and emit `OwnershipTransferred`, `Upgraded` events", async () => { - // const beacon = await ethers.deployContract( - // "VaultFactory", - // [await implOld.getAddress(), await steth.getAddress()], - // { from: deployer }, - // ); - const tx = beacon.deploymentTransaction(); await expect(tx) @@ -150,17 +157,21 @@ describe("VaultFactory.sol", () => { context("createVaultWithDelegation", () => { it("reverts if `curator` is zero address", async () => { - await expect( - createVaultProxy(vaultFactory, admin, vaultOwner1, operator, { - curator: ZeroAddress, - }), - ) + const params = { ...delegationParams, curator: ZeroAddress }; + await expect(createVaultProxy(vaultOwner1, vaultFactory, params)) .to.revertedWithCustomError(vaultFactory, "ZeroArgument") .withArgs("curator"); }); it("works with empty `params`", async () => { - const { tx, vault, delegation: delegation_ } = await createVaultProxy(vaultFactory, admin, vaultOwner1, operator); + console.log({ + delegationParams, + }); + const { + tx, + vault, + delegation: delegation_, + } = await createVaultProxy(vaultOwner1, vaultFactory, delegationParams); await expect(tx) .to.emit(vaultFactory, "VaultCreated") @@ -174,15 +185,12 @@ describe("VaultFactory.sol", () => { }); it("check `version()`", async () => { - const { vault } = await createVaultProxy(vaultFactory, admin, vaultOwner1, operator); + const { vault } = await createVaultProxy(vaultOwner1, vaultFactory, delegationParams); expect(await vault.version()).to.eq(1); }); - - it.skip("works with non-empty `params`", async () => {}); }); context("connect", () => { - it("create vault ", async () => {}); it("connect ", async () => { const vaultsBefore = await accounting.vaultsCount(); expect(vaultsBefore).to.eq(0); @@ -202,16 +210,14 @@ describe("VaultFactory.sol", () => { //create vaults const { vault: vault1, delegation: delegator1 } = await createVaultProxy( - vaultFactory, - admin, vaultOwner1, - operator, + vaultFactory, + delegationParams, ); const { vault: vault2, delegation: delegator2 } = await createVaultProxy( - vaultFactory, - admin, vaultOwner2, - operator, + vaultFactory, + delegationParams, ); //owner of vault is delegator @@ -263,7 +269,7 @@ describe("VaultFactory.sol", () => { expect(implAfter).to.eq(await implNew.getAddress()); //create new vault with new implementation - const { vault: vault3 } = await createVaultProxy(vaultFactory, admin, vaultOwner1, operator); + const { vault: vault3 } = await createVaultProxy(vaultOwner1, vaultFactory, delegationParams); //we upgrade implementation - we do not check implementation, just proxy bytecode await expect( @@ -317,7 +323,7 @@ describe("VaultFactory.sol", () => { context("After upgrade", () => { it("exists vaults - init not works, finalize works ", async () => { - const { vault: vault1 } = await createVaultProxy(vaultFactory, admin, vaultOwner1, operator); + const { vault: vault1 } = await createVaultProxy(vaultOwner1, vaultFactory, delegationParams); await beacon.connect(admin).upgradeTo(implNew); @@ -333,7 +339,7 @@ describe("VaultFactory.sol", () => { it("new vaults - init works, finalize not works ", async () => { await beacon.connect(admin).upgradeTo(implNew); - const { vault: vault2 } = await createVaultProxy(vaultFactory, admin, vaultOwner2, operator); + const { vault: vault2 } = await createVaultProxy(vaultOwner1, vaultFactory, delegationParams); const vault2WithNewImpl = await ethers.getContractAt("StakingVault__HarnessForTestUpgrade", vault2, deployer); diff --git a/test/integration/vaults-happy-path.integration.ts b/test/integration/vaults-happy-path.integration.ts index 7b5ba53e5..b94ac091f 100644 --- a/test/integration/vaults-happy-path.integration.ts +++ b/test/integration/vaults-happy-path.integration.ts @@ -142,14 +142,14 @@ describe("Scenario: Staking Vaults Happy Path", () => { const { stakingVaultFactory, stakingVaultBeacon } = ctx.contracts; const implAddress = await stakingVaultBeacon.implementation(); - const adminContractImplAddress = await stakingVaultFactory.DELEGATION_IMPL(); + const delegationAddress = await stakingVaultFactory.DELEGATION_IMPL(); - const vaultImpl = await ethers.getContractAt("StakingVault", implAddress); - const vaultFactoryAdminContract = await ethers.getContractAt("Delegation", adminContractImplAddress); + const _stakingVault = await ethers.getContractAt("StakingVault", implAddress); + const _delegation = await ethers.getContractAt("Delegation", delegationAddress); - expect(await vaultImpl.vaultHub()).to.equal(ctx.contracts.accounting.address); - expect(await vaultImpl.DEPOSIT_CONTRACT()).to.equal(depositContract); - expect(await vaultFactoryAdminContract.STETH()).to.equal(ctx.contracts.lido.address); + expect(await _stakingVault.vaultHub()).to.equal(ctx.contracts.accounting.address); + expect(await _stakingVault.DEPOSIT_CONTRACT()).to.equal(depositContract); + expect(await _delegation.STETH()).to.equal(ctx.contracts.lido.address); // TODO: check what else should be validated here });