fix: some more test-cases on batch key removal

lidofinance · Oct 10, 2021 · 6d999ea · 6d999ea
1 parent 896ba0b
commit 6d999ea
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 4 deletions.
diff --git a/contracts/0.4.24/nos/NodeOperatorsRegistry.sol b/contracts/0.4.24/nos/NodeOperatorsRegistry.sol
@@ -274,8 +274,9 @@ contract NodeOperatorsRegistry is INodeOperatorsRegistry, IsContract, AragonApp
         external
         authP(MANAGE_SIGNING_KEYS, arr(_operator_id))
     {
-        for (uint256 i = 1; i <= _amount ; ++i) {
-            _removeSigningKey(_operator_id, _index+_amount-i);
+        // removing from the last index to the highest one, so we won't get outside the array
+        for (uint256 i = _index + _amount; i > _index ; --i) {
+            _removeSigningKey(_operator_id, i - 1);
         }
     }
 
@@ -297,8 +298,9 @@ contract NodeOperatorsRegistry is INodeOperatorsRegistry, IsContract, AragonApp
       */
     function removeSigningKeysOperatorBH(uint256 _operator_id, uint256 _index, uint256 _amount) external {
         require(msg.sender == operators[_operator_id].rewardAddress, "APP_AUTH_FAILED");
-        for (uint256 i = 1; i <= _amount ; ++i) {
-            _removeSigningKey(_operator_id, _index+_amount-i);
+        // removing from the last index to the highest one, so we won't get outside the array
+        for (uint256 i = _index + _amount; i > _index ; --i) {
+            _removeSigningKey(_operator_id, i - 1);
         }
     }
 

diff --git a/test/0.4.24/node-operators-registry.test.js b/test/0.4.24/node-operators-registry.test.js
@@ -680,6 +680,7 @@ contract('NodeOperatorsRegistry', ([appManager, voting, user1, user2, user3, nob
 
     await assertRevert(app.removeSigningKeysOperatorBH(0, 4, 1, { from: voting }), 'APP_AUTH_FAILED')
     await assertRevert(app.removeSigningKeysOperatorBH(0, 4, 1, { from: user1 }), 'KEY_NOT_FOUND')
+    await assertRevert(app.removeSigningKeysOperatorBH(0, 0, 5, { from: user1 }), 'KEY_NOT_FOUND')
 
     assertBn(await app.getTotalSigningKeyCount(0, { from: nobody }), 4)
     assertBn(await app.getUnusedSigningKeyCount(0, { from: nobody }), 4)
@@ -689,6 +690,10 @@ contract('NodeOperatorsRegistry', ([appManager, voting, user1, user2, user3, nob
 
     assert.equal((await app.getSigningKey(0, 0, { from: nobody })).key, op0.keys[7])
     assert.equal((await app.getSigningKey(0, 1, { from: nobody })).key, op0.keys[5])
+
+    await app.removeSigningKeysOperatorBH(0, 0, 2, { from: user1 })
+    assertBn(await app.getTotalSigningKeyCount(0, { from: nobody }), 0)
+    assertBn(await app.getUnusedSigningKeyCount(0, { from: nobody }), 0)
   })
 
   it('getRewardsDistribution works', async () => {