Skip to content

Commit

Permalink
fix: code review
Browse files Browse the repository at this point in the history
  • Loading branch information
eddort committed Feb 27, 2023
1 parent a7fdc11 commit da65ca5
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 31 deletions.
1 change: 0 additions & 1 deletion src/contracts/deposit/deposit.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
import { OneAtTime } from 'common/decorators';
import { RepositoryService } from 'contracts/repository';
import { CacheService } from 'cache';
import { BlockData } from 'guardian';
import { BlockTag } from 'provider';
import { BlsService } from 'bls';
import { APP_VERSION } from 'app.constants';
Expand Down
7 changes: 3 additions & 4 deletions src/guardian/block-guard/block-guard.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,8 @@ export class BlockGuardService {
blockNumber: number;
blockHash: string;
}): Promise<BlockData> {
const endTimer = this.blockRequestsHistogram.startTimer();
try {
const endTimer = this.blockRequestsHistogram.startTimer();

const guardianAddress = this.securityService.getGuardianAddress();

const [depositRoot, depositedEvents, guardianIndex] = await Promise.all([
Expand All @@ -75,8 +74,6 @@ export class BlockGuardService {
this.depositService.handleNewBlock(blockNumber),
]);

endTimer();

return {
blockNumber,
blockHash,
Expand All @@ -88,6 +85,8 @@ export class BlockGuardService {
} catch (error) {
this.blockErrorsCounter.inc();
throw error;
} finally {
endTimer();
}
}
}
7 changes: 4 additions & 3 deletions src/guardian/guardian-message/guardian-message.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
MessagesService,
MessageType,
} from 'messages';
import { BlockData, StakingModuleData } from '../interfaces';
import { BlockData } from '../interfaces';
import { APP_NAME, APP_VERSION } from 'app.constants';

@Injectable()
Expand All @@ -21,10 +21,11 @@ export class GuardianMessageService {

/**
* Sends a ping message to the message broker
* @param stakingModuleIds - all staking router ids
* @param blockData - collected data from the current block
*/
public async pingMessageBroker(
{ stakingModuleId }: StakingModuleData,
stakingModuleIds: number[],
blockData: BlockData,
): Promise<void> {
const { blockNumber, guardianIndex, guardianAddress } = blockData;
Expand All @@ -34,7 +35,7 @@ export class GuardianMessageService {
blockNumber,
guardianIndex,
guardianAddress,
stakingModuleId,
stakingModuleIds,
});
}

Expand Down
5 changes: 0 additions & 5 deletions src/guardian/guardian-metrics/guardian-metrics.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ export class GuardianMetricsService {

const depositedKeys = events.map(({ pubkey }) => pubkey);
const depositedKeysSet = new Set(depositedKeys);
const depositedDubsTotal = depositedKeys.length - depositedKeysSet.size;

this.depositedKeysCounter.set(
{ type: 'total', stakingModuleId },
Expand All @@ -86,10 +85,6 @@ export class GuardianMetricsService {
{ type: 'unique', stakingModuleId },
depositedKeysSet.size,
);
this.depositedKeysCounter.set(
{ type: 'duplicates', stakingModuleId },
depositedDubsTotal,
);
}

/**
Expand Down
27 changes: 13 additions & 14 deletions src/guardian/guardian.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class GuardianService implements OnModuleInit {
// The event cache is stored with an N block lag to avoid caching data from uncle blocks
// so we don't worry about blockHash here
await this.depositService.updateEventsCache();
// Subscribes to events only after the cache is warmed up

this.subscribeToModulesUpdates();
} catch (error) {
this.logger.error(error);
Expand All @@ -69,7 +69,7 @@ export class GuardianService implements OnModuleInit {
}

/**
* Subscribes to the event of a new block appearance
* Subscribes to the staking router modules updates
*/
public subscribeToModulesUpdates() {
const cron = new CronJob(GUARDIAN_DEPOSIT_JOB_DURATION, () => {
Expand All @@ -90,7 +90,7 @@ export class GuardianService implements OnModuleInit {
*/
@OneAtTime()
public async handleNewBlock(): Promise<void> {
this.logger.log('New block cycle start');
this.logger.log('New staking router state cycle start');

const {
elBlockSnapshot: { blockHash, blockNumber },
Expand Down Expand Up @@ -118,16 +118,10 @@ export class GuardianService implements OnModuleInit {
blockHash,
);

await Promise.all([
this.stakingModuleGuardService.checkKeysIntersections(
stakingModuleData,
blockData,
),
this.guardianMessageService.pingMessageBroker(
stakingModuleData,
blockData,
),
]);
await this.stakingModuleGuardService.checkKeysIntersections(
stakingModuleData,
blockData,
);

this.guardianMetricsService.collectMetrics(
stakingModuleData,
Expand All @@ -136,11 +130,16 @@ export class GuardianService implements OnModuleInit {
}),
);

await this.guardianMessageService.pingMessageBroker(
stakingModules.map(({ id }) => id),
blockData,
);

this.blockGuardService.setLastProcessedStateMeta({
blockHash,
blockNumber,
});

this.logger.log('New block cycle end');
this.logger.log('New staking router state cycle end');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export class StakingModuleGuardService {
blockData: BlockData,
): Promise<void> {
const { blockHash } = blockData;
const { stakingModuleId } = stakingModuleData;

const keysIntersections = this.getKeysIntersections(
stakingModuleData,
Expand All @@ -88,7 +89,7 @@ export class StakingModuleGuardService {
);

if (stakingModuleData.isDepositsPaused) {
this.logger.warn('Deposits are paused', { blockHash });
this.logger.warn('Deposits are paused', { blockHash, stakingModuleId });
return;
}

Expand All @@ -110,19 +111,20 @@ export class StakingModuleGuardService {
blockData: BlockData,
): VerifiedDepositEvent[] {
const { blockHash, depositRoot, depositedEvents } = blockData;
const { nonce, unusedKeys } = stakingModuleData;
const { nonce, unusedKeys, stakingModuleId } = stakingModuleData;

const unusedKeysSet = new Set(unusedKeys);
const intersections = depositedEvents.events.filter(({ pubkey }) =>
unusedKeysSet.has(pubkey),
);

if (intersections.length) {
this.logger.warn('Already deposited keys found in the next Lido keys', {
this.logger.warn('Already deposited keys found in the module keys', {
blockHash,
depositRoot,
nonce,
intersections,
stakingModuleId,
});
}

Expand Down
3 changes: 2 additions & 1 deletion src/messages/interfaces/message.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ export interface MessageRequiredFields {
type: MessageType;
guardianAddress: string;
guardianIndex: number;
stakingModuleId: number;
}

export enum MessageType {
Expand All @@ -19,6 +18,7 @@ export interface MessageDeposit extends MessageRequiredFields {
blockNumber: number;
blockHash: string;
signature: Signature;
stakingModuleId: number;
}

export interface MessageMeta {
Expand All @@ -36,4 +36,5 @@ export interface MessagePause extends MessageRequiredFields {
blockNumber: number;
blockHash: string;
signature: Signature;
stakingModuleId: number;
}

0 comments on commit da65ca5

Please sign in to comment.