Skip to content

Commit

Permalink
fix: use of lastChangedBlockHash
Browse files Browse the repository at this point in the history
  • Loading branch information
Amuhar committed Dec 22, 2023
1 parent 5af2277 commit ac14b8d
Show file tree
Hide file tree
Showing 19 changed files with 373 additions and 264 deletions.
10 changes: 10 additions & 0 deletions src/common/custom-errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export class InconsistentLastChangedBlockHash extends Error {
constructor(
message = 'Since the last request, data in Kapi has been updated. This may result in inconsistencies between the data from two separate requests.',
) {
super(message);
this.name = 'InconsistentLastChangedBlockHash';

Object.setPrototypeOf(this, InconsistentLastChangedBlockHash.prototype);
}
}
2 changes: 2 additions & 0 deletions src/guardian/guardian.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const vettedKeysResponse = {
stakingModulesData: [
{
blockHash: 'some_hash',
lastChangedBlockHash: 'some_hash',
unusedKeys: [
'0x9948d2becf42e9f76922bc6f664545e6f50401050af95785a984802d32a95c4c61f8e3de312b78167f86e047f83a7796',
'0x911dd3091cfb1b42c960e4f343ea98d9ee6a1dc8ef215afa976fb557bd627a901717c0008bc33a0bfea15f0dfe9c5d01',
Expand Down Expand Up @@ -91,6 +92,7 @@ const vettedKeysResponse = {
},
{
blockHash: 'some_hash',
lastChangedBlockHash: 'some_hash',
unusedKeys: [
'0x84e85db03bee714dbecf01914460d9576b7f7226030bdbeae9ee923bf5f8e01eec4f7dfe54aa7eca6f4bccce59a0bf42',
],
Expand Down
1 change: 1 addition & 0 deletions src/guardian/interfaces/staking-module.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ export interface StakingModuleData {
vettedUnusedKeys: RegistryKey[];
nonce: number;
stakingModuleId: number;
lastChangedBlockHash: string;
}
1 change: 1 addition & 0 deletions src/guardian/interfaces/state.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ export interface ContractsState {
blockNumber: number;
nonce: number;
depositRoot: string;
lastChangedBlockHash: string;
}
51 changes: 31 additions & 20 deletions src/guardian/staking-module-guard/staking-module-guard.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import { GuardianMetricsService } from '../guardian-metrics';
import { GuardianMessageService } from '../guardian-message';

import { StakingRouterService } from 'staking-router';
import { RegistryKey } from 'keys-api/interfaces/RegistryKey';
import { KeysValidationService } from 'guardian/keys-validation/keys-validation.service';
import { performance } from 'perf_hooks';
import { InconsistentLastChangedBlockHash } from 'common/custom-errors';

@Injectable()
export class StakingModuleGuardService {
Expand Down Expand Up @@ -155,8 +155,8 @@ export class StakingModuleGuardService {
} else {
// it could throw error if kapi returned old data
const usedKeys = await this.findAlreadyDepositedKeys(
stakingModuleData.lastChangedBlockHash,
validIntersections,
blockData,
);

// if found used keys, Lido already made deposit on this keys
Expand Down Expand Up @@ -227,8 +227,8 @@ export class StakingModuleGuardService {
* If it was indeed made by Lido, we set a metric and skip sending deposit messages in the queue for this iteration.
*/
public async findAlreadyDepositedKeys(
lastChangedBlockHash: string,
intersectionsWithLidoWC: VerifiedDepositEvent[],
blockData: BlockData,
) {
const depositedPubkeys = intersectionsWithLidoWC.map(
(deposit) => deposit.pubkey,
Expand All @@ -246,28 +246,26 @@ export class StakingModuleGuardService {
depositedPubkeys,
);

this.checkCurrentBlockOlderThanPrev(
meta.elBlockSnapshot.blockNumber,
blockData.blockNumber,
this.isEqualLastChangedBlockHash(
lastChangedBlockHash,
meta.elBlockSnapshot.lastChangedBlockHash,
);

return data.filter((key) => key.used);
}

private async checkCurrentBlockOlderThanPrev(
currBlockNumber: number,
prevBlockNumber: number,
private isEqualLastChangedBlockHash(
firstRequestHash: string,
secondRequestHash: string,
) {
if (currBlockNumber < prevBlockNumber) {
const errorMsg =
'BlockNumber of the current response older than previous response from KAPI';
this.logger.error(errorMsg, {
previous: prevBlockNumber,
current: currBlockNumber,
});
throw Error(errorMsg);
if (firstRequestHash !== secondRequestHash) {
const error =
'Since the last request, data in Kapi has been updated. This may result in inconsistencies between the data from two separate requests.';

this.logger.error(error, { firstRequestHash, secondRequestHash });

throw new InconsistentLastChangedBlockHash();
}
return;
}

/**
Expand Down Expand Up @@ -333,9 +331,14 @@ export class StakingModuleGuardService {
guardianIndex,
} = blockData;

const { nonce, stakingModuleId } = stakingModuleData;
const { nonce, stakingModuleId, lastChangedBlockHash } = stakingModuleData;

const currentContractState = { nonce, depositRoot, blockNumber };
const currentContractState = {
nonce,
depositRoot,
blockNumber,
lastChangedBlockHash,
};

const lastContractsState =
this.lastContractsStateByModuleId[stakingModuleId];
Expand Down Expand Up @@ -433,7 +436,15 @@ export class StakingModuleGuardService {
): boolean {
if (!firstState || !secondState) return false;
if (firstState.depositRoot !== secondState.depositRoot) return false;

// Check if the nonce values are different. A difference in nonce implies a state change.
if (firstState.nonce !== secondState.nonce) return false;
// If the nonce is unchanged, the state might still have changed.
// Therefore, we also need to compare the 'lastChangedBlockHash'.
// It's important to note that it's not possible for the nonce to be different
// while having the same 'lastChangedBlockHash'.
if (firstState.lastChangedBlockHash !== secondState.lastChangedBlockHash)
return false;

if (
Math.floor(firstState.blockNumber / GUARDIAN_DEPOSIT_RESIGNING_BLOCKS) !==
Expand Down
Loading

0 comments on commit ac14b8d

Please sign in to comment.