Fix network withdrawals (#262)

* fix sequential network withdrawals issue
ssvlabs · Sep 29, 2023 · a0c2914 · a0c2914
1 parent b8e03e7
commit a0c2914
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1,13 @@
+# Changelog
+
+All notable changes to SSV Network contracts will be documented in this file.
+
+This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+## [Unreleased]
+
+## [v1.0.1.rc4] - 2023-09-19
+
+### Fixed
+
+- [22d2859](https://github.com/bloxapp/ssv-network/pull/262/commits/22d2859d8fe6267b09c7a1c9c645df19bdaa03ff) Fix bug in network earnings withdrawals.
diff --git a/contracts/modules/SSVDAO.sol b/contracts/modules/SSVDAO.sol
@@ -34,6 +34,7 @@ contract SSVDAO is ISSVDAO {
         }
 
         sp.daoBalance = networkBalance - shrunkAmount;
+        sp.daoIndexBlockNumber = uint32(block.number);
 
         CoreLib.transferBalance(msg.sender, amount);
 

diff --git a/test/account/withdraw.ts b/test/account/withdraw.ts
@@ -5,13 +5,15 @@ import { expect } from 'chai';
 import { trackGas, GasGroup } from '../helpers/gas-usage';
 
 // Declare globals
-let ssvNetworkContract: any, ssvToken: any, cluster1: any, minDepositAmount: any;
+let ssvNetworkContract: any, ssvViews: any, ssvToken: any, cluster1: any, minDepositAmount: any;
 
 describe('Withdraw Tests', () => {
   beforeEach(async () => {
     // Initialize contract
     const metadata = (await helpers.initializeContract());
     ssvNetworkContract = metadata.contract;
+    ssvNetworkContract = metadata.contract;
+    ssvViews = metadata.ssvViews;
     ssvToken = metadata.ssvToken;
 
     // Register operators
@@ -48,19 +50,19 @@ describe('Withdraw Tests', () => {
   });
 
   it('Withdraw from operator balance emits "OperatorWithdrawn"', async () => {
-    await expect(ssvNetworkContract.connect(helpers.DB.owners[0])['withdrawOperatorEarnings(uint64,uint256)'](1, helpers.CONFIG.minimalOperatorFee)).to.emit(ssvNetworkContract, 'OperatorWithdrawn');
+    await expect(ssvNetworkContract.connect(helpers.DB.owners[0]).withdrawOperatorEarnings(1, helpers.CONFIG.minimalOperatorFee)).to.emit(ssvNetworkContract, 'OperatorWithdrawn');
   });
 
   it('Withdraw from operator balance gas limits', async () => {
-    await trackGas(ssvNetworkContract.connect(helpers.DB.owners[0])['withdrawOperatorEarnings(uint64,uint256)'](1, helpers.CONFIG.minimalOperatorFee), [GasGroup.WITHDRAW_OPERATOR_BALANCE]);
+    await trackGas(ssvNetworkContract.connect(helpers.DB.owners[0]).withdrawOperatorEarnings(1, helpers.CONFIG.minimalOperatorFee), [GasGroup.WITHDRAW_OPERATOR_BALANCE]);
   });
 
   it('Withdraw the total operator balance emits "OperatorWithdrawn"', async () => {
-    await expect(ssvNetworkContract.connect(helpers.DB.owners[0])['withdrawAllOperatorEarnings(uint64)'](1)).to.emit(ssvNetworkContract, 'OperatorWithdrawn');
+    await expect(ssvNetworkContract.connect(helpers.DB.owners[0]).withdrawAllOperatorEarnings(1)).to.emit(ssvNetworkContract, 'OperatorWithdrawn');
   });
 
   it('Withdraw the total operator balance gas limits', async () => {
-    await trackGas(ssvNetworkContract.connect(helpers.DB.owners[0])['withdrawAllOperatorEarnings(uint64)'](1), [GasGroup.WITHDRAW_OPERATOR_BALANCE]);
+    await trackGas(ssvNetworkContract.connect(helpers.DB.owners[0]).withdrawAllOperatorEarnings(1), [GasGroup.WITHDRAW_OPERATOR_BALANCE]);
   });
 
   it('Withdraw from a cluster that has a removed operator emits "ClusterWithdrawn"', async () => {
@@ -72,6 +74,21 @@ describe('Withdraw Tests', () => {
     await expect(ssvNetworkContract.connect(helpers.DB.owners[4]).withdraw(cluster1.operatorIds, minDepositAmount, cluster1.cluster)).to.be.revertedWithCustomError(ssvNetworkContract, 'InsufficientBalance');
   });
 
+  it('Sequentially withdraw more than the cluster balance reverts "InsufficientBalance"', async () => {
+    const burnPerBlock = helpers.CONFIG.minimalOperatorFee * 4;
+
+    cluster1 = await helpers.deposit(1, cluster1.owner, cluster1.operatorIds, (minDepositAmount * 2).toString(), cluster1.cluster);
+    expect(await ssvViews.getBalance(helpers.DB.owners[4].address, cluster1.operatorIds, cluster1.cluster)).to.be.equals(minDepositAmount * 3 - (burnPerBlock * 2));
+
+    cluster1 = await helpers.withdraw(4, cluster1.operatorIds, minDepositAmount, cluster1.cluster);
+    expect(await ssvViews.getBalance(helpers.DB.owners[4].address, cluster1.operatorIds, cluster1.cluster)).to.be.equals(minDepositAmount * 2 - (burnPerBlock * 3));
+
+    cluster1 = await helpers.withdraw(4, cluster1.operatorIds, minDepositAmount, cluster1.cluster);
+    expect(await ssvViews.getBalance(helpers.DB.owners[4].address, cluster1.operatorIds, cluster1.cluster)).to.be.equals(minDepositAmount - (burnPerBlock * 4));
+
+    await expect(ssvNetworkContract.connect(helpers.DB.owners[4]).withdraw(cluster1.operatorIds, minDepositAmount, cluster1.cluster)).to.be.revertedWithCustomError(ssvNetworkContract, 'InsufficientBalance');
+  });
+
   it('Withdraw from a liquidatable cluster reverts "InsufficientBalance" (liquidation threshold)', async () => {
     await utils.progressBlocks(20);
     await expect(ssvNetworkContract.connect(helpers.DB.owners[4]).withdraw(cluster1.operatorIds, 4000000000, cluster1.cluster)).to.be.revertedWithCustomError(ssvNetworkContract, 'InsufficientBalance');
@@ -88,20 +105,31 @@ describe('Withdraw Tests', () => {
   });
 
   it('Withdraw balance from an operator I do not own reverts "CallerNotOwner"', async () => {
-    await expect(ssvNetworkContract.connect(helpers.DB.owners[2])['withdrawOperatorEarnings(uint64,uint256)'](1, minDepositAmount)).to.be.revertedWithCustomError(ssvNetworkContract, 'CallerNotOwner');
+    await expect(ssvNetworkContract.connect(helpers.DB.owners[2]).withdrawOperatorEarnings(1, minDepositAmount)).to.be.revertedWithCustomError(ssvNetworkContract, 'CallerNotOwner');
   });
 
   it('Withdraw more than the operator balance reverts "InsufficientBalance"', async () => {
-    await expect(ssvNetworkContract.connect(helpers.DB.owners[0])['withdrawOperatorEarnings(uint64,uint256)'](1, minDepositAmount
+    await expect(ssvNetworkContract.connect(helpers.DB.owners[0]).withdrawOperatorEarnings(1, minDepositAmount
+    )).to.be.revertedWithCustomError(ssvNetworkContract, 'InsufficientBalance');
+  });
+
+  it('Sequentially withdraw more than the operator balance reverts "InsufficientBalance"', async () => {
+    await ssvNetworkContract.connect(helpers.DB.owners[0]).withdrawOperatorEarnings(1, helpers.CONFIG.minimalOperatorFee * 3);
+    expect(await ssvViews.getOperatorEarnings(1)).to.be.equals(helpers.CONFIG.minimalOperatorFee * 4 - helpers.CONFIG.minimalOperatorFee * 3);
+
+    await ssvNetworkContract.connect(helpers.DB.owners[0]).withdrawOperatorEarnings(1, helpers.CONFIG.minimalOperatorFee * 3);
+    expect(await ssvViews.getOperatorEarnings(1)).to.be.equals(helpers.CONFIG.minimalOperatorFee * 6 - helpers.CONFIG.minimalOperatorFee * 6);
+
+    await expect(ssvNetworkContract.connect(helpers.DB.owners[0]).withdrawOperatorEarnings(1, helpers.CONFIG.minimalOperatorFee * 3
     )).to.be.revertedWithCustomError(ssvNetworkContract, 'InsufficientBalance');
   });
 
   it('Withdraw the total balance from an operator I do not own reverts "CallerNotOwner"', async () => {
-    await expect(ssvNetworkContract.connect(helpers.DB.owners[2])['withdrawAllOperatorEarnings(uint64)'](12)).to.be.revertedWithCustomError(ssvNetworkContract, 'CallerNotOwner');
+    await expect(ssvNetworkContract.connect(helpers.DB.owners[2]).withdrawAllOperatorEarnings(12)).to.be.revertedWithCustomError(ssvNetworkContract, 'CallerNotOwner');
   });
 
   it('Withdraw more than the operator total balance reverts "InsufficientBalance"', async () => {
-    await expect(ssvNetworkContract.connect(helpers.DB.owners[0])['withdrawAllOperatorEarnings(uint64)'](12)).to.be.revertedWithCustomError(ssvNetworkContract, 'InsufficientBalance');
+    await expect(ssvNetworkContract.connect(helpers.DB.owners[0]).withdrawAllOperatorEarnings(12)).to.be.revertedWithCustomError(ssvNetworkContract, 'InsufficientBalance');
   });
 
   it('Withdraw from a cluster without validators', async () => {

diff --git a/test/dao/network-fee-withdraw.ts b/test/dao/network-fee-withdraw.ts
@@ -33,9 +33,6 @@ describe('DAO Network Fee Withdraw Tests', () => {
     await helpers.registerValidators(4, 1, minDepositAmount, helpers.DataGenerator.cluster.new(), [GasGroup.REGISTER_VALIDATOR_NEW_STATE]);
     await utils.progressBlocks(10);
 
-    // Temporary till deposit logic not available
-    // Mint tokens
-    await helpers.DB.ssvToken.mint(ssvNetworkContract.address, minDepositAmount);
   });
 
   it('Withdraw network earnings emits "NetworkEarningsWithdrawn"', async () => {
@@ -68,4 +65,17 @@ describe('DAO Network Fee Withdraw Tests', () => {
     await expect(ssvNetworkContract.connect(helpers.DB.owners[3]).withdrawNetworkEarnings(amount
     )).to.be.revertedWith('Ownable: caller is not the owner');
   });
+
+  it('Withdraw network earnings sequentially when not enough balance reverts "InsufficientBalance"', async () => {
+    const amount = await ssvViews.getNetworkEarnings() / 2;
+
+    await ssvNetworkContract.withdrawNetworkEarnings(amount);
+    expect(await ssvViews.getNetworkEarnings()).to.be.equals(((networkFee * 13) + (networkFee * 11) - amount));
+
+    await ssvNetworkContract.withdrawNetworkEarnings(amount);
+    expect(await ssvViews.getNetworkEarnings()).to.be.equals(((networkFee * 14) + (networkFee * 12) - amount * 2));
+
+    await expect(ssvNetworkContract.withdrawNetworkEarnings(amount
+    )).to.be.revertedWithCustomError(ssvNetworkContract, 'InsufficientBalance');
+  });
 });