Friendly revert when validators decreased #179

Fixes unobvious revert reason with `MATH_SUB_UNDERFLOW` when oracle pushes decreased number of beacon validators.
lidofinance · Nov 26, 2020 · 842956b · 842956b
1 parent 9db7845
commit 842956b
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 2 deletions.
diff --git a/contracts/0.4.24/Lido.sol b/contracts/0.4.24/Lido.sol
@@ -239,7 +239,13 @@ contract Lido is ILido, IsContract, Pausable, AragonApp {
         uint256 depositedValidators = DEPOSITED_VALIDATORS_VALUE_POSITION.getStorageUint256();
         require(_beaconValidators <= depositedValidators, "REPORTED_MORE_DEPOSITED");
 
-        uint256 appearedValidators = _beaconValidators.sub(BEACON_VALIDATORS_VALUE_POSITION.getStorageUint256());
+        uint256 beaconValidators = BEACON_VALIDATORS_VALUE_POSITION.getStorageUint256();
+        // Since the calculation of funds in the ingress queue is based on the number of validators 
+        // that are in a transient state (deposited but not seen on beacon yet), we can't decrease the previously
+        // reported number (we'll be unable to figure out who is in the queue and count them).
+        // See LIP-1 for details https://github.com/lidofinance/lido-improvement-proposals/blob/develop/LIPS/lip-1.md
+        require(_beaconValidators >= beaconValidators, "REPORTED_LESS_VALIDATORS");
+        uint256 appearedValidators = _beaconValidators.sub(beaconValidators);
 
         // RewardBase is the amount of money that is not included in the reward calculation
         // Just appeared validators * 32 added to the previously reported beacon balance

diff --git a/test/0.4.24/lidoPushBeacon.test.js b/test/0.4.24/lidoPushBeacon.test.js
@@ -204,7 +204,7 @@ contract('Lido pushBeacon', ([appManager, voting, user1, user2, user3, nobody])
       assertBn(await app.totalRewards(), ETH(1))
     })
 
-    it('report BcnValidators:3 = revert', async () => {
+    it('report BcnValidators:3 = revert with REPORTED_MORE_DEPOSITED', async () => {
       await assertRevert(oracle.reportBeacon(110, 3, ETH(65), { from: user2 }), 'REPORTED_MORE_DEPOSITED')
       checkStat({ depositedValidators: 2, beaconValidators: 1, beaconBalance: ETH(30) })
       assertBn(await app.getBufferedEther(), ETH(5))
@@ -213,4 +213,27 @@ contract('Lido pushBeacon', ([appManager, voting, user1, user2, user3, nobody])
       assertBn(await app.totalRewards(), 0)
     })
   })
+
+  context('with depositedVals=5, beaconVals=4, bcnBal=1, bufferedEth=0', async () => {
+    beforeEach(async function () {
+      await app.setDepositedValidators(5)
+      await app.setBeaconBalance(ETH(1))
+      await app.setBufferedEther({ from: user1, value: ETH(0) })
+      await app.setBeaconValidators(4)
+    })
+
+    // See LIP-1 for explanation
+    // https://github.com/lidofinance/lido-improvement-proposals/blob/develop/LIPS/lip-1.md
+    it('report decreased BcnValidators:3 = revert with REPORTED_LESS_VALIDATORS', async () => {
+      await assertRevert(oracle.reportBeacon(123, 3, ETH(1), { from: user2 }), 'REPORTED_LESS_VALIDATORS')
+      await assertRevert(oracle.reportBeacon(321, 2, ETH(10), { from: user2 }), 'REPORTED_LESS_VALIDATORS')
+      await assertRevert(oracle.reportBeacon(12345, 1, ETH(123), { from: user2 }), 'REPORTED_LESS_VALIDATORS')
+      // values stay intact
+      checkStat({ depositedValidators: 5, beaconValidators: 4, beaconBalance: ETH(1) })
+      assertBn(await app.getBufferedEther(), ETH(0))
+      assertBn(await app.getTotalPooledEther(), ETH(33))
+      assert.equal(await app.distributeRewardsCalled(), false)
+      assertBn(await app.totalRewards(), 0)
+    })
+  })
 })