From 41728a7a0d4ca925292827e3c74257718079ed76 Mon Sep 17 00:00:00 2001 From: Yuri Tkachenko Date: Wed, 29 Jan 2025 08:02:27 +0000 Subject: [PATCH] chore: renaming --- contracts/0.8.25/vaults/StakingVault.sol | 6 +- ...sol => StakingVaultBeaconChainManager.sol} | 10 +- ...ccounting.test.ts => stakingVault.test.ts} | 122 ++++++++++- ...=> stakingVaultBeaconChainManager.test.ts} | 207 ++++++++---------- 4 files changed, 218 insertions(+), 127 deletions(-) rename contracts/0.8.25/vaults/{VaultValidatorsManager.sol => StakingVaultBeaconChainManager.sol} (97%) rename test/0.8.25/vaults/staking-vault/{staking-vault.accounting.test.ts => stakingVault.test.ts} (80%) rename test/0.8.25/vaults/staking-vault/{staking-vault.validators.test.ts => stakingVaultBeaconChainManager.test.ts} (51%) diff --git a/contracts/0.8.25/vaults/StakingVault.sol b/contracts/0.8.25/vaults/StakingVault.sol index af7ccd36b..96d33a3d2 100644 --- a/contracts/0.8.25/vaults/StakingVault.sol +++ b/contracts/0.8.25/vaults/StakingVault.sol @@ -7,7 +7,7 @@ pragma solidity 0.8.25; import {OwnableUpgradeable} from "contracts/openzeppelin/5.2/upgradeable/access/OwnableUpgradeable.sol"; import {VaultHub} from "./VaultHub.sol"; -import {VaultValidatorsManager} from "./VaultValidatorsManager.sol"; +import {StakingVaultBeaconChainManager} from "./StakingVaultBeaconChainManager.sol"; import {IStakingVault} from "./interfaces/IStakingVault.sol"; @@ -56,7 +56,7 @@ import {IStakingVault} from "./interfaces/IStakingVault.sol"; * deposit contract. * */ -contract StakingVault is IStakingVault, VaultValidatorsManager, OwnableUpgradeable { +contract StakingVault is IStakingVault, StakingVaultBeaconChainManager, OwnableUpgradeable { /** * @notice ERC-7201 storage namespace for the vault * @dev ERC-7201 namespace is used to prevent upgrade collisions @@ -110,7 +110,7 @@ contract StakingVault is IStakingVault, VaultValidatorsManager, OwnableUpgradeab constructor( address _vaultHub, address _beaconChainDepositContract - ) VaultValidatorsManager(_beaconChainDepositContract) { + ) StakingVaultBeaconChainManager(_beaconChainDepositContract) { if (_vaultHub == address(0)) revert ZeroArgument("_vaultHub"); VAULT_HUB = VaultHub(_vaultHub); diff --git a/contracts/0.8.25/vaults/VaultValidatorsManager.sol b/contracts/0.8.25/vaults/StakingVaultBeaconChainManager.sol similarity index 97% rename from contracts/0.8.25/vaults/VaultValidatorsManager.sol rename to contracts/0.8.25/vaults/StakingVaultBeaconChainManager.sol index 901c638f5..24ec3b7d9 100644 --- a/contracts/0.8.25/vaults/VaultValidatorsManager.sol +++ b/contracts/0.8.25/vaults/StakingVaultBeaconChainManager.sol @@ -10,7 +10,7 @@ import {IDepositContract} from "../interfaces/IDepositContract.sol"; import {IStakingVault} from "./interfaces/IStakingVault.sol"; /// @notice Abstract contract that manages validator deposits and exits for staking vaults -abstract contract VaultValidatorsManager { +abstract contract StakingVaultBeaconChainManager { /// @notice The Beacon Chain deposit contract used for staking validators IDepositContract private immutable BEACON_CHAIN_DEPOSIT_CONTRACT; @@ -184,18 +184,18 @@ abstract contract VaultValidatorsManager { * @notice Emitted when a validator exit request is made * @dev Signals `nodeOperator` to exit the validator * @param _sender Address that requested the validator exit - * @param _pubkey Public key of the validator requested to exit + * @param _pubkeys Public key of the validator requested to exit */ - event ValidatorsExitRequested(address indexed _sender, bytes _pubkey); + event ValidatorsExitRequested(address indexed _sender, bytes _pubkeys); /** * @notice Emitted when a validator partial exit request is made * @dev Signals `nodeOperator` to exit the validator * @param _sender Address that requested the validator partial exit - * @param _pubkey Public key of the validator requested to exit + * @param _pubkeys Public key of the validator requested to exit * @param _amounts Amounts of ether requested to exit */ - event ValidatorsPartialExitRequested(address indexed _sender, bytes _pubkey, uint64[] _amounts); + event ValidatorsPartialExitRequested(address indexed _sender, bytes _pubkeys, uint64[] _amounts); /** * @notice Emitted when an excess fee is refunded back to the sender diff --git a/test/0.8.25/vaults/staking-vault/staking-vault.accounting.test.ts b/test/0.8.25/vaults/staking-vault/stakingVault.test.ts similarity index 80% rename from test/0.8.25/vaults/staking-vault/staking-vault.accounting.test.ts rename to test/0.8.25/vaults/staking-vault/stakingVault.test.ts index 0562a5f42..3731ba155 100644 --- a/test/0.8.25/vaults/staking-vault/staking-vault.accounting.test.ts +++ b/test/0.8.25/vaults/staking-vault/stakingVault.test.ts @@ -12,7 +12,7 @@ import { VaultHub__MockForStakingVault, } from "typechain-types"; -import { de0x, ether, impersonate } from "lib"; +import { computeDepositDataRoot, de0x, ether, impersonate, streccak } from "lib"; import { deployStakingVaultBehindBeaconProxy } from "test/deploy"; import { Snapshot } from "test/suite"; @@ -21,7 +21,7 @@ const MAX_INT128 = 2n ** 127n - 1n; const MAX_UINT128 = 2n ** 128n - 1n; // @TODO: test reentrancy attacks -describe("StakingVault.sol:Accounting", () => { +describe("StakingVault.sol", () => { let vaultOwner: HardhatEthersSigner; let operator: HardhatEthersSigner; let stranger: HardhatEthersSigner; @@ -57,13 +57,9 @@ describe("StakingVault.sol:Accounting", () => { vaultHubSigner = await impersonate(vaultHubAddress, ether("10")); }); - beforeEach(async () => { - originalState = await Snapshot.take(); - }); + beforeEach(async () => (originalState = await Snapshot.take())); - afterEach(async () => { - await Snapshot.restore(originalState); - }); + afterEach(async () => await Snapshot.restore(originalState)); context("constructor", () => { it("sets the vault hub address in the implementation", async () => { @@ -82,7 +78,7 @@ describe("StakingVault.sol:Accounting", () => { it("reverts on construction if the deposit contract address is zero", async () => { await expect(ethers.deployContract("StakingVault", [vaultHubAddress, ZeroAddress])) - .to.be.revertedWithCustomError(stakingVaultImplementation, "ZeroArgument") + .to.be.revertedWithCustomError(stakingVaultImplementation, "ZeroBeaconChainDepositContract") .withArgs("_beaconChainDepositContract"); }); @@ -118,6 +114,114 @@ describe("StakingVault.sol:Accounting", () => { }); }); + context("pauseBeaconChainDeposits", () => { + it("reverts if called by a non-owner", async () => { + await expect(stakingVault.connect(stranger).pauseBeaconChainDeposits()) + .to.be.revertedWithCustomError(stakingVault, "OwnableUnauthorizedAccount") + .withArgs(await stranger.getAddress()); + }); + + it("reverts if the beacon deposits are already paused", async () => { + await stakingVault.connect(vaultOwner).pauseBeaconChainDeposits(); + + await expect(stakingVault.connect(vaultOwner).pauseBeaconChainDeposits()).to.be.revertedWithCustomError( + stakingVault, + "BeaconChainDepositsResumeExpected", + ); + }); + + it("allows to pause deposits", async () => { + await expect(stakingVault.connect(vaultOwner).pauseBeaconChainDeposits()).to.emit( + stakingVault, + "BeaconChainDepositsPaused", + ); + expect(await stakingVault.beaconChainDepositsPaused()).to.be.true; + }); + }); + + context("resumeBeaconChainDeposits", () => { + it("reverts if called by a non-owner", async () => { + await expect(stakingVault.connect(stranger).resumeBeaconChainDeposits()) + .to.be.revertedWithCustomError(stakingVault, "OwnableUnauthorizedAccount") + .withArgs(await stranger.getAddress()); + }); + + it("reverts if the beacon deposits are already resumed", async () => { + await expect(stakingVault.connect(vaultOwner).resumeBeaconChainDeposits()).to.be.revertedWithCustomError( + stakingVault, + "BeaconChainDepositsPauseExpected", + ); + }); + + it("allows to resume deposits", async () => { + await stakingVault.connect(vaultOwner).pauseBeaconChainDeposits(); + + await expect(stakingVault.connect(vaultOwner).resumeBeaconChainDeposits()).to.emit( + stakingVault, + "BeaconChainDepositsResumed", + ); + expect(await stakingVault.beaconChainDepositsPaused()).to.be.false; + }); + }); + + context("depositToBeaconChain", () => { + it("reverts if called by a non-operator", async () => { + await expect( + stakingVault + .connect(stranger) + .depositToBeaconChain([ + { pubkey: "0x", signature: "0x", amount: 0, depositDataRoot: streccak("random-root") }, + ]), + ) + .to.be.revertedWithCustomError(stakingVault, "NotAuthorized") + .withArgs("depositToBeaconChain", stranger); + }); + + it("reverts if the number of deposits is zero", async () => { + await expect(stakingVault.depositToBeaconChain([])) + .to.be.revertedWithCustomError(stakingVault, "ZeroArgument") + .withArgs("_deposits"); + }); + + it("reverts if the vault is not balanced", async () => { + await stakingVault.connect(vaultHubSigner).lock(ether("1")); + await expect( + stakingVault + .connect(operator) + .depositToBeaconChain([ + { pubkey: "0x", signature: "0x", amount: 0, depositDataRoot: streccak("random-root") }, + ]), + ).to.be.revertedWithCustomError(stakingVault, "Unbalanced"); + }); + + it("reverts if the deposits are paused", async () => { + await stakingVault.connect(vaultOwner).pauseBeaconChainDeposits(); + await expect( + stakingVault + .connect(operator) + .depositToBeaconChain([ + { pubkey: "0x", signature: "0x", amount: 0, depositDataRoot: streccak("random-root") }, + ]), + ).to.be.revertedWithCustomError(stakingVault, "BeaconChainDepositsArePaused"); + }); + + it("makes deposits to the beacon chain and emits the DepositedToBeaconChain event", async () => { + await stakingVault.fund({ value: ether("32") }); + + const pubkey = "0x" + "ab".repeat(48); + const signature = "0x" + "ef".repeat(96); + const amount = ether("32"); + const withdrawalCredentials = await stakingVault.withdrawalCredentials(); + const depositDataRoot = computeDepositDataRoot(withdrawalCredentials, pubkey, signature, amount); + + await expect( + stakingVault.connect(operator).depositToBeaconChain([{ pubkey, signature, amount, depositDataRoot }]), + ) + .to.emit(stakingVault, "DepositedToBeaconChain") + .withArgs(operator, 1, amount); + }); + }); + context("unlocked", () => { it("returns the correct unlocked balance", async () => { expect(await stakingVault.unlocked()).to.equal(0n); diff --git a/test/0.8.25/vaults/staking-vault/staking-vault.validators.test.ts b/test/0.8.25/vaults/staking-vault/stakingVaultBeaconChainManager.test.ts similarity index 51% rename from test/0.8.25/vaults/staking-vault/staking-vault.validators.test.ts rename to test/0.8.25/vaults/staking-vault/stakingVaultBeaconChainManager.test.ts index 6633ce129..9b535d91a 100644 --- a/test/0.8.25/vaults/staking-vault/staking-vault.validators.test.ts +++ b/test/0.8.25/vaults/staking-vault/stakingVaultBeaconChainManager.test.ts @@ -1,18 +1,27 @@ import { expect } from "chai"; +import { ZeroAddress } from "ethers"; import { ethers } from "hardhat"; import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers"; -import { StakingVault, VaultHub__MockForStakingVault } from "typechain-types"; +import { + DepositContract__MockForStakingVault, + EIP7002WithdrawalRequest_Mock, + StakingVault, + VaultHub__MockForStakingVault, +} from "typechain-types"; -import { computeDepositDataRoot, ether, impersonate, streccak } from "lib"; +import { computeDepositDataRoot, de0x, ether, impersonate } from "lib"; import { deployStakingVaultBehindBeaconProxy } from "test/deploy"; -import { Snapshot } from "test/suite"; +import { EIP7002_PREDEPLOYED_ADDRESS, Snapshot } from "test/suite"; -const getValidatorPubkey = (index: number) => "0x" + "ab".repeat(48 * index); +const getPubkey = (index: number) => index.toString(16).padStart(4, "0").toLocaleLowerCase().repeat(24); +const getSignature = (index: number) => index.toString(16).padStart(8, "0").toLocaleLowerCase().repeat(12); -describe("StakingVault.sol:ValidatorsManagement", () => { +const getPubkeys = (num: number) => `0x${Array.from({ length: num }, (_, i) => getPubkey(i + 1)).join("")}`; + +describe("StakingVaultBeaconChainManager.sol", () => { let vaultOwner: HardhatEthersSigner; let operator: HardhatEthersSigner; let stranger: HardhatEthersSigner; @@ -20,154 +29,132 @@ describe("StakingVault.sol:ValidatorsManagement", () => { let stakingVault: StakingVault; let vaultHub: VaultHub__MockForStakingVault; + let depositContract: DepositContract__MockForStakingVault; + let withdrawalRequest: EIP7002WithdrawalRequest_Mock; let vaultOwnerAddress: string; let vaultHubAddress: string; let operatorAddress: string; + let depositContractAddress: string; + let stakingVaultAddress: string; + let originalState: string; before(async () => { [vaultOwner, operator, stranger] = await ethers.getSigners(); - ({ stakingVault, vaultHub } = await deployStakingVaultBehindBeaconProxy(vaultOwner, operator)); + ({ stakingVault, vaultHub, depositContract } = await deployStakingVaultBehindBeaconProxy(vaultOwner, operator)); vaultOwnerAddress = await vaultOwner.getAddress(); vaultHubAddress = await vaultHub.getAddress(); operatorAddress = await operator.getAddress(); + depositContractAddress = await depositContract.getAddress(); + stakingVaultAddress = await stakingVault.getAddress(); - vaultHubSigner = await impersonate(vaultHubAddress, ether("10")); - }); - - beforeEach(async () => { - originalState = await Snapshot.take(); - }); + withdrawalRequest = await ethers.getContractAt("EIP7002WithdrawalRequest_Mock", EIP7002_PREDEPLOYED_ADDRESS); - afterEach(async () => { - await Snapshot.restore(originalState); + vaultHubSigner = await impersonate(vaultHubAddress, ether("10")); }); - context("pauseBeaconChainDeposits", () => { - it("reverts if called by a non-owner", async () => { - await expect(stakingVault.connect(stranger).pauseBeaconChainDeposits()) - .to.be.revertedWithCustomError(stakingVault, "OwnableUnauthorizedAccount") - .withArgs(await stranger.getAddress()); - }); + beforeEach(async () => (originalState = await Snapshot.take())); - it("reverts if the beacon deposits are already paused", async () => { - await stakingVault.connect(vaultOwner).pauseBeaconChainDeposits(); + afterEach(async () => await Snapshot.restore(originalState)); - await expect(stakingVault.connect(vaultOwner).pauseBeaconChainDeposits()).to.be.revertedWithCustomError( + context("constructor", () => { + it("reverts if the deposit contract address is zero", async () => { + await expect(ethers.deployContract("StakingVault", [vaultHubAddress, ZeroAddress])).to.be.revertedWithCustomError( stakingVault, - "BeaconChainDepositsResumeExpected", + "ZeroBeaconChainDepositContract", ); }); - - it("allows to pause deposits", async () => { - await expect(stakingVault.connect(vaultOwner).pauseBeaconChainDeposits()).to.emit( - stakingVault, - "BeaconChainDepositsPaused", - ); - expect(await stakingVault.beaconChainDepositsPaused()).to.be.true; - }); }); - context("resumeBeaconChainDeposits", () => { - it("reverts if called by a non-owner", async () => { - await expect(stakingVault.connect(stranger).resumeBeaconChainDeposits()) - .to.be.revertedWithCustomError(stakingVault, "OwnableUnauthorizedAccount") - .withArgs(await stranger.getAddress()); + context("_getDepositContract", () => { + it("returns the deposit contract address", async () => { + expect(await stakingVault.depositContract()).to.equal(depositContractAddress); }); + }); - it("reverts if the beacon deposits are already resumed", async () => { - await expect(stakingVault.connect(vaultOwner).resumeBeaconChainDeposits()).to.be.revertedWithCustomError( - stakingVault, - "BeaconChainDepositsPauseExpected", + context("_withdrawalCredentials", () => { + it("returns the withdrawal credentials", async () => { + expect(await stakingVault.withdrawalCredentials()).to.equal( + ("0x01" + "00".repeat(11) + de0x(stakingVaultAddress)).toLowerCase(), ); }); + }); - it("allows to resume deposits", async () => { - await stakingVault.connect(vaultOwner).pauseBeaconChainDeposits(); + context("_depositToBeaconChain", () => { + it("makes deposits to the beacon chain and emits the DepositedToBeaconChain event", async () => { + const numberOfKeys = 2; // number because of Array.from + const totalAmount = ether("32") * BigInt(numberOfKeys); + const withdrawalCredentials = await stakingVault.withdrawalCredentials(); - await expect(stakingVault.connect(vaultOwner).resumeBeaconChainDeposits()).to.emit( - stakingVault, - "BeaconChainDepositsResumed", - ); - expect(await stakingVault.beaconChainDepositsPaused()).to.be.false; + await stakingVault.fund({ value: totalAmount }); + + const deposits = Array.from({ length: numberOfKeys }, (_, i) => { + const pubkey = `0x${getPubkey(i + 1)}`; + const signature = `0x${getSignature(i + 1)}`; + const amount = ether("32"); + const depositDataRoot = computeDepositDataRoot(withdrawalCredentials, pubkey, signature, amount); + return { pubkey, signature, amount, depositDataRoot }; + }); + + await expect(stakingVault.connect(operator).depositToBeaconChain(deposits)) + .to.emit(stakingVault, "DepositedToBeaconChain") + .withArgs(operator, 2, totalAmount); }); }); - context("depositToBeaconChain", () => { - it("reverts if called by a non-operator", async () => { - await expect( - stakingVault - .connect(stranger) - .depositToBeaconChain([ - { pubkey: "0x", signature: "0x", amount: 0, depositDataRoot: streccak("random-root") }, - ]), - ) - .to.be.revertedWithCustomError(stakingVault, "NotAuthorized") - .withArgs("depositToBeaconChain", stranger); - }); + context("_calculateTotalExitRequestFee", () => { + it("returns the total fee for given number of validator keys", async () => { + const newFee = 100n; + await withdrawalRequest.setFee(newFee); - it("reverts if the number of deposits is zero", async () => { - await expect(stakingVault.depositToBeaconChain([])) - .to.be.revertedWithCustomError(stakingVault, "ZeroArgument") - .withArgs("_deposits"); - }); + const fee = await stakingVault.calculateTotalExitRequestFee(1n); + expect(fee).to.equal(newFee); - it("reverts if the vault is not balanced", async () => { - await stakingVault.connect(vaultHubSigner).lock(ether("1")); - await expect( - stakingVault - .connect(operator) - .depositToBeaconChain([ - { pubkey: "0x", signature: "0x", amount: 0, depositDataRoot: streccak("random-root") }, - ]), - ).to.be.revertedWithCustomError(stakingVault, "Unbalanced"); - }); + const feePerRequest = await withdrawalRequest.fee(); + expect(fee).to.equal(feePerRequest); - it("reverts if the deposits are paused", async () => { - await stakingVault.connect(vaultOwner).pauseBeaconChainDeposits(); - await expect( - stakingVault - .connect(operator) - .depositToBeaconChain([ - { pubkey: "0x", signature: "0x", amount: 0, depositDataRoot: streccak("random-root") }, - ]), - ).to.be.revertedWithCustomError(stakingVault, "BeaconChainDepositsArePaused"); + const feeForMultipleKeys = await stakingVault.calculateTotalExitRequestFee(2n); + expect(feeForMultipleKeys).to.equal(newFee * 2n); }); + }); - it("makes deposits to the beacon chain and emits the DepositedToBeaconChain event", async () => { - await stakingVault.fund({ value: ether("32") }); - - const pubkey = "0x" + "ab".repeat(48); - const signature = "0x" + "ef".repeat(96); - const amount = ether("32"); - const withdrawalCredentials = await stakingVault.withdrawalCredentials(); - const depositDataRoot = computeDepositDataRoot(withdrawalCredentials, pubkey, signature, amount); + context("_requestValidatorsExit", () => { + it("reverts if passed fee is less than the required fee", async () => { + const numberOfKeys = 4; + const pubkeys = getPubkeys(numberOfKeys); + const fee = await stakingVault.calculateTotalExitRequestFee(numberOfKeys - 1); - await expect( - stakingVault.connect(operator).depositToBeaconChain([{ pubkey, signature, amount, depositDataRoot }]), - ) - .to.emit(stakingVault, "DepositedToBeaconChain") - .withArgs(operator, 1, amount); + await expect(stakingVault.connect(vaultOwner).requestValidatorsExit(pubkeys, { value: fee })) + .to.be.revertedWithCustomError(stakingVault, "InsufficientExitFee") + .withArgs(fee, numberOfKeys); }); - }); - context("calculateTotalExitRequestFee", () => { - it("reverts if the number of keys is zero", async () => { - await expect(stakingVault.calculateTotalExitRequestFee(0)) - .to.be.revertedWithCustomError(stakingVault, "ZeroArgument") - .withArgs("_numberOfKeys"); + it("allows owner to request validators exit providing a fee", async () => { + const numberOfKeys = 1; + const pubkeys = getPubkeys(numberOfKeys); + const fee = await stakingVault.calculateTotalExitRequestFee(numberOfKeys); + + await expect(stakingVault.connect(vaultOwner).requestValidatorsExit(pubkeys, { value: fee })) + .to.emit(stakingVault, "ValidatorsExitRequested") + .withArgs(vaultOwnerAddress, pubkeys); }); - it("returns the total fee for given number of validator keys", async () => { - const fee = await stakingVault.calculateTotalExitRequestFee(1); - expect(fee).to.equal(1); + it("refunds the fee if passed fee is greater than the required fee", async () => { + const numberOfKeys = 1; + const pubkeys = getPubkeys(numberOfKeys); + const fee = await stakingVault.calculateTotalExitRequestFee(numberOfKeys); + const overpaid = 100n; + + await expect(stakingVault.connect(vaultOwner).requestValidatorsExit(pubkeys, { value: fee + overpaid })) + .to.emit(stakingVault, "ValidatorsExitRequested") + .withArgs(vaultOwnerAddress, pubkeys) + .and.to.emit(stakingVault, "ExitFeeRefunded") + .withArgs(vaultOwnerAddress, overpaid); }); - }); - context("requestValidatorsExit", () => { - context("vault is balanced", () => { + context.skip("vault is balanced", () => { it("reverts if called by a non-owner or non-node operator", async () => { const keys = getValidatorPubkey(1); await expect(stakingVault.connect(stranger).requestValidatorsExit(keys)) @@ -216,7 +203,7 @@ describe("StakingVault.sol:ValidatorsManagement", () => { }); }); - context("vault is unbalanced", () => { + context.skip("vault is unbalanced", () => { beforeEach(async () => { await stakingVault.connect(vaultHubSigner).report(ether("1"), ether("0.1"), ether("1.1")); expect(await stakingVault.isBalanced()).to.be.false;