From bdcbeaf8d5ebe7f20430b829c9bbe2f892ab5b12 Mon Sep 17 00:00:00 2001 From: failingtwice Date: Mon, 13 Oct 2025 19:11:43 +0500 Subject: [PATCH 1/9] feat(Dashboard): reset settled growth on disconnect --- contracts/0.8.25/vaults/VaultFactory.sol | 7 +- .../0.8.25/vaults/dashboard/Dashboard.sol | 7 +- .../vaults/dashboard/NodeOperatorFee.sol | 34 +--- .../0.8.25/vaults/dashboard/dashboard.test.ts | 159 +----------------- .../nodeOperatorFee/nodeOperatorFee.test.ts | 4 - .../vaults/disconnected.integration.ts | 3 - .../vaults/operator.grid.integration.ts | 1 - .../scenario/lazyOracle.report.integration.ts | 1 - 8 files changed, 12 insertions(+), 204 deletions(-) diff --git a/contracts/0.8.25/vaults/VaultFactory.sol b/contracts/0.8.25/vaults/VaultFactory.sol index 34d62a6076..23be39db56 100644 --- a/contracts/0.8.25/vaults/VaultFactory.sol +++ b/contracts/0.8.25/vaults/VaultFactory.sol @@ -79,14 +79,9 @@ contract VaultFactory { vault.initialize(address(dashboard), _nodeOperator, locator.predepositGuarantee()); // initialize Dashboard with the factory address as the default admin, grant optional roles and connect to VaultHub - dashboard.initialize(address(this), address(this), _nodeOperatorManager, _nodeOperatorFeeBP, _confirmExpiry); + dashboard.initialize(address(this), _nodeOperatorManager, _nodeOperatorManager, _nodeOperatorFeeBP, _confirmExpiry); - // connection must be pre-approved by the node operator manager - dashboard.setApprovedToConnect(true); dashboard.connectToVaultHub{value: msg.value}(); - - dashboard.grantRole(dashboard.NODE_OPERATOR_MANAGER_ROLE(), _nodeOperatorManager); - dashboard.revokeRole(dashboard.NODE_OPERATOR_MANAGER_ROLE(), address(this)); // _roleAssignments can only include DEFAULT_ADMIN_ROLE's subroles, // which is why it's important to revoke the NODE_OPERATOR_MANAGER_ROLE BEFORE granting roles diff --git a/contracts/0.8.25/vaults/dashboard/Dashboard.sol b/contracts/0.8.25/vaults/dashboard/Dashboard.sol index 96bde967f2..52fb4b2287 100644 --- a/contracts/0.8.25/vaults/dashboard/Dashboard.sol +++ b/contracts/0.8.25/vaults/dashboard/Dashboard.sol @@ -251,9 +251,11 @@ contract Dashboard is NodeOperatorFee { * @notice Disconnects the underlying StakingVault from the hub and passing its ownership to Dashboard. * After receiving the final report, one can call reconnectToVaultHub() to reconnect to the hub * or abandonDashboard() to transfer the ownership to a new owner. + * Resets the settled growth to 0 to encourage correction before reconnection. */ function voluntaryDisconnect() external { disburseFee(); + if (settledGrowth != 0) _setSettledGrowth(0); _voluntaryDisconnect(); } @@ -284,14 +286,9 @@ contract Dashboard is NodeOperatorFee { * @notice Connects to VaultHub, transferring ownership to VaultHub. */ function connectToVaultHub() public payable { - if (!isApprovedToConnect) revert ForbiddenToConnectByNodeOperator(); - if (msg.value > 0) _stakingVault().fund{value: msg.value}(); _transferOwnership(address(VAULT_HUB)); VAULT_HUB.connectVault(address(_stakingVault())); - - // node operator approval is one time only and is reset after connect - _setApprovedToConnect(false); } /** diff --git a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol index 90f46890e3..dd63f5d2d4 100644 --- a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol +++ b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol @@ -93,13 +93,6 @@ contract NodeOperatorFee is Permissions { */ uint64 public latestCorrectionTimestamp; - /** - * @notice Flag indicating whether the vault is approved by the node operator to connect to VaultHub. - * The node operator's approval is needed to confirm the validity of fee calculations, - * particularly the settled growth. - */ - bool public isApprovedToConnect; - /** * @notice Passes the address of the vault hub up the inheritance chain. * @param _vaultHub The address of the vault hub. @@ -170,15 +163,6 @@ contract NodeOperatorFee is Permissions { (fee, ) = _calculateFee(); } - /** - * @notice Approves/forbids connection to VaultHub. Approval implies that the node operator agrees - * with the current fee parameters, particularly the settled growth used as baseline for fee calculations. - * @param _isApproved True to approve, False to forbid - */ - function setApprovedToConnect(bool _isApproved) external onlyRoleMemberOrAdmin(NODE_OPERATOR_MANAGER_ROLE) { - _setApprovedToConnect(_isApproved); - } - /** * @notice Permissionless function to disburse node operator fees. * @@ -284,13 +268,7 @@ contract NodeOperatorFee is Permissions { return LazyOracle(LIDO_LOCATOR.lazyOracle()); } - function _setApprovedToConnect(bool _isApproved) internal { - isApprovedToConnect = _isApproved; - - emit ApprovedToConnectSet(_isApproved); - } - - function _setSettledGrowth(int256 _newSettledGrowth) private { + function _setSettledGrowth(int256 _newSettledGrowth) internal { int128 oldSettledGrowth = settledGrowth; if (oldSettledGrowth == _newSettledGrowth) revert SameSettledGrowth(); @@ -389,11 +367,6 @@ contract NodeOperatorFee is Permissions { */ event CorrectionTimestampUpdated(uint64 timestamp); - /** - * @dev Emitted when the node operator approves/forbids to connect to VaultHub. - */ - event ApprovedToConnectSet(bool isApproved); - // ==================== Errors ==================== /** @@ -431,11 +404,6 @@ contract NodeOperatorFee is Permissions { */ error UnexpectedFeeExemptionAmount(); - /** - * @dev Error emitted when the settled growth is pending manual adjustment. - */ - error ForbiddenToConnectByNodeOperator(); - /** * @dev Error emitted when the vault is quarantined. */ diff --git a/test/0.8.25/vaults/dashboard/dashboard.test.ts b/test/0.8.25/vaults/dashboard/dashboard.test.ts index 567635b6ef..1241a3d1a1 100644 --- a/test/0.8.25/vaults/dashboard/dashboard.test.ts +++ b/test/0.8.25/vaults/dashboard/dashboard.test.ts @@ -321,7 +321,6 @@ describe("Dashboard.sol", () => { expect(await dashboard.LIDO_LOCATOR()).to.equal(lidoLocator); expect(await dashboard.settledGrowth()).to.equal(0n); expect(await dashboard.latestCorrectionTimestamp()).to.equal(0n); - expect(await dashboard.isApprovedToConnect()).to.be.false; expect(await dashboard.feeRate()).to.equal(nodeOperatorFeeBP); expect(await dashboard.feeRecipient()).to.equal(nodeOperator); expect(await dashboard.getConfirmExpiry()).to.equal(confirmExpiry); @@ -626,23 +625,13 @@ describe("Dashboard.sol", () => { }); it("reverts if called by a non-admin", async () => { - await newDashboard.connect(nodeOperator).setApprovedToConnect(true); await expect(newDashboard.connect(stranger).connectAndAcceptTier(1, 1n)).to.be.revertedWithCustomError( newDashboard, "AccessControlUnauthorizedAccount", ); }); - it("reverts if connect is not approved by node operator", async () => { - expect(await newDashboard.isApprovedToConnect()).to.be.false; - await expect(newDashboard.connect(vaultOwner).connectAndAcceptTier(1, 1n)).to.be.revertedWithCustomError( - newDashboard, - "ForbiddenToConnectByNodeOperator", - ); - }); - it("reverts if change tier is not confirmed by node operator", async () => { - await newDashboard.connect(nodeOperator).setApprovedToConnect(true); await expect(newDashboard.connect(vaultOwner).connectAndAcceptTier(1, 1n)).to.be.revertedWithCustomError( newDashboard, "TierChangeNotConfirmed", @@ -650,7 +639,6 @@ describe("Dashboard.sol", () => { }); it("works", async () => { - await newDashboard.connect(nodeOperator).setApprovedToConnect(true); await operatorGrid.connect(nodeOperator).changeTier(newVault, 1, 1n); await expect(newDashboard.connect(vaultOwner).connectAndAcceptTier(1, 1n)).to.emit(hub, "Mock__VaultConnected"); }); @@ -658,7 +646,6 @@ describe("Dashboard.sol", () => { it("works with connection deposit", async () => { const connectDeposit = await hub.CONNECT_DEPOSIT(); - await newDashboard.connect(nodeOperator).setApprovedToConnect(true); await operatorGrid.connect(nodeOperator).changeTier(newVault, 1, 1n); await expect(newDashboard.connect(vaultOwner).connectAndAcceptTier(1, 1n, { value: connectDeposit })) .to.emit(hub, "Mock__VaultConnected") @@ -674,8 +661,16 @@ describe("Dashboard.sol", () => { }); it("invokes the voluntaryDisconnect function on the vault hub", async () => { + // set settled growth + await dashboard.connect(vaultOwner).correctSettledGrowth(1000n, 0n); + await dashboard.connect(nodeOperator).correctSettledGrowth(1000n, 0n); + expect(await dashboard.settledGrowth()).to.equal(1000n); + await dashboard.connect(vaultOwner).grantRole(await dashboard.VOLUNTARY_DISCONNECT_ROLE(), vaultOwner); await expect(dashboard.voluntaryDisconnect()).to.emit(hub, "Mock__VaultDisconnectInitiated").withArgs(vault); + + // settled growth is reset + expect(await dashboard.settledGrowth()).to.equal(0n); }); }); @@ -1497,147 +1492,9 @@ describe("Dashboard.sol", () => { expect(await vault.owner()).to.equal(vaultOwner); // reconnect - await dashboard.connect(nodeOperator).setApprovedToConnect(true); await vault.connect(vaultOwner).transferOwnership(dashboard); await dashboard.reconnectToVaultHub(); expect(await vault.owner()).to.equal(hub); }); }); - - context("Approval to Connect", () => { - let newVault: StakingVault; - let newDashboard: Dashboard; - - beforeEach(async () => { - // Create a new vault without hub connection for each test - const createVaultTx = await factory.createVaultWithDashboardWithoutConnectingToVaultHub( - vaultOwner.address, - nodeOperator.address, - nodeOperator.address, - nodeOperatorFeeBP, - confirmExpiry, - [], - ); - const createVaultReceipt = await createVaultTx.wait(); - if (!createVaultReceipt) throw new Error("Vault creation receipt not found"); - - const vaultCreatedEvents = findEvents(createVaultReceipt, "VaultCreated"); - expect(vaultCreatedEvents.length).to.equal(1); - - const newVaultAddress = vaultCreatedEvents[0].args.vault; - newVault = await ethers.getContractAt("StakingVault", newVaultAddress, vaultOwner); - - const dashboardCreatedEvents = findEvents(createVaultReceipt, "DashboardCreated"); - expect(dashboardCreatedEvents.length).to.equal(1); - - const newDashboardAddress = dashboardCreatedEvents[0].args.dashboard; - newDashboard = await ethers.getContractAt("Dashboard", newDashboardAddress, vaultOwner); - }); - - context("Initial state", () => { - it("should have isApprovedToConnect set to false initially", async () => { - expect(await newDashboard.isApprovedToConnect()).to.be.false; - }); - }); - - context("approveToConnect", () => { - it("allows node operator to approve connection", async () => { - expect(await newDashboard.isApprovedToConnect()).to.be.false; - - await expect(newDashboard.connect(nodeOperator).setApprovedToConnect(true)) - .to.emit(newDashboard, "ApprovedToConnectSet") - .withArgs(true); - - expect(await newDashboard.isApprovedToConnect()).to.be.true; - }); - - it("reverts if called by a stranger", async () => { - expect(await newDashboard.isApprovedToConnect()).to.be.false; - - await expect(newDashboard.connect(stranger).setApprovedToConnect(true)) - .to.be.revertedWithCustomError(newDashboard, "AccessControlUnauthorizedAccount") - .withArgs(stranger, await newDashboard.NODE_OPERATOR_MANAGER_ROLE()); - - expect(await newDashboard.isApprovedToConnect()).to.be.false; - }); - - it("should allow multiple calls to approveToConnect", async () => { - await newDashboard.connect(nodeOperator).setApprovedToConnect(true); - expect(await newDashboard.isApprovedToConnect()).to.be.true; - - // Should not revert when called again - await expect(newDashboard.connect(nodeOperator).setApprovedToConnect(true)) - .to.emit(newDashboard, "ApprovedToConnectSet") - .withArgs(true); - - expect(await newDashboard.isApprovedToConnect()).to.be.true; - }); - }); - - context("forbidToConnect", () => { - beforeEach(async () => { - // First approve to connect - await newDashboard.connect(nodeOperator).setApprovedToConnect(true); - expect(await newDashboard.isApprovedToConnect()).to.be.true; - }); - - it("allows node operator to forbid connection", async () => { - await expect(newDashboard.connect(nodeOperator).setApprovedToConnect(false)) - .to.emit(newDashboard, "ApprovedToConnectSet") - .withArgs(false); - - expect(await newDashboard.isApprovedToConnect()).to.be.false; - }); - - it("reverts when called by a stranger", async () => { - expect(await newDashboard.isApprovedToConnect()).to.be.true; - - await expect(newDashboard.connect(stranger).setApprovedToConnect(false)) - .to.be.revertedWithCustomError(newDashboard, "AccessControlUnauthorizedAccount") - .withArgs(stranger, await newDashboard.NODE_OPERATOR_MANAGER_ROLE()); - - expect(await newDashboard.isApprovedToConnect()).to.be.true; - }); - - it("allows multiple calls to forbidToConnect", async () => { - await newDashboard.connect(nodeOperator).setApprovedToConnect(false); - expect(await newDashboard.isApprovedToConnect()).to.be.false; - - // Should not revert when called again - await expect(newDashboard.connect(nodeOperator).setApprovedToConnect(false)) - .to.emit(newDashboard, "ApprovedToConnectSet") - .withArgs(false); - - expect(await newDashboard.isApprovedToConnect()).to.be.false; - }); - }); - - context("connectToVaultHub approval requirements", () => { - it("reverts when not approved to connect", async () => { - expect(await newDashboard.isApprovedToConnect()).to.be.false; - - await expect(newDashboard.connectToVaultHub()).to.be.revertedWithCustomError( - newDashboard, - "ForbiddenToConnectByNodeOperator", - ); - }); - - it("succeeds when approved to connect", async () => { - await newDashboard.connect(nodeOperator).setApprovedToConnect(true); - expect(await newDashboard.isApprovedToConnect()).to.be.true; - - await expect(newDashboard.connectToVaultHub()).to.emit(hub, "Mock__VaultConnected").withArgs(newVault); - }); - - it("resets approval after successful connection", async () => { - await newDashboard.connect(nodeOperator).setApprovedToConnect(true); - expect(await newDashboard.isApprovedToConnect()).to.be.true; - - await newDashboard.connectToVaultHub(); - - // Approval should be reset to false after connection - expect(await newDashboard.isApprovedToConnect()).to.be.false; - }); - }); - }); }); diff --git a/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts b/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts index 24c439dcf5..b4f1f81123 100644 --- a/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts +++ b/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts @@ -145,7 +145,6 @@ describe("NodeOperatorFee.sol", () => { expect(await nodeOperatorFee.accruedFee()).to.equal(0n); expect(await nodeOperatorFee.settledGrowth()).to.equal(0n); expect(await nodeOperatorFee.latestCorrectionTimestamp()).to.equal(0n); - expect(await nodeOperatorFee.isApprovedToConnect()).to.be.false; }); }); @@ -527,8 +526,6 @@ describe("NodeOperatorFee.sol", () => { msgData, ); - expect(await nodeOperatorFee.isApprovedToConnect()).to.be.false; - expect(await nodeOperatorFee.settledGrowth()).to.equal(currentSettledGrowth); confirmTimestamp = await getNextBlockTimestamp(); @@ -547,7 +544,6 @@ describe("NodeOperatorFee.sol", () => { expect(await nodeOperatorFee.settledGrowth()).to.deep.equal(newSettledGrowth); expect(await nodeOperatorFee.latestCorrectionTimestamp()).to.deep.equal(timestamp); - expect(await nodeOperatorFee.isApprovedToConnect()).to.be.false; }); }); diff --git a/test/integration/vaults/disconnected.integration.ts b/test/integration/vaults/disconnected.integration.ts index 4f20aebcff..059bbd8637 100644 --- a/test/integration/vaults/disconnected.integration.ts +++ b/test/integration/vaults/disconnected.integration.ts @@ -93,7 +93,6 @@ describe("Integration: Actions with vault disconnected from hub", () => { it("Can reconnect the vault to the hub", async () => { const { vaultHub } = ctx.contracts; - await dashboard.connect(nodeOperator).setApprovedToConnect(true); await dashboard.reconnectToVaultHub(); expect(await vaultHub.isVaultConnected(stakingVault)).to.equal(true); @@ -138,8 +137,6 @@ describe("Integration: Actions with vault disconnected from hub", () => { const { vaultHub } = ctx.contracts; - await dashboard.connect(nodeOperator).setApprovedToConnect(true); - await expect(dashboard.reconnectToVaultHub()) .to.emit(stakingVault, "OwnershipTransferred") .withArgs(owner, dashboard) diff --git a/test/integration/vaults/operator.grid.integration.ts b/test/integration/vaults/operator.grid.integration.ts index f31b27b82d..79e4ddc529 100644 --- a/test/integration/vaults/operator.grid.integration.ts +++ b/test/integration/vaults/operator.grid.integration.ts @@ -381,7 +381,6 @@ describe("Integration: OperatorGrid", () => { expect(await vaultHub.isVaultConnected(stakingVault)).to.be.false; // Reconnect vault - await dashboard.connect(nodeOperator).setApprovedToConnect(true); await dashboard.connect(owner).reconnectToVaultHub(); // Verify vault is reconnected diff --git a/test/integration/vaults/scenario/lazyOracle.report.integration.ts b/test/integration/vaults/scenario/lazyOracle.report.integration.ts index a030ce78de..14c66d87f2 100644 --- a/test/integration/vaults/scenario/lazyOracle.report.integration.ts +++ b/test/integration/vaults/scenario/lazyOracle.report.integration.ts @@ -55,7 +55,6 @@ describe("Scenario: Lazy Oracle prevents overwriting freshly reconnected vault r expect(await lazyOracle.latestReportTimestamp()).to.be.greaterThan(0); expect(await vaultHub.isVaultConnected(stakingVault)).to.be.false; - await dashboard.connect(nodeOperator).setApprovedToConnect(true); await dashboard.connect(owner).reconnectToVaultHub(); await expect( From 66bec361345889796e0b9c0fd69a596be038589b Mon Sep 17 00:00:00 2001 From: failingtwice Date: Tue, 14 Oct 2025 12:25:21 +0500 Subject: [PATCH 2/9] feat: reset settled growth on abandon --- contracts/0.8.25/vaults/dashboard/Dashboard.sol | 4 ++-- test/0.8.25/vaults/dashboard/dashboard.test.ts | 17 +++++++++-------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/contracts/0.8.25/vaults/dashboard/Dashboard.sol b/contracts/0.8.25/vaults/dashboard/Dashboard.sol index 52fb4b2287..8d704f688f 100644 --- a/contracts/0.8.25/vaults/dashboard/Dashboard.sol +++ b/contracts/0.8.25/vaults/dashboard/Dashboard.sol @@ -251,11 +251,9 @@ contract Dashboard is NodeOperatorFee { * @notice Disconnects the underlying StakingVault from the hub and passing its ownership to Dashboard. * After receiving the final report, one can call reconnectToVaultHub() to reconnect to the hub * or abandonDashboard() to transfer the ownership to a new owner. - * Resets the settled growth to 0 to encourage correction before reconnection. */ function voluntaryDisconnect() external { disburseFee(); - if (settledGrowth != 0) _setSettledGrowth(0); _voluntaryDisconnect(); } @@ -263,11 +261,13 @@ contract Dashboard is NodeOperatorFee { * @notice Accepts the ownership over the disconnected StakingVault transferred from VaultHub * and immediately passes it to a new pending owner. This new owner will have to accept the ownership * on the StakingVault contract. + * Resets the settled growth to 0 to encourage correction before reconnection. * @param _newOwner The address to transfer the StakingVault ownership to. */ function abandonDashboard(address _newOwner) external { if (VAULT_HUB.isVaultConnected(address(_stakingVault()))) revert ConnectedToVaultHub(); if (_newOwner == address(this)) revert DashboardNotAllowed(); + if (settledGrowth != 0) _setSettledGrowth(0); _acceptOwnership(); _transferOwnership(_newOwner); diff --git a/test/0.8.25/vaults/dashboard/dashboard.test.ts b/test/0.8.25/vaults/dashboard/dashboard.test.ts index 1241a3d1a1..4017e6d65b 100644 --- a/test/0.8.25/vaults/dashboard/dashboard.test.ts +++ b/test/0.8.25/vaults/dashboard/dashboard.test.ts @@ -661,16 +661,8 @@ describe("Dashboard.sol", () => { }); it("invokes the voluntaryDisconnect function on the vault hub", async () => { - // set settled growth - await dashboard.connect(vaultOwner).correctSettledGrowth(1000n, 0n); - await dashboard.connect(nodeOperator).correctSettledGrowth(1000n, 0n); - expect(await dashboard.settledGrowth()).to.equal(1000n); - await dashboard.connect(vaultOwner).grantRole(await dashboard.VOLUNTARY_DISCONNECT_ROLE(), vaultOwner); await expect(dashboard.voluntaryDisconnect()).to.emit(hub, "Mock__VaultDisconnectInitiated").withArgs(vault); - - // settled growth is reset - expect(await dashboard.settledGrowth()).to.equal(0n); }); }); @@ -1465,11 +1457,20 @@ describe("Dashboard.sol", () => { await setup({ isConnected: false }); const hubSigner = await impersonate(await hub.getAddress(), ether("1")); await vault.connect(hubSigner).transferOwnership(dashboard); + + // set settled growth + await dashboard.connect(vaultOwner).correctSettledGrowth(1000n, 0n); + await dashboard.connect(nodeOperator).correctSettledGrowth(1000n, 0n); + expect(await dashboard.settledGrowth()).to.equal(1000n); + await expect(dashboard.connect(vaultOwner).abandonDashboard(vaultOwner)) .to.emit(vault, "OwnershipTransferred") .withArgs(hub, dashboard) .and.to.emit(vault, "OwnershipTransferStarted") .withArgs(dashboard, vaultOwner); + + // settled growth is reset + expect(await dashboard.settledGrowth()).to.equal(0n); }); }); From bdeff5005273bae93fbc989475aa5261d568fdf5 Mon Sep 17 00:00:00 2001 From: failingtwice Date: Tue, 14 Oct 2025 13:16:33 +0500 Subject: [PATCH 3/9] feat: disburse fee circuit breaker --- .../vaults/dashboard/NodeOperatorFee.sol | 58 ++++++++++++++++++- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol index dd63f5d2d4..3c37771155 100644 --- a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol +++ b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol @@ -57,6 +57,12 @@ contract NodeOperatorFee is Permissions { */ bytes32 public constant NODE_OPERATOR_PROVE_UNKNOWN_VALIDATOR_ROLE = keccak256("vaults.NodeOperatorFee.ProveUnknownValidatorsRole"); + + /** + * @notice A sane maximum fee ratio to total value, in basis points (1 bp = 0.01%). + * Blocks fee disbursement if the fee exceeds this value, 1% of total value. + */ + uint256 constant internal MAX_SANE_RELATIVE_FEE_BP = 1_00; // ==================== Packed Storage Slot 1 ==================== /** @@ -71,6 +77,13 @@ contract NodeOperatorFee is Permissions { */ uint16 public feeRate; + /** + * @notice Indicates if fee disbursement is paused. + * When true, the `disburseFee` function will not transfer any fees. + * Triggered when the fee exceeds the sane maximum fee ratio to total value. + */ + bool public feeDisbursementPaused; + // ==================== Packed Storage Slot 2 ==================== /** * @notice Growth of the vault not subject to fees. @@ -160,7 +173,7 @@ contract NodeOperatorFee is Permissions { * @return fee The amount of ETH accrued as fee */ function accruedFee() public view returns (uint256 fee) { - (fee, ) = _calculateFee(); + (fee,, ) = _calculateFee(); } /** @@ -173,17 +186,39 @@ contract NodeOperatorFee is Permissions { * 4. Withdraws fee amount from vault to node operator recipient */ function disburseFee() public { - (uint256 fee, int128 growth) = _calculateFee(); + (uint256 fee, int128 growth, uint256 pauseThreshold) = _calculateFee(); // it's important not to revert here so as not to block disconnect if (fee == 0) return; + // pause fee disbursement if the fee is abnormally high + if (fee > pauseThreshold) { + feeDisbursementPaused = true; + emit FeeDisbursementPaused(); + return; + } + _setSettledGrowth(growth); VAULT_HUB.withdraw(address(_stakingVault()), feeRecipient, fee); emit FeeDisbursed(msg.sender, fee); } + /** + * @notice Resumes fee disbursement after being paused due to excessive fee ratio. + * Requires dual confirmation from admin and node operator manager. + * @return bool True if fee disbursement was resumed, false if still awaiting confirmations + */ + function resumeFeeDisbursement() external returns (bool) { + if (!feeDisbursementPaused) revert FeeDisbursementNotPaused(); + if (!_collectAndCheckConfirmations(msg.data, confirmingRoles())) return false; + + feeDisbursementPaused = false; + emit FeeDisbursementResumed(); + + return true; + } + /** * @notice Updates the node operator's fee rate with dual confirmation. * @param _newFeeRate The new fee rate in basis points (max 10000 = 100%) @@ -301,7 +336,7 @@ contract NodeOperatorFee is Permissions { _correctSettledGrowth(settledGrowth + int256(_amount)); } - function _calculateFee() internal view returns (uint256 fee, int128 growth) { + function _calculateFee() internal view returns (uint256 fee, int128 growth, uint256 pauseThreshold) { VaultHub.Report memory report = latestReport(); growth = int128(uint128(report.totalValue)) - int128(report.inOutDelta); int256 unsettledGrowth = growth - settledGrowth; @@ -309,6 +344,8 @@ contract NodeOperatorFee is Permissions { if (unsettledGrowth > 0) { fee = (uint256(unsettledGrowth) * feeRate) / TOTAL_BASIS_POINTS; } + + pauseThreshold = (report.totalValue * MAX_SANE_RELATIVE_FEE_BP) / TOTAL_BASIS_POINTS; } function _setFeeRate(uint256 _newFeeRate) internal { @@ -367,6 +404,16 @@ contract NodeOperatorFee is Permissions { */ event CorrectionTimestampUpdated(uint64 timestamp); + /** + * @dev Emitted when fee disbursement is paused due to abnormally high fee. + */ + event FeeDisbursementPaused(); + + /** + * @dev Emitted when fee disbursement is resumed. + */ + event FeeDisbursementResumed(); + // ==================== Errors ==================== /** @@ -374,6 +421,11 @@ contract NodeOperatorFee is Permissions { */ error FeeValueExceed100Percent(); + /** + * @dev Error emitted when trying to resume fee disbursement while it's not paused + */ + error FeeDisbursementNotPaused(); + /** * @dev Error emitted when trying to set same value for recipient */ From 0782a40e63b3da95c93b15d2881ca6325facbfac Mon Sep 17 00:00:00 2001 From: failingtwice Date: Tue, 14 Oct 2025 13:21:20 +0500 Subject: [PATCH 4/9] feat: revert disburse when paused --- contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol index 3c37771155..798ad451ac 100644 --- a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol +++ b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol @@ -186,6 +186,8 @@ contract NodeOperatorFee is Permissions { * 4. Withdraws fee amount from vault to node operator recipient */ function disburseFee() public { + if (feeDisbursementPaused) revert FeeDisbursementOnPause(); + (uint256 fee, int128 growth, uint256 pauseThreshold) = _calculateFee(); // it's important not to revert here so as not to block disconnect @@ -421,6 +423,11 @@ contract NodeOperatorFee is Permissions { */ error FeeValueExceed100Percent(); + /** + * @dev Error emitted when trying to disburse fee while it's paused + */ + error FeeDisbursementOnPause(); + /** * @dev Error emitted when trying to resume fee disbursement while it's not paused */ From d4ae689250b6471951232c59088877a0fa816f42 Mon Sep 17 00:00:00 2001 From: failingtwice Date: Tue, 14 Oct 2025 15:12:17 +0500 Subject: [PATCH 5/9] feat: special function for high fee disburse --- .../vaults/dashboard/NodeOperatorFee.sol | 79 ++++++----------- lib/constants.ts | 1 + .../nodeOperatorFee/nodeOperatorFee.test.ts | 85 ++++++++++++++++--- 3 files changed, 104 insertions(+), 61 deletions(-) diff --git a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol index 798ad451ac..b1157fda91 100644 --- a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol +++ b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol @@ -77,12 +77,6 @@ contract NodeOperatorFee is Permissions { */ uint16 public feeRate; - /** - * @notice Indicates if fee disbursement is paused. - * When true, the `disburseFee` function will not transfer any fees. - * Triggered when the fee exceeds the sane maximum fee ratio to total value. - */ - bool public feeDisbursementPaused; // ==================== Packed Storage Slot 2 ==================== /** @@ -177,7 +171,8 @@ contract NodeOperatorFee is Permissions { } /** - * @notice Permissionless function to disburse node operator fees. + * @notice Disburses node operator fees permissionlessly. + * Can be called by anyone as long as fee is not abnormally high. * * Fee disbursement steps: * 1. Calculate current vault growth from latest report @@ -186,37 +181,24 @@ contract NodeOperatorFee is Permissions { * 4. Withdraws fee amount from vault to node operator recipient */ function disburseFee() public { - if (feeDisbursementPaused) revert FeeDisbursementOnPause(); - - (uint256 fee, int128 growth, uint256 pauseThreshold) = _calculateFee(); - - // it's important not to revert here so as not to block disconnect - if (fee == 0) return; - - // pause fee disbursement if the fee is abnormally high - if (fee > pauseThreshold) { - feeDisbursementPaused = true; - emit FeeDisbursementPaused(); - return; - } + (uint256 fee, int128 growth, uint256 relativeThreshold) = _calculateFee(); + if (fee > relativeThreshold) revert AbnormallyHighFee(); - _setSettledGrowth(growth); - - VAULT_HUB.withdraw(address(_stakingVault()), feeRecipient, fee); - emit FeeDisbursed(msg.sender, fee); + _disburseFee(fee, growth); } /** - * @notice Resumes fee disbursement after being paused due to excessive fee ratio. - * Requires dual confirmation from admin and node operator manager. - * @return bool True if fee disbursement was resumed, false if still awaiting confirmations + * @notice Disburses an abnormally high fee with dual confirmation. + * Before calling this function, both parties must ensure that the high fee is expected, + * and the settled growth (used as baseline for fee) is set correctly. + * @return bool True if fee was disbursed, false if still awaiting confirmations */ - function resumeFeeDisbursement() external returns (bool) { - if (!feeDisbursementPaused) revert FeeDisbursementNotPaused(); + function disburseAbnormallyHighFee() external returns (bool) { if (!_collectAndCheckConfirmations(msg.data, confirmingRoles())) return false; - - feeDisbursementPaused = false; - emit FeeDisbursementResumed(); + + (uint256 fee, int128 growth,) = _calculateFee(); + + _disburseFee(fee, growth); return true; } @@ -305,6 +287,16 @@ contract NodeOperatorFee is Permissions { return LazyOracle(LIDO_LOCATOR.lazyOracle()); } + function _disburseFee(uint256 fee, int128 growth) internal { + // it's important not to revert here so as not to block disconnect + if (fee == 0) return; + + _setSettledGrowth(growth); + + VAULT_HUB.withdraw(address(_stakingVault()), feeRecipient, fee); + emit FeeDisbursed(msg.sender, fee); + } + function _setSettledGrowth(int256 _newSettledGrowth) internal { int128 oldSettledGrowth = settledGrowth; if (oldSettledGrowth == _newSettledGrowth) revert SameSettledGrowth(); @@ -338,7 +330,7 @@ contract NodeOperatorFee is Permissions { _correctSettledGrowth(settledGrowth + int256(_amount)); } - function _calculateFee() internal view returns (uint256 fee, int128 growth, uint256 pauseThreshold) { + function _calculateFee() internal view returns (uint256 fee, int128 growth, uint256 relativeThreshold) { VaultHub.Report memory report = latestReport(); growth = int128(uint128(report.totalValue)) - int128(report.inOutDelta); int256 unsettledGrowth = growth - settledGrowth; @@ -347,7 +339,7 @@ contract NodeOperatorFee is Permissions { fee = (uint256(unsettledGrowth) * feeRate) / TOTAL_BASIS_POINTS; } - pauseThreshold = (report.totalValue * MAX_SANE_RELATIVE_FEE_BP) / TOTAL_BASIS_POINTS; + relativeThreshold = (report.totalValue * MAX_SANE_RELATIVE_FEE_BP) / TOTAL_BASIS_POINTS; } function _setFeeRate(uint256 _newFeeRate) internal { @@ -406,16 +398,6 @@ contract NodeOperatorFee is Permissions { */ event CorrectionTimestampUpdated(uint64 timestamp); - /** - * @dev Emitted when fee disbursement is paused due to abnormally high fee. - */ - event FeeDisbursementPaused(); - - /** - * @dev Emitted when fee disbursement is resumed. - */ - event FeeDisbursementResumed(); - // ==================== Errors ==================== /** @@ -424,14 +406,9 @@ contract NodeOperatorFee is Permissions { error FeeValueExceed100Percent(); /** - * @dev Error emitted when trying to disburse fee while it's paused - */ - error FeeDisbursementOnPause(); - - /** - * @dev Error emitted when trying to resume fee disbursement while it's not paused + * @dev Error emitted when trying to disburse an abnormally high fee. */ - error FeeDisbursementNotPaused(); + error AbnormallyHighFee(); /** * @dev Error emitted when trying to set same value for recipient diff --git a/lib/constants.ts b/lib/constants.ts index f918a40477..c82102bbbc 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -59,6 +59,7 @@ export const EMPTY_SIGNATURE = "0x".padEnd(SIGNATURE_LENGTH_HEX + 2, "0"); export const ONE_GWEI = 1_000_000_000n; export const TOTAL_BASIS_POINTS = 100_00n; +export const MAX_SANE_RELATIVE_FEE_BP = 1_00n; export const MAX_FEE_BP = 65_535n; export const MAX_RESERVE_RATIO_BP = 99_99n; diff --git a/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts b/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts index b4f1f81123..ff43ceb4fc 100644 --- a/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts +++ b/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts @@ -16,7 +16,16 @@ import { WstETH__Harness, } from "typechain-types"; -import { advanceChainTime, days, ether, findEvents, getCurrentBlockTimestamp, getNextBlockTimestamp } from "lib"; +import { + advanceChainTime, + days, + ether, + findEvents, + getCurrentBlockTimestamp, + getNextBlockTimestamp, + MAX_SANE_RELATIVE_FEE_BP, + TOTAL_BASIS_POINTS, +} from "lib"; import { deployLidoLocator } from "test/deploy"; import { Snapshot } from "test/suite"; @@ -339,6 +348,61 @@ describe("NodeOperatorFee.sol", () => { expect(await nodeOperatorFee.accruedFee()).to.equal(0n); }); + + it("reverts if the fee is abnormally high", async () => { + const feeRate = await nodeOperatorFee.feeRate(); + const totalValue = ether("100"); + const pauseThreshold = (totalValue * MAX_SANE_RELATIVE_FEE_BP) / TOTAL_BASIS_POINTS; + const valueOverThreshold = 10n; + const rewards = (pauseThreshold * TOTAL_BASIS_POINTS) / feeRate + valueOverThreshold; + const inOutDelta = totalValue - rewards; + const expectedFee = (rewards * nodeOperatorFeeRate) / BP_BASE; + expect(expectedFee).to.be.greaterThan(((inOutDelta + rewards) * MAX_SANE_RELATIVE_FEE_BP) / TOTAL_BASIS_POINTS); + + await hub.setReport( + { + totalValue, + inOutDelta, + timestamp: await getCurrentBlockTimestamp(), + }, + true, + ); + + expect(await nodeOperatorFee.accruedFee()).to.equal(expectedFee); + await expect(nodeOperatorFee.disburseFee()).to.be.revertedWithCustomError(nodeOperatorFee, "AbnormallyHighFee"); + }); + + it("disburse abnormally high fee", async () => { + const feeRate = await nodeOperatorFee.feeRate(); + const totalValue = ether("100"); + const pauseThreshold = (totalValue * MAX_SANE_RELATIVE_FEE_BP) / TOTAL_BASIS_POINTS; + const valueOverThreshold = 10n; + const rewards = (pauseThreshold * TOTAL_BASIS_POINTS) / feeRate + valueOverThreshold; + const inOutDelta = totalValue - rewards; + const expectedFee = (rewards * nodeOperatorFeeRate) / BP_BASE; + expect(expectedFee).to.be.greaterThan(((inOutDelta + rewards) * MAX_SANE_RELATIVE_FEE_BP) / TOTAL_BASIS_POINTS); + + await hub.setReport( + { + totalValue, + inOutDelta, + timestamp: await getCurrentBlockTimestamp(), + }, + true, + ); + + expect(await nodeOperatorFee.accruedFee()).to.equal(expectedFee); + await expect(nodeOperatorFee.connect(vaultOwner).disburseAbnormallyHighFee()).to.emit( + nodeOperatorFee, + "RoleMemberConfirmed", + ); + expect(await nodeOperatorFee.accruedFee()).to.equal(expectedFee); + + await expect(nodeOperatorFee.connect(nodeOperatorManager).disburseAbnormallyHighFee()) + .to.emit(nodeOperatorFee, "RoleMemberConfirmed") + .and.to.emit(nodeOperatorFee, "FeeDisbursed"); + expect(await nodeOperatorFee.accruedFee()).to.equal(0n); + }); }); context("addFeeExemption", () => { @@ -421,23 +485,23 @@ describe("NodeOperatorFee.sol", () => { }); it("settledGrowth is updated fee claim", async () => { + const totalValue = ether("100"); const operatorFee = await nodeOperatorFee.feeRate(); - - const rewards = ether("10"); + const rewards = ether("0.01"); + const adjustment = ether("32"); // e.g. side deposit await hub.setReport( { - totalValue: rewards, - inOutDelta: 0n, + totalValue: totalValue + rewards + adjustment, + inOutDelta: totalValue, timestamp: await getCurrentBlockTimestamp(), }, true, ); - const expectedFee = (rewards * operatorFee) / BP_BASE; + const expectedFee = ((rewards + adjustment) * operatorFee) / BP_BASE; expect(await nodeOperatorFee.accruedFee()).to.equal(expectedFee); - const adjustment = rewards / 2n; const timestamp = await getNextBlockTimestamp(); await nodeOperatorFee.connect(nodeOperatorFeeExempter).addFeeExemption(adjustment); expect(await nodeOperatorFee.settledGrowth()).to.deep.equal(adjustment); @@ -698,12 +762,13 @@ describe("NodeOperatorFee.sol", () => { const noFeeRate = await nodeOperatorFee.feeRate(); - const rewards = ether("1"); + const totalValue = ether("100"); + const rewards = ether("0.1"); await hub.setReport( { - totalValue: rewards, - inOutDelta: 0n, + totalValue: totalValue + rewards, + inOutDelta: totalValue, timestamp: await getCurrentBlockTimestamp(), }, true, From 3155612ddd6d60ee08854527d85bd8d187090852 Mon Sep 17 00:00:00 2001 From: failingtwice Date: Tue, 14 Oct 2025 15:36:56 +0500 Subject: [PATCH 6/9] feat: 1% threshold rationale --- .../vaults/dashboard/NodeOperatorFee.sol | 21 +++++++++++++++---- lib/constants.ts | 2 +- .../nodeOperatorFee/nodeOperatorFee.test.ts | 14 ++++++++----- 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol index b1157fda91..aa4278870d 100644 --- a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol +++ b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol @@ -59,10 +59,23 @@ contract NodeOperatorFee is Permissions { keccak256("vaults.NodeOperatorFee.ProveUnknownValidatorsRole"); /** - * @notice A sane maximum fee ratio to total value, in basis points (1 bp = 0.01%). - * Blocks fee disbursement if the fee exceeds this value, 1% of total value. + * @notice If the accrued fee exceeds this BP of the total value, it is considered abnormally high. + * An abnormally high fee can only be disbursed with dual confirmation. + * This threshold is to prevent accidental overpayment due to outdated settled growth. + * + * Why 1% threshold? + * + * - Assume a very generous annual staking APR of ~5% (≈3% CL + 2% EL). + * - In the extreme case where the node operator fee rate is 100% of rewards + * (unrealistic, but technically possible), this translates to a 5% annual fee. + * - A 1% fee threshold would therefore be reached in ~73 days (365 / 5). + * - Meaning: as long as the operator disburses fees at least once every ~2 months, + * the threshold will never be hit. + * + * Since these assumptions are highly conservative, in practice the operator + * would need to disburse even less frequently before approaching the limit. */ - uint256 constant internal MAX_SANE_RELATIVE_FEE_BP = 1_00; + uint256 constant internal ABNORMALLY_HIGH_FEE_THRESHOLD_BP = 1_00; // ==================== Packed Storage Slot 1 ==================== /** @@ -339,7 +352,7 @@ contract NodeOperatorFee is Permissions { fee = (uint256(unsettledGrowth) * feeRate) / TOTAL_BASIS_POINTS; } - relativeThreshold = (report.totalValue * MAX_SANE_RELATIVE_FEE_BP) / TOTAL_BASIS_POINTS; + relativeThreshold = (report.totalValue * ABNORMALLY_HIGH_FEE_THRESHOLD_BP) / TOTAL_BASIS_POINTS; } function _setFeeRate(uint256 _newFeeRate) internal { diff --git a/lib/constants.ts b/lib/constants.ts index c82102bbbc..6f61b776b1 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -59,7 +59,7 @@ export const EMPTY_SIGNATURE = "0x".padEnd(SIGNATURE_LENGTH_HEX + 2, "0"); export const ONE_GWEI = 1_000_000_000n; export const TOTAL_BASIS_POINTS = 100_00n; -export const MAX_SANE_RELATIVE_FEE_BP = 1_00n; +export const ABNORMALLY_HIGH_FEE_THRESHOLD_BP = 1_00n; export const MAX_FEE_BP = 65_535n; export const MAX_RESERVE_RATIO_BP = 99_99n; diff --git a/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts b/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts index ff43ceb4fc..3ee01e3359 100644 --- a/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts +++ b/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts @@ -17,13 +17,13 @@ import { } from "typechain-types"; import { + ABNORMALLY_HIGH_FEE_THRESHOLD_BP, advanceChainTime, days, ether, findEvents, getCurrentBlockTimestamp, getNextBlockTimestamp, - MAX_SANE_RELATIVE_FEE_BP, TOTAL_BASIS_POINTS, } from "lib"; @@ -352,12 +352,14 @@ describe("NodeOperatorFee.sol", () => { it("reverts if the fee is abnormally high", async () => { const feeRate = await nodeOperatorFee.feeRate(); const totalValue = ether("100"); - const pauseThreshold = (totalValue * MAX_SANE_RELATIVE_FEE_BP) / TOTAL_BASIS_POINTS; + const pauseThreshold = (totalValue * ABNORMALLY_HIGH_FEE_THRESHOLD_BP) / TOTAL_BASIS_POINTS; const valueOverThreshold = 10n; const rewards = (pauseThreshold * TOTAL_BASIS_POINTS) / feeRate + valueOverThreshold; const inOutDelta = totalValue - rewards; const expectedFee = (rewards * nodeOperatorFeeRate) / BP_BASE; - expect(expectedFee).to.be.greaterThan(((inOutDelta + rewards) * MAX_SANE_RELATIVE_FEE_BP) / TOTAL_BASIS_POINTS); + expect(expectedFee).to.be.greaterThan( + ((inOutDelta + rewards) * ABNORMALLY_HIGH_FEE_THRESHOLD_BP) / TOTAL_BASIS_POINTS, + ); await hub.setReport( { @@ -375,12 +377,14 @@ describe("NodeOperatorFee.sol", () => { it("disburse abnormally high fee", async () => { const feeRate = await nodeOperatorFee.feeRate(); const totalValue = ether("100"); - const pauseThreshold = (totalValue * MAX_SANE_RELATIVE_FEE_BP) / TOTAL_BASIS_POINTS; + const pauseThreshold = (totalValue * ABNORMALLY_HIGH_FEE_THRESHOLD_BP) / TOTAL_BASIS_POINTS; const valueOverThreshold = 10n; const rewards = (pauseThreshold * TOTAL_BASIS_POINTS) / feeRate + valueOverThreshold; const inOutDelta = totalValue - rewards; const expectedFee = (rewards * nodeOperatorFeeRate) / BP_BASE; - expect(expectedFee).to.be.greaterThan(((inOutDelta + rewards) * MAX_SANE_RELATIVE_FEE_BP) / TOTAL_BASIS_POINTS); + expect(expectedFee).to.be.greaterThan( + ((inOutDelta + rewards) * ABNORMALLY_HIGH_FEE_THRESHOLD_BP) / TOTAL_BASIS_POINTS, + ); await hub.setReport( { From 0f5cfdf6bedf7af901ffb7591e5e7e74110d3dda Mon Sep 17 00:00:00 2001 From: failingtwice Date: Wed, 15 Oct 2025 13:23:01 +0500 Subject: [PATCH 7/9] feat: disburse abnormally high fee as admin --- .../vaults/dashboard/NodeOperatorFee.sol | 28 +++++++------------ .../nodeOperatorFee/nodeOperatorFee.test.ts | 7 +---- 2 files changed, 11 insertions(+), 24 deletions(-) diff --git a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol index aa4278870d..636d662665 100644 --- a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol +++ b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol @@ -60,20 +60,19 @@ contract NodeOperatorFee is Permissions { /** * @notice If the accrued fee exceeds this BP of the total value, it is considered abnormally high. - * An abnormally high fee can only be disbursed with dual confirmation. + * An abnormally high fee can only be disbursed by `DEFAULT_ADMIN_ROLE`. * This threshold is to prevent accidental overpayment due to outdated settled growth. * * Why 1% threshold? * - * - Assume a very generous annual staking APR of ~5% (≈3% CL + 2% EL). - * - In the extreme case where the node operator fee rate is 100% of rewards - * (unrealistic, but technically possible), this translates to a 5% annual fee. - * - A 1% fee threshold would therefore be reached in ~73 days (365 / 5). - * - Meaning: as long as the operator disburses fees at least once every ~2 months, + * - Assume a very generous annual staking APR of ~5% (3% CL + 2% EL). + * - A very high node operator fee rate of 10% translates to a 0.5% annual fee. + * - Thus, a 1% fee threshold would therefore be reached in 2 years. + * - Meaning: as long as the operator disburses fees at least once every 2 years, * the threshold will never be hit. * * Since these assumptions are highly conservative, in practice the operator - * would need to disburse even less frequently before approaching the limit. + * would need to disburse even less frequently before approaching the threshold. */ uint256 constant internal ABNORMALLY_HIGH_FEE_THRESHOLD_BP = 1_00; @@ -90,7 +89,6 @@ contract NodeOperatorFee is Permissions { */ uint16 public feeRate; - // ==================== Packed Storage Slot 2 ==================== /** * @notice Growth of the vault not subject to fees. @@ -201,19 +199,13 @@ contract NodeOperatorFee is Permissions { } /** - * @notice Disburses an abnormally high fee with dual confirmation. - * Before calling this function, both parties must ensure that the high fee is expected, + * @notice Disburses an abnormally high fee as `DEFAULT_ADMIN_ROLE`. + * Before calling this function, the caller must ensure that the high fee is expected, * and the settled growth (used as baseline for fee) is set correctly. - * @return bool True if fee was disbursed, false if still awaiting confirmations */ - function disburseAbnormallyHighFee() external returns (bool) { - if (!_collectAndCheckConfirmations(msg.data, confirmingRoles())) return false; - + function disburseAbnormallyHighFee() external onlyRoleMemberOrAdmin(DEFAULT_ADMIN_ROLE) { (uint256 fee, int128 growth,) = _calculateFee(); - - _disburseFee(fee, growth); - - return true; + _disburseFee(fee, growth); } /** diff --git a/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts b/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts index 3ee01e3359..6cb02deac7 100644 --- a/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts +++ b/test/0.8.25/vaults/nodeOperatorFee/nodeOperatorFee.test.ts @@ -398,13 +398,8 @@ describe("NodeOperatorFee.sol", () => { expect(await nodeOperatorFee.accruedFee()).to.equal(expectedFee); await expect(nodeOperatorFee.connect(vaultOwner).disburseAbnormallyHighFee()).to.emit( nodeOperatorFee, - "RoleMemberConfirmed", + "FeeDisbursed", ); - expect(await nodeOperatorFee.accruedFee()).to.equal(expectedFee); - - await expect(nodeOperatorFee.connect(nodeOperatorManager).disburseAbnormallyHighFee()) - .to.emit(nodeOperatorFee, "RoleMemberConfirmed") - .and.to.emit(nodeOperatorFee, "FeeDisbursed"); expect(await nodeOperatorFee.accruedFee()).to.equal(0n); }); }); From ec3b2fa30e494168b46c58d7d330844aacbdf70f Mon Sep 17 00:00:00 2001 From: failingtwice Date: Wed, 15 Oct 2025 13:24:30 +0500 Subject: [PATCH 8/9] feat: rename high fee threshold --- contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol index 636d662665..c0ade2893b 100644 --- a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol +++ b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol @@ -192,8 +192,8 @@ contract NodeOperatorFee is Permissions { * 4. Withdraws fee amount from vault to node operator recipient */ function disburseFee() public { - (uint256 fee, int128 growth, uint256 relativeThreshold) = _calculateFee(); - if (fee > relativeThreshold) revert AbnormallyHighFee(); + (uint256 fee, int128 growth, uint256 abnormallyHighFeeThreshold) = _calculateFee(); + if (fee > abnormallyHighFeeThreshold) revert AbnormallyHighFee(); _disburseFee(fee, growth); } @@ -335,7 +335,7 @@ contract NodeOperatorFee is Permissions { _correctSettledGrowth(settledGrowth + int256(_amount)); } - function _calculateFee() internal view returns (uint256 fee, int128 growth, uint256 relativeThreshold) { + function _calculateFee() internal view returns (uint256 fee, int128 growth, uint256 abnormallyHighFeeThreshold) { VaultHub.Report memory report = latestReport(); growth = int128(uint128(report.totalValue)) - int128(report.inOutDelta); int256 unsettledGrowth = growth - settledGrowth; @@ -344,7 +344,7 @@ contract NodeOperatorFee is Permissions { fee = (uint256(unsettledGrowth) * feeRate) / TOTAL_BASIS_POINTS; } - relativeThreshold = (report.totalValue * ABNORMALLY_HIGH_FEE_THRESHOLD_BP) / TOTAL_BASIS_POINTS; + abnormallyHighFeeThreshold = (report.totalValue * ABNORMALLY_HIGH_FEE_THRESHOLD_BP) / TOTAL_BASIS_POINTS; } function _setFeeRate(uint256 _newFeeRate) internal { From 1a2f158cbbbd60a5df494432357ad1ae13d72a1e Mon Sep 17 00:00:00 2001 From: failingtwice Date: Wed, 15 Oct 2025 14:01:57 +0500 Subject: [PATCH 9/9] feat: pass settled growth to connect to avoid foot violence --- contracts/0.8.25/vaults/VaultFactory.sol | 2 +- contracts/0.8.25/vaults/dashboard/Dashboard.sol | 16 +++++++++++----- .../0.8.25/vaults/dashboard/NodeOperatorFee.sol | 5 +++++ test/0.8.25/vaults/dashboard/dashboard.test.ts | 15 +++++++++------ .../vaults/disconnected.integration.ts | 4 ++-- .../vaults/operator.grid.integration.ts | 2 +- .../scenario/lazyOracle.report.integration.ts | 2 +- 7 files changed, 30 insertions(+), 16 deletions(-) diff --git a/contracts/0.8.25/vaults/VaultFactory.sol b/contracts/0.8.25/vaults/VaultFactory.sol index 23be39db56..d786cd7837 100644 --- a/contracts/0.8.25/vaults/VaultFactory.sol +++ b/contracts/0.8.25/vaults/VaultFactory.sol @@ -81,7 +81,7 @@ contract VaultFactory { // initialize Dashboard with the factory address as the default admin, grant optional roles and connect to VaultHub dashboard.initialize(address(this), _nodeOperatorManager, _nodeOperatorManager, _nodeOperatorFeeBP, _confirmExpiry); - dashboard.connectToVaultHub{value: msg.value}(); + dashboard.connectToVaultHub{value: msg.value}(0); // _roleAssignments can only include DEFAULT_ADMIN_ROLE's subroles, // which is why it's important to revoke the NODE_OPERATOR_MANAGER_ROLE BEFORE granting roles diff --git a/contracts/0.8.25/vaults/dashboard/Dashboard.sol b/contracts/0.8.25/vaults/dashboard/Dashboard.sol index 8d704f688f..a585ffe059 100644 --- a/contracts/0.8.25/vaults/dashboard/Dashboard.sol +++ b/contracts/0.8.25/vaults/dashboard/Dashboard.sol @@ -276,16 +276,21 @@ contract Dashboard is NodeOperatorFee { /** * @notice Accepts the ownership over the StakingVault and connects to VaultHub. Can be called to reconnect * to the hub after voluntaryDisconnect() + * @param _currentSettledGrowth The current settled growth value to verify against the stored one */ - function reconnectToVaultHub() external { + function reconnectToVaultHub(uint256 _currentSettledGrowth) external { _acceptOwnership(); - connectToVaultHub(); + connectToVaultHub(_currentSettledGrowth); } /** * @notice Connects to VaultHub, transferring ownership to VaultHub. + * @param _currentSettledGrowth The current settled growth value to verify against the stored one */ - function connectToVaultHub() public payable { + function connectToVaultHub(uint256 _currentSettledGrowth) public payable { + if (settledGrowth != int256(_currentSettledGrowth)) { + revert SettledGrowthMismatch(); + } if (msg.value > 0) _stakingVault().fund{value: msg.value}(); _transferOwnership(address(VAULT_HUB)); VAULT_HUB.connectVault(address(_stakingVault())); @@ -295,9 +300,10 @@ contract Dashboard is NodeOperatorFee { * @notice Changes the tier of the vault and connects to VaultHub * @param _tierId The tier to change to * @param _requestedShareLimit The requested share limit + * @param _currentSettledGrowth The current settled growth value to verify against the stored one */ - function connectAndAcceptTier(uint256 _tierId, uint256 _requestedShareLimit) external payable { - connectToVaultHub(); + function connectAndAcceptTier(uint256 _tierId, uint256 _requestedShareLimit, uint256 _currentSettledGrowth) external payable { + connectToVaultHub(_currentSettledGrowth); if (!_changeTier(_tierId, _requestedShareLimit)) { revert TierChangeNotConfirmed(); } diff --git a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol index c0ade2893b..3ef9434261 100644 --- a/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol +++ b/contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol @@ -425,6 +425,11 @@ contract NodeOperatorFee is Permissions { */ error SameSettledGrowth(); + /** + * @dev Error emitted when the settled growth does not match the expected value during connection. + */ + error SettledGrowthMismatch(); + /** * @dev Error emitted when the report is stale. */ diff --git a/test/0.8.25/vaults/dashboard/dashboard.test.ts b/test/0.8.25/vaults/dashboard/dashboard.test.ts index 4017e6d65b..542097c12d 100644 --- a/test/0.8.25/vaults/dashboard/dashboard.test.ts +++ b/test/0.8.25/vaults/dashboard/dashboard.test.ts @@ -625,14 +625,14 @@ describe("Dashboard.sol", () => { }); it("reverts if called by a non-admin", async () => { - await expect(newDashboard.connect(stranger).connectAndAcceptTier(1, 1n)).to.be.revertedWithCustomError( + await expect(newDashboard.connect(stranger).connectAndAcceptTier(1, 1n, 0n)).to.be.revertedWithCustomError( newDashboard, "AccessControlUnauthorizedAccount", ); }); it("reverts if change tier is not confirmed by node operator", async () => { - await expect(newDashboard.connect(vaultOwner).connectAndAcceptTier(1, 1n)).to.be.revertedWithCustomError( + await expect(newDashboard.connect(vaultOwner).connectAndAcceptTier(1, 1n, 0n)).to.be.revertedWithCustomError( newDashboard, "TierChangeNotConfirmed", ); @@ -640,14 +640,17 @@ describe("Dashboard.sol", () => { it("works", async () => { await operatorGrid.connect(nodeOperator).changeTier(newVault, 1, 1n); - await expect(newDashboard.connect(vaultOwner).connectAndAcceptTier(1, 1n)).to.emit(hub, "Mock__VaultConnected"); + await expect(newDashboard.connect(vaultOwner).connectAndAcceptTier(1, 1n, 0n)).to.emit( + hub, + "Mock__VaultConnected", + ); }); it("works with connection deposit", async () => { const connectDeposit = await hub.CONNECT_DEPOSIT(); await operatorGrid.connect(nodeOperator).changeTier(newVault, 1, 1n); - await expect(newDashboard.connect(vaultOwner).connectAndAcceptTier(1, 1n, { value: connectDeposit })) + await expect(newDashboard.connect(vaultOwner).connectAndAcceptTier(1, 1n, 0n, { value: connectDeposit })) .to.emit(hub, "Mock__VaultConnected") .withArgs(newVault); }); @@ -1477,7 +1480,7 @@ describe("Dashboard.sol", () => { context("reconnectToVaultHub", () => { it("reverts if called by a non-admin", async () => { await setup({ isConnected: false }); - await expect(dashboard.connect(stranger).reconnectToVaultHub()).to.be.revertedWithCustomError( + await expect(dashboard.connect(stranger).reconnectToVaultHub(0n)).to.be.revertedWithCustomError( dashboard, "AccessControlUnauthorizedAccount", ); @@ -1494,7 +1497,7 @@ describe("Dashboard.sol", () => { // reconnect await vault.connect(vaultOwner).transferOwnership(dashboard); - await dashboard.reconnectToVaultHub(); + await dashboard.reconnectToVaultHub(0n); expect(await vault.owner()).to.equal(hub); }); }); diff --git a/test/integration/vaults/disconnected.integration.ts b/test/integration/vaults/disconnected.integration.ts index 059bbd8637..65dd90f49b 100644 --- a/test/integration/vaults/disconnected.integration.ts +++ b/test/integration/vaults/disconnected.integration.ts @@ -93,7 +93,7 @@ describe("Integration: Actions with vault disconnected from hub", () => { it("Can reconnect the vault to the hub", async () => { const { vaultHub } = ctx.contracts; - await dashboard.reconnectToVaultHub(); + await dashboard.reconnectToVaultHub(0n); expect(await vaultHub.isVaultConnected(stakingVault)).to.equal(true); }); @@ -137,7 +137,7 @@ describe("Integration: Actions with vault disconnected from hub", () => { const { vaultHub } = ctx.contracts; - await expect(dashboard.reconnectToVaultHub()) + await expect(dashboard.reconnectToVaultHub(0n)) .to.emit(stakingVault, "OwnershipTransferred") .withArgs(owner, dashboard) .to.emit(stakingVault, "OwnershipTransferStarted") diff --git a/test/integration/vaults/operator.grid.integration.ts b/test/integration/vaults/operator.grid.integration.ts index 79e4ddc529..72d491334b 100644 --- a/test/integration/vaults/operator.grid.integration.ts +++ b/test/integration/vaults/operator.grid.integration.ts @@ -381,7 +381,7 @@ describe("Integration: OperatorGrid", () => { expect(await vaultHub.isVaultConnected(stakingVault)).to.be.false; // Reconnect vault - await dashboard.connect(owner).reconnectToVaultHub(); + await dashboard.connect(owner).reconnectToVaultHub(0n); // Verify vault is reconnected expect(await vaultHub.isVaultConnected(stakingVault)).to.be.true; diff --git a/test/integration/vaults/scenario/lazyOracle.report.integration.ts b/test/integration/vaults/scenario/lazyOracle.report.integration.ts index 14c66d87f2..be417d926e 100644 --- a/test/integration/vaults/scenario/lazyOracle.report.integration.ts +++ b/test/integration/vaults/scenario/lazyOracle.report.integration.ts @@ -55,7 +55,7 @@ describe("Scenario: Lazy Oracle prevents overwriting freshly reconnected vault r expect(await lazyOracle.latestReportTimestamp()).to.be.greaterThan(0); expect(await vaultHub.isVaultConnected(stakingVault)).to.be.false; - await dashboard.connect(owner).reconnectToVaultHub(); + await dashboard.connect(owner).reconnectToVaultHub(0n); await expect( reportVaultDataWithProof(ctx, stakingVault, { updateReportData: false }),