diff --git a/contracts/0.8.25/vaults/predeposit_guarantee/MeIfNobodyElse.sol b/contracts/0.8.25/vaults/predeposit_guarantee/MeIfNobodyElse.sol new file mode 100644 index 0000000000..a8b3e5bc64 --- /dev/null +++ b/contracts/0.8.25/vaults/predeposit_guarantee/MeIfNobodyElse.sol @@ -0,0 +1,21 @@ +// SPDX-FileCopyrightText: 2025 Lido +// SPDX-License-Identifier: GPL-3.0 + +// See contracts/COMPILERS.md +pragma solidity 0.8.25; + +/// @title MeIfNobodyElse +/// @author Lido +/// @notice A library for mapping(address => address) that defaults to the key if the value is not set +library MeIfNobodyElse { + /// @notice Returns the value for the key if it is set, otherwise returns the key + function get(mapping(address => address) storage map, address key) internal view returns (address) { + address value = map[key]; + return value == address(0) ? key : value; + } + + /// @notice Sets the value for the key if it is not the key itself, otherwise resets the value to the zero address + function set(mapping(address => address) storage map, address key, address value) internal { + map[key] = key == value ? address(0) : value; + } +} diff --git a/contracts/0.8.25/vaults/predeposit_guarantee/PredepositGuarantee.sol b/contracts/0.8.25/vaults/predeposit_guarantee/PredepositGuarantee.sol index 0aa98b2cb3..ff4dcda983 100644 --- a/contracts/0.8.25/vaults/predeposit_guarantee/PredepositGuarantee.sol +++ b/contracts/0.8.25/vaults/predeposit_guarantee/PredepositGuarantee.sol @@ -10,6 +10,7 @@ import {BLS12_381} from "contracts/0.8.25/lib/BLS.sol"; import {PausableUntilWithRoles} from "contracts/0.8.25/utils/PausableUntilWithRoles.sol"; import {CLProofVerifier} from "./CLProofVerifier.sol"; +import {MeIfNobodyElse} from "./MeIfNobodyElse.sol"; import {IStakingVault} from "../interfaces/IStakingVault.sol"; import {IPredepositGuarantee} from "../interfaces/IPredepositGuarantee.sol"; @@ -31,6 +32,8 @@ import {IPredepositGuarantee} from "../interfaces/IPredepositGuarantee.sol"; * Internal guards for NO<->Guarantor are used only to prevent mistakes and provide operational recovery paths. * But can not be used to fully prevent misbehavior in this relationship where NO's can access guarantor provided ether. * + * !NB: + * There is a mutual trust assumption between NO's and the assigned depositor. * * !NB: * PDG is permissionless by design. Anyone can be an NO, provided there is a compatible staking vault @@ -40,6 +43,8 @@ import {IPredepositGuarantee} from "../interfaces/IPredepositGuarantee.sol"; * - PDG can be used outside of Lido */ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableUntilWithRoles { + using MeIfNobodyElse for mapping(address => address); + /** * @notice represents validator stages in PDG flow * @param NONE - initial stage @@ -93,6 +98,7 @@ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableU mapping(address nodeOperator => address guarantor) nodeOperatorGuarantor; mapping(address guarantor => uint256 claimableEther) guarantorClaimableEther; mapping(bytes validatorPubkey => ValidatorStatus validatorStatus) validatorStatus; + mapping(address nodeOperator => address depositor) nodeOperatorDepositor; } uint8 public constant MIN_SUPPORTED_WC_VERSION = 0x01; @@ -175,6 +181,15 @@ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableU return _guarantorOf(_nodeOperator); } + /** + * @notice returns address of the depositor for the NO + * @param _nodeOperator to check depositor for + * @return address of depositor for the NO + */ + function nodeOperatorDepositor(address _nodeOperator) external view returns (address) { + return _depositorOf(_nodeOperator); + } + /** * @notice returns amount of ether refund that guarantor can claim * @param _guarantor address of the guarantor @@ -281,11 +296,25 @@ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableU emit GuarantorRefundAdded(prevGuarantor, msg.sender, refund); } - $.nodeOperatorGuarantor[msg.sender] = _newGuarantor != msg.sender ? _newGuarantor : address(0); + $.nodeOperatorGuarantor.set(msg.sender, _newGuarantor); emit GuarantorSet(msg.sender, _newGuarantor, prevGuarantor); } + /** + * @notice sets the depositor for the NO + * @param _newDepositor address of the depositor + */ + function setNodeOperatorDepositor(address _newDepositor) external { + if (_newDepositor == address(0)) revert ZeroArgument("_newDepositor"); + address prevDepositor = _depositorOf(msg.sender); + if (_newDepositor == prevDepositor) revert SameDepositor(); + + _getStorage().nodeOperatorDepositor.set(msg.sender, _newDepositor); + + emit DepositorSet(msg.sender, _newDepositor, prevDepositor); + } + /** * @notice claims refund for the previous guarantor of the NO * @param _recipient address to send the refund to @@ -325,7 +354,7 @@ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableU if (_deposits.length == 0) revert EmptyDeposits(); address nodeOperator = _stakingVault.nodeOperator(); - if (msg.sender != nodeOperator) revert NotNodeOperator(); + if (msg.sender != _depositorOf(nodeOperator)) revert NotDepositor(); if (msg.value != 0) { // check that node operator is self-guarantor is inside @@ -406,8 +435,8 @@ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableU IStakingVault _stakingVault, IStakingVault.Deposit[] calldata _deposits ) public payable whenResumed { - if (msg.sender != _stakingVault.nodeOperator()) { - revert NotNodeOperator(); + if (msg.sender != _depositorOf(_stakingVault.nodeOperator())) { + revert NotDepositor(); } ERC7201Storage storage $ = _getStorage(); @@ -421,8 +450,8 @@ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableU } // sanity check because first check relies on external contract - if (validator.nodeOperator != msg.sender) { - revert NotNodeOperator(); + if (_depositorOf(validator.nodeOperator) != msg.sender) { + revert NotDepositor(); } if (validator.stakingVault != _stakingVault) { @@ -612,8 +641,7 @@ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableU /// @notice returns guarantor of the NO /// @dev if guarantor is not set, returns NO address function _guarantorOf(address _nodeOperator) internal view returns (address) { - address stored = _getStorage().nodeOperatorGuarantor[_nodeOperator]; - return stored == address(0) ? _nodeOperator : stored; + return _getStorage().nodeOperatorGuarantor.get(_nodeOperator); } /// @notice enforces that only NO's guarantor can call the function @@ -624,6 +652,12 @@ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableU _; } + /// @notice returns depositor of the NO + /// @dev if depositor is not set, returns NO address + function _depositorOf(address _nodeOperator) internal view returns (address) { + return _getStorage().nodeOperatorDepositor.get(_nodeOperator); + } + /// @notice validates that WC belong to the vault function _validateWC(IStakingVault _stakingVault, bytes32 _withdrawalCredentials) internal pure { uint8 version = uint8(_withdrawalCredentials[0]); @@ -659,9 +693,11 @@ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableU event BalanceCompensated(address indexed nodeOperator, address indexed to, uint128 total, uint128 locked); event BalanceRefunded(address indexed nodeOperator, address indexed to); - /// NO Guarantor events + /// NO delegate events event GuarantorSet(address indexed nodeOperator, address indexed newGuarantor, address indexed prevGuarantor); + event DepositorSet(address indexed nodeOperator, address indexed newDepositor, address indexed prevDepositor); + event GuarantorRefundAdded(address indexed guarantor, address indexed nodeOperator, uint256 amount); event GuarantorRefundClaimed(address indexed guarantor, address indexed recipient, uint256 amount); @@ -700,6 +736,7 @@ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableU error NothingToRefund(); error WithdrawalFailed(); error SameGuarantor(); + error SameDepositor(); error RefundFailed(); // predeposit errors @@ -727,7 +764,7 @@ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableU // auth error NotStakingVaultOwner(); error NotGuarantor(); - error NotNodeOperator(); + error NotDepositor(); // general error ZeroArgument(string argument); diff --git a/test/0.8.25/vaults/predeposit-guarantee/contracts/StakingVault__MockForPDG.sol b/test/0.8.25/vaults/predeposit-guarantee/contracts/StakingVault__MockForPDG.sol new file mode 100644 index 0000000000..088cc6839c --- /dev/null +++ b/test/0.8.25/vaults/predeposit-guarantee/contracts/StakingVault__MockForPDG.sol @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: UNLICENSED +// for testing purposes only + +pragma solidity 0.8.25; + +import {IStakingVault} from "contracts/0.8.25/vaults/interfaces/IStakingVault.sol"; + +contract StakingVault__MockForPDG { + event Mock_depositToBeaconChain(address indexed _depositor, uint256 _depositCount, uint256 _totalDepositAmount); + + uint256 private constant WC_0X02_PREFIX = 0x02 << 248; + + address private nodeOperator_; + address private owner_; + bytes32 private withdrawalCredentials_; + + constructor(address _owner, address _nodeOperator) { + owner_ = _owner; + nodeOperator_ = _nodeOperator; + } + + function totalValue() external view returns (uint256) { + return address(this).balance; + } + + function fund() external payable {} + + function setNodeOperator(address _nodeOperator) external { + nodeOperator_ = _nodeOperator; + } + + function setOwner(address _owner) external { + owner_ = _owner; + } + + function withdrawalCredentials() public view returns (bytes32) { + return withdrawalCredentials_ == bytes32(0) ? bytes32(WC_0X02_PREFIX | uint160(address(this))) : withdrawalCredentials_; + } + + function nodeOperator() external view returns (address) { + return nodeOperator_; + } + + function owner() external view returns (address) { + return owner_; + } + + function depositToBeaconChain(IStakingVault.Deposit[] calldata _deposits) external { + uint256 totalDepositAmount = 0; + for (uint256 i = 0; i < _deposits.length; i++) { + totalDepositAmount += _deposits[i].amount; + } + + emit Mock_depositToBeaconChain(msg.sender, _deposits.length, totalDepositAmount); + } + + function mock__setWithdrawalCredentials(bytes32 _withdrawalCredentials) external { + withdrawalCredentials_ = _withdrawalCredentials; + } +} \ No newline at end of file diff --git a/test/0.8.25/vaults/predeposit-guarantee/contracts/VaultHub__MockForPDG.sol b/test/0.8.25/vaults/predeposit-guarantee/contracts/VaultHub__MockForPDG.sol new file mode 100644 index 0000000000..92af4c7068 --- /dev/null +++ b/test/0.8.25/vaults/predeposit-guarantee/contracts/VaultHub__MockForPDG.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: UNLICENSED +// for testing purposes only + +pragma solidity 0.8.25; + + + +contract VaultHub__MockForPDG { + +} \ No newline at end of file diff --git a/test/0.8.25/vaults/predeposit-guarantee/predeposit-guarantee.test.ts b/test/0.8.25/vaults/predeposit-guarantee/predeposit-guarantee.test.ts index b61027e10e..4498ad7d9d 100644 --- a/test/0.8.25/vaults/predeposit-guarantee/predeposit-guarantee.test.ts +++ b/test/0.8.25/vaults/predeposit-guarantee/predeposit-guarantee.test.ts @@ -5,24 +5,19 @@ import { ethers } from "hardhat"; import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers"; import { - DepositContract__MockForStakingVault, EthRejector, IPredepositGuarantee, LidoLocator, OssifiableProxy, PredepositGuarantee, SSZMerkleTree, - StakingVault, - StakingVault__factory, - StakingVault__MockForVaultHub, - VaultFactory__MockForStakingVault, - VaultHub__MockForStakingVault, + StakingVault__MockForPDG } from "typechain-types"; import { addressToWC, + certainAddress, ether, - findEvents, generateBeaconHeader, generatePostDeposit, generatePredeposit, @@ -50,43 +45,14 @@ describe("PredepositGuarantee.sol", () => { let pdgImpl: PredepositGuarantee; let pdg: PredepositGuarantee; let locator: LidoLocator; - let vaultHub: VaultHub__MockForStakingVault; let sszMerkleTree: SSZMerkleTree; - let stakingVault: StakingVault; - let wcMockStakingVault: StakingVault__MockForVaultHub; - let depositContract: DepositContract__MockForStakingVault; + let stakingVault: StakingVault__MockForPDG; let rejector: EthRejector; let firstValidatorLeafIndex: bigint; let originalState: string; - async function deployStakingVault( - owner: HardhatEthersSigner, - operator: HardhatEthersSigner, - vaultHub_: VaultHub__MockForStakingVault, - ): Promise { - const stakingVaultImplementation_ = await ethers.deployContract("StakingVault", [vaultHub_, depositContract]); - - // deploying factory/beacon - const vaultFactory_: VaultFactory__MockForStakingVault = await ethers.deployContract( - "VaultFactory__MockForStakingVault", - [await stakingVaultImplementation_.getAddress()], - ); - - // deploying beacon proxy - const vaultCreation = await vaultFactory_.createVault(owner, operator, pdg).then((tx) => tx.wait()); - if (!vaultCreation) throw new Error("Vault creation failed"); - const events = findEvents(vaultCreation, "VaultCreated"); - if (events.length != 1) throw new Error("There should be exactly one VaultCreated event"); - const vaultCreatedEvent = events[0]; - - const stakingVault_ = StakingVault__factory.connect(vaultCreatedEvent.args.vault, owner); - expect(await stakingVault_.owner()).to.equal(owner); - - return stakingVault_; - } - before(async () => { [deployer, admin, vaultOwner, vaultOperator, vaultOperatorGuarantor, pauser, stranger] = await ethers.getSigners(); @@ -98,8 +64,8 @@ describe("PredepositGuarantee.sol", () => { // eth rejector rejector = await ethers.deployContract("EthRejector"); - // ether deposit contract - depositContract = await ethers.deployContract("DepositContract__MockForStakingVault"); + // staking vault + stakingVault = await ethers.deployContract("StakingVault__MockForPDG", [vaultOwner, vaultOperator]); // PDG pdgImpl = await ethers.deployContract( @@ -117,10 +83,6 @@ describe("PredepositGuarantee.sol", () => { // PDG dependants locator = await deployLidoLocator({ predepositGuarantee: pdg }); expect(await locator.predepositGuarantee()).to.equal(await pdg.getAddress()); - vaultHub = await ethers.deployContract("VaultHub__MockForStakingVault", [locator]); - stakingVault = await deployStakingVault(vaultOwner, vaultOperator, vaultHub); - wcMockStakingVault = await ethers.deployContract("StakingVault__MockForVaultHub", [vaultHub, depositContract]); - await wcMockStakingVault.initialize(vaultOwner, vaultOperator, pdg, "0x00"); }); beforeEach(async () => (originalState = await Snapshot.take())); @@ -190,10 +152,8 @@ describe("PredepositGuarantee.sol", () => { await expect(predepositTX) .to.emit(pdg, "ValidatorPreDeposited") .withArgs(deposit.pubkey, vaultOperator, stakingVault, vaultWC) - .to.emit(stakingVault, "DepositedToBeaconChain") - .withArgs(pdg, 1, deposit.amount) - .to.emit(depositContract, "DepositEvent") - .withArgs(deposit.pubkey, vaultWC, deposit.signature, deposit.depositDataRoot); + .to.emit(stakingVault, "Mock_depositToBeaconChain") + .withArgs(pdg, 1, deposit.amount); [operatorBondTotal, operatorBondLocked] = await pdg.nodeOperatorBalance(vaultOperator); expect(operatorBondTotal).to.equal(ether("1")); @@ -237,10 +197,8 @@ describe("PredepositGuarantee.sol", () => { await expect(proveAndDepositTx) .to.emit(pdg, "ValidatorProven") .withArgs(validator.container.pubkey, vaultOperator, stakingVault, vaultWC) - .to.emit(stakingVault, "DepositedToBeaconChain") - .withArgs(pdg, 1, postDepositData.amount) - .to.emit(depositContract, "DepositEvent") - .withArgs(postDepositData.pubkey, vaultWC, postDepositData.signature, postDepositData.depositDataRoot); + .to.emit(stakingVault, "Mock_depositToBeaconChain") + .withArgs(pdg, 1, postDepositData.amount); [operatorBondTotal, operatorBondLocked] = await pdg.nodeOperatorBalance(vaultOperator); expect(operatorBondTotal).to.equal(ether("1")); @@ -585,10 +543,10 @@ describe("PredepositGuarantee.sol", () => { ); await expect( pdg.connect(vaultOwner).predeposit(stakingVault, [deposit], [depositY]), - ).to.be.revertedWithCustomError(pdg, "NotNodeOperator"); + ).to.be.revertedWithCustomError(pdg, "NotDepositor"); await expect( pdg.connect(stranger).predeposit(stakingVault, [deposit], [depositY]), - ).to.be.revertedWithCustomError(pdg, "NotNodeOperator"); + ).to.be.revertedWithCustomError(pdg, "NotDepositor"); }); it("reverts when using locked balance", async () => { @@ -729,7 +687,7 @@ describe("PredepositGuarantee.sol", () => { await expect(predepositTX) .to.emit(pdg, "BalanceLocked") .withArgs(vaultOperator, totalBalance, totalBalance) - .to.emit(stakingVault, "DepositedToBeaconChain") + .to.emit(stakingVault, "Mock_depositToBeaconChain") .withArgs(pdg, batchCount, totalBalance); expect(await pdg.nodeOperatorBalance(vaultOperator)).to.deep.equal([totalBalance, totalBalance]); @@ -739,7 +697,7 @@ describe("PredepositGuarantee.sol", () => { context("invalid WC vault", () => { it("reverts when vault has WC with wrong version", async () => { - let wc = await wcMockStakingVault.withdrawalCredentials(); + let wc = await stakingVault.withdrawalCredentials(); await pdg.topUpNodeOperatorBalance(vaultOperator, { value: ether("200") }); const min = await pdg.MIN_SUPPORTED_WC_VERSION(); @@ -751,39 +709,39 @@ describe("PredepositGuarantee.sol", () => { for (let version = 0n; version < 5n; version++) { wc = `0x0${version.toString()}` + wc.slice(4); const predeposit = await generatePredeposit(generateValidator(wc)); - await wcMockStakingVault.mock__setWithdrawalCredentials(wc); + await stakingVault.mock__setWithdrawalCredentials(wc); const shouldRevert = version < min || version > max; if (shouldRevert) { - await expect(pdg.predeposit(wcMockStakingVault, [predeposit.deposit], [predeposit.depositY])) + await expect(pdg.predeposit(stakingVault, [predeposit.deposit], [predeposit.depositY])) .to.be.revertedWithCustomError(pdg, "WithdrawalCredentialsInvalidVersion") .withArgs(version); } else { - await pdg.predeposit(wcMockStakingVault, [predeposit.deposit], [predeposit.depositY]); + await pdg.predeposit(stakingVault, [predeposit.deposit], [predeposit.depositY]); } } }); it("reverts when WC are misformed", async () => { - let wc = await wcMockStakingVault.withdrawalCredentials(); + let wc = await stakingVault.withdrawalCredentials(); await pdg.topUpNodeOperatorBalance(vaultOperator, { value: ether("200") }); wc = wc.slice(0, 4) + "ff" + wc.slice(6); - await wcMockStakingVault.mock__setWithdrawalCredentials(wc); + await stakingVault.mock__setWithdrawalCredentials(wc); const predeposit = await generatePredeposit(generateValidator(wc)); - await expect(pdg.predeposit(wcMockStakingVault, [predeposit.deposit], [predeposit.depositY])) + await expect(pdg.predeposit(stakingVault, [predeposit.deposit], [predeposit.depositY])) .to.be.revertedWithCustomError(pdg, "WithdrawalCredentialsMisformed") .withArgs(wc); }); it("reverts when WC do not belong to the vault", async () => { await pdg.topUpNodeOperatorBalance(vaultOperator, { value: ether("200") }); - await wcMockStakingVault.mock__setWithdrawalCredentials(addressToWC(stranger.address)); - const wc = await wcMockStakingVault.withdrawalCredentials(); + await stakingVault.mock__setWithdrawalCredentials(addressToWC(stranger.address)); + const wc = await stakingVault.withdrawalCredentials(); const predeposit = await generatePredeposit(generateValidator(wc)); - await expect(pdg.predeposit(wcMockStakingVault, [predeposit.deposit], [predeposit.depositY])) + await expect(pdg.predeposit(stakingVault, [predeposit.deposit], [predeposit.depositY])) .to.be.revertedWithCustomError(pdg, "WithdrawalCredentialsMismatch") - .withArgs(await wcMockStakingVault.getAddress(), stranger.address); + .withArgs(await stakingVault.getAddress(), stranger.address); }); }); @@ -990,13 +948,20 @@ describe("PredepositGuarantee.sol", () => { await expect(pdg.connect(stranger).depositToBeaconChain(stakingVault, [deposit])).to.be.revertedWithCustomError( pdg, - "NotNodeOperator", + "NotDepositor", ); }); + it("reverts when deposits are delegated to a depositor", async () => { + await pdg.connect(vaultOperator).setNodeOperatorDepositor(stranger); + const validator = generateValidator(); + const deposit = generatePostDeposit(validator.container); + await expect(pdg.connect(vaultOperator).depositToBeaconChain(stakingVault, [deposit])).to.be.revertedWithCustomError(pdg, "NotDepositor"); + }); + it("reverts to deposit someone else validators", async () => { - const sideStakingVault = await deployStakingVault(stranger, stranger, vaultHub); - const sameNOVault = await deployStakingVault(stranger, vaultOperator, vaultHub); + const sideStakingVault = await ethers.deployContract("StakingVault__MockForPDG", [stranger, stranger]); + const sameNOVault = await ethers.deployContract("StakingVault__MockForPDG", [stranger, vaultOperator]); const sideValidator = generateValidator(await sideStakingVault.withdrawalCredentials()); const mainValidator = generateValidator(await stakingVault.withdrawalCredentials()); const sameNOValidator = generateValidator(await sameNOVault.withdrawalCredentials()); @@ -1080,7 +1045,7 @@ describe("PredepositGuarantee.sol", () => { await expect(pdg.depositToBeaconChain(stakingVault, [mainDeposit, sideDeposit])).to.be.revertedWithCustomError( pdg, - "NotNodeOperator", + "NotDepositor", ); await expect(pdg.depositToBeaconChain(stakingVault, [mainDeposit, sameNoDeposit])) @@ -1443,6 +1408,26 @@ describe("PredepositGuarantee.sol", () => { }); }); + context("nodeOperatorDepositor", () => { + it("returns the node operator if not set", async () => { + expect(await pdg.nodeOperatorDepositor(vaultOperator)).to.equal(vaultOperator); + }); + + it("returns the depositor if set", async () => { + const depositor = certainAddress("depositor"); + await expect(pdg.setNodeOperatorDepositor(depositor)).to.emit(pdg, "DepositorSet").withArgs(vaultOperator, depositor, vaultOperator); + expect(await pdg.nodeOperatorDepositor(vaultOperator)).to.equal(depositor); + }); + + it("reverts if trying to set the same depositor", async () => { + await expect(pdg.setNodeOperatorDepositor(vaultOperator)).to.be.revertedWithCustomError(pdg, "SameDepositor"); + }); + + it("reverts if trying to set the depositor to zero address", async () => { + await expect(pdg.setNodeOperatorDepositor(ZeroAddress)).to.be.revertedWithCustomError(pdg, "ZeroArgument"); + }); + }); + context("Pausing", () => { it("should pause core methods", async () => { // Roles