From 57cd0af677c494f28d83fb8abaaf06139ac3568b Mon Sep 17 00:00:00 2001 From: Anna Mukharram Date: Tue, 6 Feb 2024 20:00:17 +0400 Subject: [PATCH 1/5] fix: add staking module id --- src/common/prometheus/prometheus.constants.ts | 4 ++-- .../guardian-metrics/guardian-metrics.service.ts | 14 ++++++++------ .../staking-module-guard.service.ts | 15 ++++++++++++--- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/common/prometheus/prometheus.constants.ts b/src/common/prometheus/prometheus.constants.ts index 3c49cf95..cc28c74a 100644 --- a/src/common/prometheus/prometheus.constants.ts +++ b/src/common/prometheus/prometheus.constants.ts @@ -22,8 +22,8 @@ export const METRIC_OPERATORS_KEYS_TOTAL = `${METRICS_PREFIX}operators_keys_tota export const METRIC_KEYS_API_REQUEST_DURATION = `${METRICS_PREFIX}keys_api_requests_duration_seconds`; -export const METRIC_DUPLICATED_VETTED_UNUSED_KEYS_EVENT_COUNTER = `${METRICS_PREFIX}vetted_unused_keys_event_total`; +export const METRIC_DUPLICATED_VETTED_UNUSED_KEYS_EVENT_COUNTER = `${METRICS_PREFIX}duplicated_vetted_unused_keys_event_total`; -export const METRIC_DUPLICATED_USED_KEYS_EVENT_COUNTER = `${METRICS_PREFIX}used_keys_event_total`; +export const METRIC_DUPLICATED_USED_KEYS_EVENT_COUNTER = `${METRICS_PREFIX}duplicated_used_keys_event_total`; export const METRIC_INVALID_KEYS_EVENT_COUNTER = `${METRICS_PREFIX}invalid_keys_event_total`; diff --git a/src/guardian/guardian-metrics/guardian-metrics.service.ts b/src/guardian/guardian-metrics/guardian-metrics.service.ts index 230b3cc9..95c11c8f 100644 --- a/src/guardian/guardian-metrics/guardian-metrics.service.ts +++ b/src/guardian/guardian-metrics/guardian-metrics.service.ts @@ -133,21 +133,23 @@ export class GuardianMetricsService { /** * increment duplicated vetted unused keys event counter */ - public incrDuplicatedVettedUnusedKeysEventCounter() { - this.duplicatedVettedUnusedKeysEventCounter.inc(); + public incrDuplicatedVettedUnusedKeysEventCounter(stakingModuleId: number) { + this.duplicatedVettedUnusedKeysEventCounter + .labels({ stakingModuleId }) + .inc(); } /** * increment duplicated used keys event counter */ - public incrDuplicatedUsedKeysEventCounter() { - this.duplicatedUsedKeysEventCounter.inc(); + public incrDuplicatedUsedKeysEventCounter(stakingModuleId: number) { + this.duplicatedUsedKeysEventCounter.labels({ stakingModuleId }).inc(); } /** * increment invalid keys event counter */ - public incrInvalidKeysEventCounter() { - this.invalidKeysEventCounter.inc(); + public incrInvalidKeysEventCounter(stakingModuleId: number) { + this.invalidKeysEventCounter.labels({ stakingModuleId }).inc(); } } diff --git a/src/guardian/staking-module-guard/staking-module-guard.service.ts b/src/guardian/staking-module-guard/staking-module-guard.service.ts index a63cfe27..13928ff9 100644 --- a/src/guardian/staking-module-guard/staking-module-guard.service.ts +++ b/src/guardian/staking-module-guard/staking-module-guard.service.ts @@ -81,7 +81,12 @@ export class StakingModuleGuardService { moduleAddressesWithDuplicates: moduleAddressesWithDuplicatesList, }); - this.guardianMetricsService.incrDuplicatedVettedUnusedKeysEventCounter(); + for (const id of modulesWithDuplicatedKeysSet) { + this.guardianMetricsService.incrDuplicatedVettedUnusedKeysEventCounter( + id, + ); + } + return moduleAddressesWithDuplicatesList; } @@ -164,7 +169,9 @@ export class StakingModuleGuardService { blockHash, stakingModuleId, }); - this.guardianMetricsService.incrDuplicatedUsedKeysEventCounter(); + this.guardianMetricsService.incrDuplicatedUsedKeysEventCounter( + stakingModuleData.stakingModuleId, + ); return; } @@ -439,7 +446,9 @@ export class StakingModuleGuardService { this.logger.error( `Found invalid keys, will skip deposits until solving problem, stakingModuleId: ${stakingModuleId}`, ); - this.guardianMetricsService.incrInvalidKeysEventCounter(); + this.guardianMetricsService.incrInvalidKeysEventCounter( + stakingModuleData.stakingModuleId, + ); // save info about invalid keys in cache this.lastContractsStateByModuleId[stakingModuleId] = { nonce, From 2087eae6085f75b7d4996bc057fa42c0b40f8e79 Mon Sep 17 00:00:00 2001 From: Anna Mukharram Date: Tue, 6 Feb 2024 20:21:34 +0400 Subject: [PATCH 2/5] fix: metric type --- src/common/prometheus/prometheus.provider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/prometheus/prometheus.provider.ts b/src/common/prometheus/prometheus.provider.ts index 32a2e314..9e680a6e 100644 --- a/src/common/prometheus/prometheus.provider.ts +++ b/src/common/prometheus/prometheus.provider.ts @@ -109,7 +109,7 @@ export const PrometheusUsedKeysEventProvider = makeCounterProvider({ labelNames: ['stakingModuleId'] as const, }); -export const PrometheusInvalidKeysEventProvider = makeGaugeProvider({ +export const PrometheusInvalidKeysEventProvider = makeCounterProvider({ name: METRIC_INVALID_KEYS_EVENT_COUNTER, help: 'Number of invalid keys', labelNames: ['stakingModuleId'] as const, From 25b97f79c9681a3952e21ec143c26dfc4bde7509 Mon Sep 17 00:00:00 2001 From: Anna Mukharram Date: Wed, 7 Feb 2024 23:02:45 +0400 Subject: [PATCH 3/5] fix: set in metrics duplicates number --- src/common/prometheus/prometheus.constants.ts | 6 +- src/common/prometheus/prometheus.module.ts | 12 +- src/common/prometheus/prometheus.provider.ts | 18 +-- .../guardian-metrics.service.ts | 49 ++++++--- .../staking-module-guard.service.ts | 103 ++++++++++-------- test/helpers/mockKeysApi.ts | 2 +- test/manifest.e2e-spec.ts | 9 ++ 7 files changed, 114 insertions(+), 85 deletions(-) diff --git a/src/common/prometheus/prometheus.constants.ts b/src/common/prometheus/prometheus.constants.ts index cc28c74a..5ac5a13d 100644 --- a/src/common/prometheus/prometheus.constants.ts +++ b/src/common/prometheus/prometheus.constants.ts @@ -22,8 +22,8 @@ export const METRIC_OPERATORS_KEYS_TOTAL = `${METRICS_PREFIX}operators_keys_tota export const METRIC_KEYS_API_REQUEST_DURATION = `${METRICS_PREFIX}keys_api_requests_duration_seconds`; -export const METRIC_DUPLICATED_VETTED_UNUSED_KEYS_EVENT_COUNTER = `${METRICS_PREFIX}duplicated_vetted_unused_keys_event_total`; +export const METRIC_DUPLICATED_VETTED_UNUSED_KEYS_TOTAL = `${METRICS_PREFIX}duplicated_vetted_unused_keys`; -export const METRIC_DUPLICATED_USED_KEYS_EVENT_COUNTER = `${METRICS_PREFIX}duplicated_used_keys_event_total`; +export const METRIC_DUPLICATED_USED_KEYS_TOTAL = `${METRICS_PREFIX}duplicated_used_keys`; -export const METRIC_INVALID_KEYS_EVENT_COUNTER = `${METRICS_PREFIX}invalid_keys_event_total`; +export const METRIC_INVALID_KEYS_TOTAL = `${METRICS_PREFIX}invalid_keys`; diff --git a/src/common/prometheus/prometheus.module.ts b/src/common/prometheus/prometheus.module.ts index f0be4a78..aae759ea 100644 --- a/src/common/prometheus/prometheus.module.ts +++ b/src/common/prometheus/prometheus.module.ts @@ -13,9 +13,9 @@ import { PrometheusDepositedKeysProvider, PrometheusOperatorsKeysProvider, PrometheusKeysApiRequestsProvider, - PrometheusVettedUnusedKeysEventProvider, - PrometheusUsedKeysEventProvider, - PrometheusInvalidKeysEventProvider, + PrometheusUsedKeysProvider, + PrometheusVettedUnusedKeysProvider, + PrometheusInvalidKeysProvider, } from './prometheus.provider'; import { METRICS_PREFIX, METRICS_URL } from './prometheus.constants'; @@ -41,9 +41,9 @@ const providers = [ PrometheusDepositedKeysProvider, PrometheusOperatorsKeysProvider, PrometheusKeysApiRequestsProvider, - PrometheusVettedUnusedKeysEventProvider, - PrometheusUsedKeysEventProvider, - PrometheusInvalidKeysEventProvider, + PrometheusVettedUnusedKeysProvider, + PrometheusUsedKeysProvider, + PrometheusInvalidKeysProvider, ]; PrometheusModule.global = true; diff --git a/src/common/prometheus/prometheus.provider.ts b/src/common/prometheus/prometheus.provider.ts index 9e680a6e..63c46211 100644 --- a/src/common/prometheus/prometheus.provider.ts +++ b/src/common/prometheus/prometheus.provider.ts @@ -17,9 +17,9 @@ import { METRIC_DEPOSITED_KEYS_TOTAL, METRIC_OPERATORS_KEYS_TOTAL, METRIC_KEYS_API_REQUEST_DURATION, - METRIC_DUPLICATED_VETTED_UNUSED_KEYS_EVENT_COUNTER, - METRIC_DUPLICATED_USED_KEYS_EVENT_COUNTER, - METRIC_INVALID_KEYS_EVENT_COUNTER, + METRIC_DUPLICATED_VETTED_UNUSED_KEYS_TOTAL, + METRIC_DUPLICATED_USED_KEYS_TOTAL, + METRIC_INVALID_KEYS_TOTAL, } from './prometheus.constants'; export const PrometheusTransportMessageCounterProvider = makeCounterProvider({ @@ -97,20 +97,20 @@ export const PrometheusKeysApiRequestsProvider = makeHistogramProvider({ labelNames: ['result', 'status'] as const, }); -export const PrometheusVettedUnusedKeysEventProvider = makeCounterProvider({ - name: METRIC_DUPLICATED_VETTED_UNUSED_KEYS_EVENT_COUNTER, +export const PrometheusVettedUnusedKeysProvider = makeGaugeProvider({ + name: METRIC_DUPLICATED_VETTED_UNUSED_KEYS_TOTAL, help: 'Number of duplicated vetted unused keys events', labelNames: ['stakingModuleId'] as const, }); -export const PrometheusUsedKeysEventProvider = makeCounterProvider({ - name: METRIC_DUPLICATED_USED_KEYS_EVENT_COUNTER, +export const PrometheusUsedKeysProvider = makeGaugeProvider({ + name: METRIC_DUPLICATED_USED_KEYS_TOTAL, help: 'Number of duplicated used keys events', labelNames: ['stakingModuleId'] as const, }); -export const PrometheusInvalidKeysEventProvider = makeCounterProvider({ - name: METRIC_INVALID_KEYS_EVENT_COUNTER, +export const PrometheusInvalidKeysProvider = makeGaugeProvider({ + name: METRIC_INVALID_KEYS_TOTAL, help: 'Number of invalid keys', labelNames: ['stakingModuleId'] as const, }); diff --git a/src/guardian/guardian-metrics/guardian-metrics.service.ts b/src/guardian/guardian-metrics/guardian-metrics.service.ts index 95c11c8f..a04c2ba6 100644 --- a/src/guardian/guardian-metrics/guardian-metrics.service.ts +++ b/src/guardian/guardian-metrics/guardian-metrics.service.ts @@ -7,11 +7,11 @@ import { METRIC_DEPOSITED_KEYS_TOTAL, METRIC_OPERATORS_KEYS_TOTAL, METRIC_INTERSECTIONS_TOTAL, - METRIC_DUPLICATED_USED_KEYS_EVENT_COUNTER, - METRIC_INVALID_KEYS_EVENT_COUNTER, - METRIC_DUPLICATED_VETTED_UNUSED_KEYS_EVENT_COUNTER, + METRIC_INVALID_KEYS_TOTAL, + METRIC_DUPLICATED_VETTED_UNUSED_KEYS_TOTAL, + METRIC_DUPLICATED_USED_KEYS_TOTAL, } from 'common/prometheus'; -import { Counter, Gauge } from 'prom-client'; +import { Gauge } from 'prom-client'; @Injectable() export class GuardianMetricsService { @@ -28,14 +28,14 @@ export class GuardianMetricsService { @InjectMetric(METRIC_INTERSECTIONS_TOTAL) private intersectionsCounter: Gauge, - @InjectMetric(METRIC_DUPLICATED_USED_KEYS_EVENT_COUNTER) - private duplicatedUsedKeysEventCounter: Counter, + @InjectMetric(METRIC_DUPLICATED_USED_KEYS_TOTAL) + private duplicatedUsedKeysCounter: Gauge, - @InjectMetric(METRIC_DUPLICATED_VETTED_UNUSED_KEYS_EVENT_COUNTER) - private duplicatedVettedUnusedKeysEventCounter: Counter, + @InjectMetric(METRIC_DUPLICATED_VETTED_UNUSED_KEYS_TOTAL) + private duplicatedVettedUnusedKeysCounter: Gauge, - @InjectMetric(METRIC_INVALID_KEYS_EVENT_COUNTER) - private invalidKeysEventCounter: Counter, + @InjectMetric(METRIC_INVALID_KEYS_TOTAL) + private invalidKeysCounter: Gauge, ) {} /** @@ -133,23 +133,36 @@ export class GuardianMetricsService { /** * increment duplicated vetted unused keys event counter */ - public incrDuplicatedVettedUnusedKeysEventCounter(stakingModuleId: number) { - this.duplicatedVettedUnusedKeysEventCounter - .labels({ stakingModuleId }) - .inc(); + public collectDuplicatedVettedUnusedKeysMetrics( + stakingModuleId: number, + duplicatedVettedUnusedKeysCount: number, + ) { + this.duplicatedVettedUnusedKeysCounter.set( + { stakingModuleId }, + duplicatedVettedUnusedKeysCount, + ); } /** * increment duplicated used keys event counter */ - public incrDuplicatedUsedKeysEventCounter(stakingModuleId: number) { - this.duplicatedUsedKeysEventCounter.labels({ stakingModuleId }).inc(); + public collectDuplicatedUsedKeysMetrics( + stakingModuleId: number, + duplicatedUsedKeysCount: number, + ) { + this.duplicatedUsedKeysCounter.set( + { stakingModuleId }, + duplicatedUsedKeysCount, + ); } /** * increment invalid keys event counter */ - public incrInvalidKeysEventCounter(stakingModuleId: number) { - this.invalidKeysEventCounter.labels({ stakingModuleId }).inc(); + public collectInvalidKeysMetrics( + stakingModuleId: number, + invalidKeysCount: number, + ) { + this.invalidKeysCounter.set({ stakingModuleId }, invalidKeysCount); } } diff --git a/src/guardian/staking-module-guard/staking-module-guard.service.ts b/src/guardian/staking-module-guard/staking-module-guard.service.ts index 13928ff9..82bd3e39 100644 --- a/src/guardian/staking-module-guard/staking-module-guard.service.ts +++ b/src/guardian/staking-module-guard/staking-module-guard.service.ts @@ -36,69 +36,71 @@ export class StakingModuleGuardService { stakingModulesData: StakingModuleData[], blockData: BlockData, ): number[] { - // Collects the duplicate count for each unique key across staking modules. - // The outer Map uses the key string as the key and holds an inner Map. - // The inner Map uses module id as keys and stores the duplicate count for each module. - const keyMap = new Map>(); - const modulesWithDuplicatedKeysSet = new Set(); - const duplicatedKeys = new Map>(); + // Collects the duplicate count for each unique key across staking modules + // This map collects, for every key, a set of module IDs where the key was found. + const keyModuleOccurrences = new Map>(); + // This map collects, for every module, the number of duplicated keys found in it. + const moduleDuplicatedKeysCount = new Map(); stakingModulesData.forEach(({ vettedUnusedKeys, stakingModuleId }) => { - // check module keys on duplicates across all modules - vettedUnusedKeys.forEach((key) => { - const stakingModules = keyMap.get(key.key); - - if (!stakingModules) { + // This set tracks keys that are duplicated within the same module. + const duplicatedKeysInsideOneModule = new Set(); + vettedUnusedKeys.forEach(({ key }) => { + // is a key new for module + const isNewKeyForModule = !duplicatedKeysInsideOneModule.has(key); + // Mark this key as encountered for this module + duplicatedKeysInsideOneModule.add(key); + + // modules where the key was found + const moduleIds = keyModuleOccurrences.get(key); + + if (!moduleIds) { // add new key - keyMap.set(key.key, new Map([[stakingModuleId, 1]])); + keyModuleOccurrences.set(key, new Set([stakingModuleId])); } else { - // found duplicate - // Duplicate key found - const moduleCount = stakingModules.get(stakingModuleId) || 0; - stakingModules.set(stakingModuleId, moduleCount + 1); - - if (this.hasDuplicateKeys(stakingModules)) { - stakingModules.forEach((_, id) => { - modulesWithDuplicatedKeysSet.add(id); + moduleIds.add(stakingModuleId); + + if (moduleIds.size > 1 || !isNewKeyForModule) { + moduleIds.forEach((id) => { + moduleDuplicatedKeysCount.set( + id, + (moduleDuplicatedKeysCount.get(id) || 0) + 1, + ); }); - duplicatedKeys.set(key.key, stakingModules); } } }); }); - if (modulesWithDuplicatedKeysSet.size) { - const moduleAddressesWithDuplicatesList: number[] = Array.from( - modulesWithDuplicatedKeysSet, + // set metrics + stakingModulesData.forEach(({ stakingModuleId }) => { + const countOfDuplicatedKeys = + moduleDuplicatedKeysCount.get(stakingModuleId) || 0; + this.guardianMetricsService.collectDuplicatedVettedUnusedKeysMetrics( + stakingModuleId, + countOfDuplicatedKeys, ); + }); + + if (moduleDuplicatedKeysCount.size) { + const moduleDuplicatedKeysCountList = Array.from( + moduleDuplicatedKeysCount, + ).map(([stakingModuleId, duplicatedKeys]) => ({ + stakingModuleId, + duplicatedKeys, + })); this.logger.error('Found duplicated vetted keys'); this.logger.log('Duplicated keys', { blockHash: blockData.blockHash, - duplicatedKeys: Array.from(duplicatedKeys).map(([key, innerMap]) => ({ - key: key, - stakingModuleIds: Array.from(innerMap.keys()), - })), - moduleAddressesWithDuplicates: moduleAddressesWithDuplicatesList, + modulesWithDuplicates: moduleDuplicatedKeysCountList, }); - for (const id of modulesWithDuplicatedKeysSet) { - this.guardianMetricsService.incrDuplicatedVettedUnusedKeysEventCounter( - id, - ); - } - - return moduleAddressesWithDuplicatesList; + return Array.from(moduleDuplicatedKeysCount.keys()); } return []; } - private hasDuplicateKeys(stakingModules: Map): boolean { - const moduleCounts = Array.from(stakingModules.values()); - - return stakingModules.size > 1 || moduleCounts[0] > 1; - } - public excludeModulesWithDuplicatedKeys( stakingModulesData: StakingModuleData[], modulesIdWithDuplicateKeys: number[], @@ -163,15 +165,17 @@ export class StakingModuleGuardService { validIntersections, ); + this.guardianMetricsService.collectDuplicatedUsedKeysMetrics( + stakingModuleData.stakingModuleId, + usedKeys.length, + ); + // if found used keys, Lido already made deposit on this keys if (usedKeys.length) { this.logger.log('Found that we already deposited on these keys', { blockHash, stakingModuleId, }); - this.guardianMetricsService.incrDuplicatedUsedKeysEventCounter( - stakingModuleData.stakingModuleId, - ); return; } @@ -441,14 +445,17 @@ export class StakingModuleGuardService { blockData, ); + this.guardianMetricsService.collectInvalidKeysMetrics( + stakingModuleData.stakingModuleId, + invalidKeys.length, + ); + // if found invalid keys, update state and exit if (invalidKeys.length) { this.logger.error( `Found invalid keys, will skip deposits until solving problem, stakingModuleId: ${stakingModuleId}`, ); - this.guardianMetricsService.incrInvalidKeysEventCounter( - stakingModuleData.stakingModuleId, - ); + // save info about invalid keys in cache this.lastContractsStateByModuleId[stakingModuleId] = { nonce, diff --git a/test/helpers/mockKeysApi.ts b/test/helpers/mockKeysApi.ts index 13189b84..3c0fb3a8 100644 --- a/test/helpers/mockKeysApi.ts +++ b/test/helpers/mockKeysApi.ts @@ -66,7 +66,7 @@ export const mockedOperators: RegistryOperator[] = [ stakingLimit: 12, stoppedValidators: 0, totalSigningKeys: 12, - usedSigningKeys: 10, + usedSigningKeys: 9, index: 0, active: true, moduleAddress: NOP_REGISTRY, diff --git a/test/manifest.e2e-spec.ts b/test/manifest.e2e-spec.ts index c14669bf..b033d0ae 100644 --- a/test/manifest.e2e-spec.ts +++ b/test/manifest.e2e-spec.ts @@ -1091,6 +1091,15 @@ describe('ganache e2e tests', () => { index: 1, moduleAddress: NOP_REGISTRY, }, + { + key: '0xa9bfaa8207ee6c78644c079ffc91b6e5abcc5eede1b7a06abb8fb40e490a75ea269c178dd524b65185299d2bbd2eb7b2', + depositSignature: + '0xaa5f2a1053ba7d197495df44d4a32b7ae10265cf9e38560a16b782978c0a24271a113c9538453b7e45f35cb64c7adb460d7a9fe8c8ce6b8c80ca42fd5c48e180c73fc08f7d35ba32e39f32c902fd333faf47611827f0b7813f11c4c518dd2e59', + operatorIndex: 0, + used: false, + index: 12, + moduleAddress: NOP_REGISTRY, + }, { key: '0xb3c90525010a5710d43acbea46047fc37ed55306d032527fa15dd7e8cd8a9a5fa490347cc5fce59936fb8300683cd9f3', depositSignature: From 739cd04fc9d6cadaa6cc1bedda413c4c37daca92 Mon Sep 17 00:00:00 2001 From: Anna Mukharram Date: Thu, 8 Feb 2024 16:04:12 +0400 Subject: [PATCH 4/5] fix: kapi version --- src/guardian/guardian.constants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/guardian/guardian.constants.ts b/src/guardian/guardian.constants.ts index 23fab3fa..9fcaaa75 100644 --- a/src/guardian/guardian.constants.ts +++ b/src/guardian/guardian.constants.ts @@ -3,4 +3,4 @@ import { CronExpression } from '@nestjs/schedule'; export const GUARDIAN_DEPOSIT_RESIGNING_BLOCKS = 10; export const GUARDIAN_DEPOSIT_JOB_NAME = 'guardian-deposit-job'; export const GUARDIAN_DEPOSIT_JOB_DURATION = CronExpression.EVERY_5_SECONDS; -export const MIN_KAPI_VERSION = '0.10.2'; +export const MIN_KAPI_VERSION = '1.0.1'; From 87a659e35a80c31519f94aba41df2590a8d35703 Mon Sep 17 00:00:00 2001 From: Anna Mukharram Date: Fri, 9 Feb 2024 13:14:01 +0400 Subject: [PATCH 5/5] fix: version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 91b5010f..572c3b79 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "lido-council-daemon", - "version": "2.0.1", + "version": "2.0.2", "description": "Lido Council Daemon", "author": "Lido team", "private": true,