fix: tests, invalid keys improvemnt

lidofinance · Dec 27, 2023 · bbdc745 · bbdc745
1 parent d8e7dfb
commit bbdc745
Show file tree

Hide file tree

Showing 10 changed files with 467 additions and 241 deletions.
diff --git a/src/guardian/guardian.service.spec.ts b/src/guardian/guardian.service.spec.ts
@@ -24,96 +24,6 @@ import { mockRepository } from 'contracts/repository/repository.mock';
 
 jest.mock('../transport/stomp/stomp.client');
 
-const vettedKeys = [
-  {
-    key: '0x9948d2becf42e9f76922bc6f664545e6f50401050af95785a984802d32a95c4c61f8e3de312b78167f86e047f83a7796',
-    depositSignature:
-      '0x8bf4401a354de243a3716ee2efc0bde1ded56a40e2943ac7c50290bec37e935d6170b21e7c0872f203199386143ef12612a1488a8e9f1cdf1229c382f29c326bcbf6ed6a87d8fbfe0df87dacec6632fc4709d9d338f4cf81e861d942c23bba1e',
-    operatorIndex: 0,
-    used: false,
-    moduleAddress: '0x595F64Ddc3856a3b5Ff4f4CC1d1fb4B46cFd2bAC',
-    index: 100,
-  },
-  {
-    key: '0x911dd3091cfb1b42c960e4f343ea98d9ee6a1dc8ef215afa976fb557bd627a901717c0008bc33a0bfea15f0dfe9c5d01',
-    depositSignature:
-      '0x898ac7072aa26d983f9ece384c4037966dde614b75dddf982f6a415f3107cb2569b96f6d1c44e608a250ac4bbe908df51473f0de2cf732d283b07d88f3786893124967b8697a8b93d31976e7ac49ab1e568f98db0bbb13384477e8357b6d7e9b',
-    operatorIndex: 0,
-    used: false,
-    moduleAddress: '0x595F64Ddc3856a3b5Ff4f4CC1d1fb4B46cFd2bAC',
-    index: 101,
-  },
-  {
-    key: '0x84e85db03bee714dbecf01914460d9576b7f7226030bdbeae9ee923bf5f8e01eec4f7dfe54aa7eca6f4bccce59a0bf42',
-    depositSignature:
-      '0xb024b67a2f6c579213529e143bd4ebb81c5a2dc385cb526de4a816c8fe0317ebfb38369b08622e9f27e62cce2811679a13a459d4e9a8d7bd00080c36b359c1ca03bdcf4a0fcbbc2e18fe9923d8c4edb503ade58bdefe690760611e3738d5e64f',
-    operatorIndex: 28,
-    used: false,
-    moduleAddress: '0x11a93807078f8BB880c1BD0ee4C387537de4b4b6',
-    index: 5,
-  },
-];
-
-const vettedKeysResponse = {
-  blockHash: 'some_hash',
-  blockNumber: 1,
-  vettedKeys,
-  stakingModulesData: [
-    {
-      blockHash: 'some_hash',
-      lastChangedBlockHash: 'some_hash',
-      unusedKeys: [
-        '0x9948d2becf42e9f76922bc6f664545e6f50401050af95785a984802d32a95c4c61f8e3de312b78167f86e047f83a7796',
-        '0x911dd3091cfb1b42c960e4f343ea98d9ee6a1dc8ef215afa976fb557bd627a901717c0008bc33a0bfea15f0dfe9c5d01',
-      ],
-      vettedUnusedKeys: [
-        {
-          key: '0x9948d2becf42e9f76922bc6f664545e6f50401050af95785a984802d32a95c4c61f8e3de312b78167f86e047f83a7796',
-          depositSignature:
-            '0x8bf4401a354de243a3716ee2efc0bde1ded56a40e2943ac7c50290bec37e935d6170b21e7c0872f203199386143ef12612a1488a8e9f1cdf1229c382f29c326bcbf6ed6a87d8fbfe0df87dacec6632fc4709d9d338f4cf81e861d942c23bba1e',
-          operatorIndex: 0,
-          used: false,
-          moduleAddress: '0x595F64Ddc3856a3b5Ff4f4CC1d1fb4B46cFd2bAC',
-          index: 100,
-        },
-        {
-          key: '0x911dd3091cfb1b42c960e4f343ea98d9ee6a1dc8ef215afa976fb557bd627a901717c0008bc33a0bfea15f0dfe9c5d01',
-          depositSignature:
-            '0x898ac7072aa26d983f9ece384c4037966dde614b75dddf982f6a415f3107cb2569b96f6d1c44e608a250ac4bbe908df51473f0de2cf732d283b07d88f3786893124967b8697a8b93d31976e7ac49ab1e568f98db0bbb13384477e8357b6d7e9b',
-          operatorIndex: 0,
-          used: false,
-          moduleAddress: '0x595F64Ddc3856a3b5Ff4f4CC1d1fb4B46cFd2bAC',
-          index: 101,
-        },
-      ],
-      nonce: 0,
-      stakingModuleId: 2,
-      stakingModuleAddress: '0x595F64Ddc3856a3b5Ff4f4CC1d1fb4B46cFd2bAC',
-    },
-    {
-      blockHash: 'some_hash',
-      lastChangedBlockHash: 'some_hash',
-      unusedKeys: [
-        '0x84e85db03bee714dbecf01914460d9576b7f7226030bdbeae9ee923bf5f8e01eec4f7dfe54aa7eca6f4bccce59a0bf42',
-      ],
-      vettedUnusedKeys: [
-        {
-          key: '0x84e85db03bee714dbecf01914460d9576b7f7226030bdbeae9ee923bf5f8e01eec4f7dfe54aa7eca6f4bccce59a0bf42',
-          depositSignature:
-            '0xb024b67a2f6c579213529e143bd4ebb81c5a2dc385cb526de4a816c8fe0317ebfb38369b08622e9f27e62cce2811679a13a459d4e9a8d7bd00080c36b359c1ca03bdcf4a0fcbbc2e18fe9923d8c4edb503ade58bdefe690760611e3738d5e64f',
-          operatorIndex: 28,
-          used: false,
-          moduleAddress: '0x11a93807078f8BB880c1BD0ee4C387537de4b4b6',
-          index: 5,
-        },
-      ],
-      nonce: 0,
-      stakingModuleId: 3,
-      stakingModuleAddress: '0x11a93807078f8BB880c1BD0ee4C387537de4b4b6',
-    },
-  ],
-};
-
 describe('GuardianService', () => {
   let stakingRouterService: StakingRouterService;
   let blockGuardService: BlockGuardService;
@@ -165,19 +75,32 @@ describe('GuardianService', () => {
   });
 
   it('should exit if the previous call is not completed', async () => {
-    jest
-      .spyOn(stakingRouterService, 'getStakingModulesData')
-      .mockImplementation(async () => vettedKeysResponse);
+    // OneAtTime test
+    const getOperatorsAndModulesMock = jest
+      .spyOn(stakingRouterService, 'getOperatorsAndModules')
+      .mockImplementation(async () => ({
+        data: [],
+        meta: {
+          elBlockSnapshot: {
+            blockNumber: 0,
+            blockHash: 'string',
+            timestamp: 0,
+            lastChangedBlockHash: '',
+          },
+        },
+      }));
 
     const getBlockGuardServiceMock = jest
       .spyOn(blockGuardService, 'isNeedToProcessNewState')
       .mockImplementation(() => false);
 
+    // run concurrently and check that second attempt
     await Promise.all([
       guardianService.handleNewBlock(),
       guardianService.handleNewBlock(),
     ]);
 
     expect(getBlockGuardServiceMock).toBeCalledTimes(1);
+    expect(getOperatorsAndModulesMock).toBeCalledTimes(1);
   });
 });
diff --git a/src/guardian/guardian.service.ts b/src/guardian/guardian.service.ts
@@ -94,8 +94,12 @@ export class GuardianService implements OnModuleInit {
     this.logger.log('New staking router state cycle start');
 
     try {
-      const { blockHash, blockNumber, stakingModulesData } =
-        await this.stakingRouterService.getStakingModulesData();
+      const { data: operatorsByModules, meta } =
+        await this.stakingRouterService.getOperatorsAndModules();
+
+      const {
+        elBlockSnapshot: { blockHash, blockNumber },
+      } = meta;
 
       await this.repositoryService.initCachedContracts({ blockHash });
 
@@ -115,7 +119,7 @@ export class GuardianService implements OnModuleInit {
         return;
       }
 
-      const stakingModulesCount = stakingModulesData.length;
+      const stakingModulesCount = operatorsByModules.length;
 
       this.logger.log('Staking modules loaded', {
         modulesCount: stakingModulesCount,
@@ -135,6 +139,12 @@ export class GuardianService implements OnModuleInit {
         blockHash: blockData.blockHash,
       });
 
+      const stakingModulesData =
+        await this.stakingRouterService.getStakingModulesData({
+          data: operatorsByModules,
+          meta,
+        });
+
       const modulesIdWithDuplicateKeys: number[] =
         this.stakingModuleGuardService.getModulesIdsWithDuplicatedVettedUnusedKeys(
           stakingModulesData,

diff --git a/src/guardian/interfaces/state.interface.ts b/src/guardian/interfaces/state.interface.ts
@@ -3,4 +3,5 @@ export interface ContractsState {
   nonce: number;
   depositRoot: string;
   lastChangedBlockHash: string;
+  invalidKeysFound: boolean;
 }
diff --git a/src/guardian/staking-module-guard/staking-module-guard.service.ts b/src/guardian/staking-module-guard/staking-module-guard.service.ts
@@ -165,6 +165,15 @@ export class StakingModuleGuardService {
         return;
       }
 
+      const isValidKeys = await this.isVettedUnusedKeysValid(
+        stakingModuleData,
+        blockData,
+      );
+
+      if (!isValidKeys) {
+        return;
+      }
+
       await this.handleCorrectKeys(stakingModuleData, blockData);
     }
   }
@@ -318,11 +327,14 @@ export class StakingModuleGuardService {
 
     const { nonce, stakingModuleId, lastChangedBlockHash } = stakingModuleData;
 
+    // if we are here we didn't find invalid keys
     const currentContractState = {
       nonce,
       depositRoot,
       blockNumber,
       lastChangedBlockHash,
+      // if we are here we didn't find invalid keys
+      invalidKeysFound: false,
     };
 
     const lastContractsState =
@@ -335,29 +347,12 @@ export class StakingModuleGuardService {
 
     this.lastContractsStateByModuleId[stakingModuleId] = currentContractState;
 
+    // need to check invalidKeysFound
     if (isSameContractsState) {
       this.logger.log("Contract states didn't change");
       return;
     }
 
-    if (
-      !lastContractsState ||
-      currentContractState.lastChangedBlockHash !==
-        lastContractsState.lastChangedBlockHash
-    ) {
-      const invalidKeys = await this.getInvalidKeys(
-        stakingModuleData,
-        blockData,
-      );
-      if (invalidKeys.length) {
-        this.logger.error(
-          'Found invalid keys, will skip deposits until solving problem',
-        );
-        this.guardianMetricsService.incrInvalidKeysEventCounter();
-        return;
-      }
-    }
-
     const signature = await this.securityService.signDepositData(
       depositRoot,
       nonce,
@@ -386,6 +381,72 @@ export class StakingModuleGuardService {
     await this.guardianMessageService.sendDepositMessage(depositMessage);
   }
 
+  public async isVettedUnusedKeysValid(
+    stakingModuleData: StakingModuleData,
+    blockData: BlockData,
+  ): Promise<boolean> {
+    const { blockNumber, depositRoot } = blockData;
+    const { nonce, stakingModuleId, lastChangedBlockHash } = stakingModuleData;
+    const lastContractsState =
+      this.lastContractsStateByModuleId[stakingModuleId];
+
+    if (
+      lastContractsState &&
+      lastChangedBlockHash === lastContractsState.lastChangedBlockHash &&
+      lastContractsState.invalidKeysFound
+    ) {
+      // if found invalid keys on previous iteration and lastChangedBlockHash returned by kapi was not changed
+      // we dont need to validate again, but we still need to skip deposits until problem will not be solved
+      this.logger.error(
+        'LastChangedBlockHash was not changed and on previous iteration we found invalid keys, skip until solving problem ',
+      );
+
+      this.lastContractsStateByModuleId[stakingModuleId] = {
+        nonce,
+        depositRoot,
+        blockNumber,
+        lastChangedBlockHash,
+        invalidKeysFound: true,
+      };
+
+      return false;
+    }
+
+    if (
+      !lastContractsState ||
+      lastChangedBlockHash !== lastContractsState.lastChangedBlockHash
+    ) {
+      // keys was changed or it is a first attempt, need to validate again
+      const invalidKeys = await this.getInvalidKeys(
+        stakingModuleData,
+        blockData,
+      );
+
+      // if found invalid keys, update state and exit
+      if (invalidKeys.length) {
+        this.logger.error(
+          'Found invalid keys, will skip deposits until solving problem',
+        );
+        this.guardianMetricsService.incrInvalidKeysEventCounter();
+        // save info about invalid keys in cache
+        this.lastContractsStateByModuleId[stakingModuleId] = {
+          nonce,
+          depositRoot,
+          blockNumber,
+          lastChangedBlockHash,
+          invalidKeysFound: true,
+        };
+
+        return false;
+      }
+
+      // keys are valid, state will be updated later
+      return true;
+    }
+
+    return true;
+  }
+
   public async getInvalidKeys(
     stakingModuleData: StakingModuleData,
     blockData: BlockData,