Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: duplicated keys in one module will not stop deposits for other modules #166

Merged
merged 1 commit into from
Jan 10, 2024
Merged

test: duplicated keys in one module will not stop deposits for other modules #166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions src/guardian/staking-module-guard/staking-module-guard.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,10 @@ export class StakingModuleGuardService {

// 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');
this.logger.log('Found that we already deposited on these keys', {
blockHash,
stakingModuleId,
});
this.guardianMetricsService.incrDuplicatedUsedKeysEventCounter();
return;
}
Expand Down Expand Up @@ -252,6 +255,7 @@ export class StakingModuleGuardService {
return [];
}

// TODO: add staking module id
eddort marked this conversation as resolved.
Show resolved Hide resolved
this.logger.log(
'Found intersections with lido credentials, need to check used duplicated keys',
);
Expand Down Expand Up @@ -355,7 +359,7 @@ export class StakingModuleGuardService {

// need to check invalidKeysFound
if (isSameContractsState) {
this.logger.log("Contract states didn't change");
this.logger.log("Contract states didn't change", { stakingModuleId });
return;
}

Expand All @@ -379,6 +383,7 @@ export class StakingModuleGuardService {
};

this.logger.log('No problems found', {
stakingModuleId,
blockHash,
lastState: lastContractsState,
newState: currentContractState,
Expand All @@ -404,7 +409,7 @@ export class StakingModuleGuardService {
// 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 ',
`LastChangedBlockHash was not changed and on previous iteration we found invalid keys, skip until solving problem, stakingModuleId: ${stakingModuleId}`,
);

this.lastContractsStateByModuleId[stakingModuleId] = {
Expand All @@ -431,7 +436,7 @@ export class StakingModuleGuardService {
// if found invalid keys, update state and exit
if (invalidKeys.length) {
this.logger.error(
'Found invalid keys, will skip deposits until solving problem',
`Found invalid keys, will skip deposits until solving problem, stakingModuleId: ${stakingModuleId}`,
);
this.guardianMetricsService.incrInvalidKeysEventCounter();
// save info about invalid keys in cache
Expand Down Expand Up @@ -459,17 +464,21 @@ export class StakingModuleGuardService {
): Promise<{ key: string; depositSignature: string }[]> {
this.logger.log('Start keys validation', {
keysCount: stakingModuleData.vettedUnusedKeys.length,
stakingModuleId: stakingModuleData.stakingModuleId,
});
const validationTimeStart = performance.now();

const invalidKeysList = await this.keysValidationService.findInvalidKeys(
stakingModuleData.vettedUnusedKeys,
blockData.lidoWC,
);

const validationTimeEnd = performance.now();
const validationTime =
Math.ceil(validationTimeEnd - validationTimeStart) / 1000;

this.logger.log('Keys validated', {
stakingModuleId: stakingModuleData.stakingModuleId,
invalidKeysList,
validationTime,
});
Expand Down
137 changes: 121 additions & 16 deletions test/manifest.e2e-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ import { RepositoryModule } from '../src/contracts/repository';
import { DepositService } from '../src/contracts/deposit';
import { DepositModule } from '../src/contracts/deposit';

import { SecurityModule } from '../src/contracts/security';
import { SecurityModule, SecurityService } from '../src/contracts/security';

import { LidoService } from '../src/contracts/lido';
import { LidoModule } from '../src/contracts/lido';
Expand Down Expand Up @@ -109,6 +109,8 @@ describe('ganache e2e tests', () => {
let keyValidator: KeyValidatorInterface;
let validateKeys: jest.SpyInstance;

let securityService: SecurityService;

beforeEach(async () => {
server = makeServer(FORK_BLOCK, CHAIN_ID, UNLOCKED_ACCOUNTS);
await server.listen(GANACHE_PORT);
Expand Down Expand Up @@ -158,6 +160,7 @@ describe('ganache e2e tests', () => {
depositService = moduleRef.get(DepositService);
guardianMessageService = moduleRef.get(GuardianMessageService);
keyValidator = moduleRef.get(KeyValidatorInterface);
securityService = moduleRef.get(SecurityService);

// Initializing needed service instead of the whole app
blsService = moduleRef.get(BlsService);
Expand Down Expand Up @@ -872,12 +875,15 @@ describe('ganache e2e tests', () => {

// mocked curated module
const stakingModule = mockedModule(currentBlock, currentBlock.hash);
const stakingDvtModule = mockedModuleDvt(currentBlock, currentBlock.hash);
const meta = mockedMeta(currentBlock, currentBlock.hash);

mockedKeysApiOperators(
mockedKeysApiOperatorsMany(
keysApiService,
mockedOperators,
stakingModule,
[
{ operators: mockedOperators, module: stakingModule },
{ operators: mockedDvtOperators, module: stakingDvtModule },
],
meta,
);

Expand All @@ -901,6 +907,15 @@ describe('ganache e2e tests', () => {
index: 1,
moduleAddress: NOP_REGISTRY,
},
{
key: '0xb3c90525010a5710d43acbea46047fc37ed55306d032527fa15dd7e8cd8a9a5fa490347cc5fce59936fb8300683cd9f3',
depositSignature:
'0x8a77d9411781360cc107344a99f6660b206d2c708ae7fa35565b76ec661a0b86b6c78f5b5691d2cf469c27d0655dfc6311451a9e0501f3c19c6f7e35a770d1a908bfec7cba2e07339dc633b8b6626216ce76ec0fa48ee56aaaf2f9dc7ccb2fe2',
operatorIndex: 0,
used: false,
moduleAddress: FAKE_SIMPLE_DVT,
index: 0,
},
];

mockedKeysApiUnusedKeys(keysApiService, unusedKeys, meta);
Expand All @@ -915,10 +930,34 @@ describe('ganache e2e tests', () => {
);
expect(isOnPause).toBe(false);

const originalIsDepositsPaused = securityService.isDepositsPaused;

// as we have faked simple dvt
jest
.spyOn(securityService, 'isDepositsPaused')
.mockImplementation((stakingModuleId, blockTag) => {
if (stakingModuleId === stakingDvtModule.id) {
return Promise.resolve(false);
}
return originalIsDepositsPaused.call(
securityService,
stakingModuleId,
blockTag,
);
});

await guardianService.handleNewBlock();

// just skip on this iteration deposit for staking module
expect(sendDepositMessage).toBeCalledTimes(0);
expect(sendDepositMessage).toBeCalledTimes(1);
expect(sendDepositMessage).toHaveBeenCalledWith(
expect.objectContaining({
blockNumber: currentBlock.number,
guardianAddress: wallet.address,
guardianIndex: 9,
stakingModuleId: 2,
}),
);
expect(sendPauseMessage).toBeCalledTimes(0);

// after deleting duplicates in staking module,
Expand All @@ -933,16 +972,28 @@ describe('ganache e2e tests', () => {
index: 0,
moduleAddress: NOP_REGISTRY,
},
{
key: '0xb3c90525010a5710d43acbea46047fc37ed55306d032527fa15dd7e8cd8a9a5fa490347cc5fce59936fb8300683cd9f3',
depositSignature:
'0x8a77d9411781360cc107344a99f6660b206d2c708ae7fa35565b76ec661a0b86b6c78f5b5691d2cf469c27d0655dfc6311451a9e0501f3c19c6f7e35a770d1a908bfec7cba2e07339dc633b8b6626216ce76ec0fa48ee56aaaf2f9dc7ccb2fe2',
operatorIndex: 0,
used: false,
moduleAddress: FAKE_SIMPLE_DVT,
index: 0,
},
];

const newBlock = await tempProvider.getBlock('latest');
const newMeta = mockedMeta(newBlock, newBlock.hash);
const newStakingModule = mockedModule(currentBlock, newBlock.hash);
const newStakingModule = mockedModule(newBlock, newBlock.hash);
const newStakingDvtModule = mockedModuleDvt(newBlock, newBlock.hash);

mockedKeysApiOperators(
mockedKeysApiOperatorsMany(
keysApiService,
mockedOperators,
newStakingModule,
[
{ operators: mockedOperators, module: newStakingModule },
{ operators: mockedDvtOperators, module: newStakingDvtModule },
],
newMeta,
);

Expand All @@ -952,9 +1003,11 @@ describe('ganache e2e tests', () => {
newMeta,
);

sendDepositMessage.mockReset();

await guardianService.handleNewBlock();

expect(sendDepositMessage).toBeCalledTimes(1);
expect(sendDepositMessage).toBeCalledTimes(2);

expect(sendDepositMessage).toHaveBeenLastCalledWith(
expect.objectContaining({
Expand All @@ -964,6 +1017,17 @@ describe('ganache e2e tests', () => {
stakingModuleId: 1,
}),
);

expect(sendDepositMessage).toHaveBeenCalledWith(
expect.objectContaining({
blockNumber: newBlock.number,
guardianAddress: wallet.address,
guardianIndex: 9,
stakingModuleId: 2,
}),
);

jest.spyOn(securityService, 'isDepositsPaused').mockRestore();
},
TESTS_TIMEOUT,
);
Expand Down Expand Up @@ -1083,16 +1147,28 @@ describe('ganache e2e tests', () => {
index: 0,
moduleAddress: NOP_REGISTRY,
},
{
key: '0xb3c90525010a5710d43acbea46047fc37ed55306d032527fa15dd7e8cd8a9a5fa490347cc5fce59936fb8300683cd9f3',
depositSignature:
'0x8a77d9411781360cc107344a99f6660b206d2c708ae7fa35565b76ec661a0b86b6c78f5b5691d2cf469c27d0655dfc6311451a9e0501f3c19c6f7e35a770d1a908bfec7cba2e07339dc633b8b6626216ce76ec0fa48ee56aaaf2f9dc7ccb2fe2',
operatorIndex: 0,
used: false,
moduleAddress: FAKE_SIMPLE_DVT,
index: 0,
},
];

const newBlock = await tempProvider.getBlock('latest');
const newMeta = mockedMeta(newBlock, newBlock.hash);
const newStakingModule = mockedModule(currentBlock, newBlock.hash);
const newStakingModule = mockedModule(newBlock, newBlock.hash);
const newStakingDvtModule = mockedModuleDvt(newBlock, newBlock.hash);

mockedKeysApiOperators(
mockedKeysApiOperatorsMany(
keysApiService,
mockedOperators,
newStakingModule,
[
{ operators: mockedOperators, module: newStakingModule },
{ operators: mockedDvtOperators, module: newStakingDvtModule },
],
newMeta,
);

Expand All @@ -1102,18 +1178,47 @@ describe('ganache e2e tests', () => {
newMeta,
);

const originalIsDepositsPaused = securityService.isDepositsPaused;

// as we have faked simple dvt
jest
.spyOn(securityService, 'isDepositsPaused')
.mockImplementation((stakingModuleId, blockTag) => {
if (stakingModuleId === newStakingDvtModule.id) {
return Promise.resolve(false);
}
return originalIsDepositsPaused.call(
securityService,
stakingModuleId,
blockTag,
);
});

sendDepositMessage.mockReset();

await guardianService.handleNewBlock();

expect(sendDepositMessage).toBeCalledTimes(1);
expect(sendDepositMessage).toBeCalledTimes(2);

expect(sendDepositMessage).toHaveBeenLastCalledWith(
expect(sendDepositMessage).toHaveBeenCalledWith(
expect.objectContaining({
blockNumber: newBlock.number,
guardianAddress: wallet.address,
guardianIndex: 9,
stakingModuleId: 1,
}),
);

expect(sendDepositMessage).toHaveBeenCalledWith(
expect.objectContaining({
blockNumber: newBlock.number,
guardianAddress: wallet.address,
guardianIndex: 9,
stakingModuleId: 2,
}),
);

jest.spyOn(securityService, 'isDepositsPaused').mockRestore();
},
TESTS_TIMEOUT,
);
Expand Down