From abb27d3746354ef55c5f8b01ce6fa116e8a6daaf Mon Sep 17 00:00:00 2001 From: sitetester Date: Fri, 3 Nov 2023 17:28:45 +0200 Subject: [PATCH] Update State recovery commands (#9119) Update tests for State recovery commands --- .../interoperability/base_state_recovery.ts | 6 +- .../commands/initialize_state_recovery.ts | 7 +- .../base_state_recovery.spec.ts | 12 +- .../initialize_state_recovery.spec.ts | 182 ++++++++++++++---- 4 files changed, 160 insertions(+), 47 deletions(-) diff --git a/framework/src/modules/interoperability/base_state_recovery.ts b/framework/src/modules/interoperability/base_state_recovery.ts index 0cd116393d..62b0f08ac9 100644 --- a/framework/src/modules/interoperability/base_state_recovery.ts +++ b/framework/src/modules/interoperability/base_state_recovery.ts @@ -78,7 +78,7 @@ export class BaseStateRecoveryCommand< if (!moduleMethod.recover) { return { status: VerifyStatus.FAIL, - error: new Error('Module is not recoverable.'), + error: new Error("Module is not recoverable, as it doesn't have a recover method."), }; } @@ -93,7 +93,7 @@ export class BaseStateRecoveryCommand< if (!objectUtils.bufferArrayUniqueItems(queryKeys)) { return { status: VerifyStatus.FAIL, - error: new Error('Recovered store keys are not pairwise distinct.'), + error: new Error('Recoverable store keys are not pairwise distinct.'), }; } @@ -160,7 +160,7 @@ export class BaseStateRecoveryCommand< }); storeQueriesUpdate.push({ key: Buffer.concat([storePrefix, entry.substorePrefix, utils.hash(entry.storeKey)]), - value: RECOVERED_STORE_VALUE, + value: RECOVERED_STORE_VALUE, // The value is set to a constant without known pre-image. bitmap: entry.bitmap, }); } catch (err) { diff --git a/framework/src/modules/interoperability/sidechain/commands/initialize_state_recovery.ts b/framework/src/modules/interoperability/sidechain/commands/initialize_state_recovery.ts index c4319d01ce..5533a5e2f8 100644 --- a/framework/src/modules/interoperability/sidechain/commands/initialize_state_recovery.ts +++ b/framework/src/modules/interoperability/sidechain/commands/initialize_state_recovery.ts @@ -33,6 +33,7 @@ import { getMainchainID } from '../../utils'; import { SidechainInteroperabilityInternalMethod } from '../internal_method'; import { InvalidSMTVerificationEvent } from '../../events/invalid_smt_verification'; +// https://github.com/LiskHQ/lips/blob/main/proposals/lip-0054.md#state-recovery-initialization-command export class InitializeStateRecoveryCommand extends BaseInteroperabilityCommand { public schema = stateRecoveryInitParamsSchema; @@ -106,19 +107,23 @@ export class InitializeStateRecoveryCommand extends BaseInteroperabilityCommand< const smt = new SparseMerkleTree(); let stateRoot: Buffer; + // it will help whether error is for input chainID or mainchainID + let msg; if (terminatedStateAccountExists) { const terminatedStateAccount = await terminatedStateSubstore.get(context, chainID); stateRoot = terminatedStateAccount.mainchainStateRoot; + msg = `given chainID: ${chainID.toString('hex')}.`; } else { const mainchainID = getMainchainID(context.chainID); const mainchainAccount = await this.stores.get(ChainAccountStore).get(context, mainchainID); stateRoot = mainchainAccount.lastCertificate.stateRoot; + msg = `mainchainID: ${mainchainID.toString('hex')}`; } const verified = await smt.verifyInclusionProof(stateRoot, [queryKey], proofOfInclusion); if (!verified) { this.events.get(InvalidSMTVerificationEvent).error(context); - throw new Error('State recovery initialization proof of inclusion is not valid.'); + throw new Error(`State recovery initialization proof of inclusion is not valid for ${msg}.`); } const deserializedSidechainAccount = codec.decode( diff --git a/framework/test/unit/modules/interoperability/base_state_recovery.spec.ts b/framework/test/unit/modules/interoperability/base_state_recovery.spec.ts index 069aa5407c..c6e6d6607e 100644 --- a/framework/test/unit/modules/interoperability/base_state_recovery.spec.ts +++ b/framework/test/unit/modules/interoperability/base_state_recovery.spec.ts @@ -171,7 +171,9 @@ describe('RecoverStateCommand', () => { const result = await stateRecoveryCommand.verify(commandVerifyContext); expect(result.status).toBe(VerifyStatus.FAIL); - expect(result.error?.message).toInclude('Module is not recoverable.'); + expect(result.error?.message).toInclude( + "Module is not recoverable, as it doesn't have a recover method.", + ); }); it('should return error if recovered store keys are not pairwise distinct', async () => { @@ -180,7 +182,7 @@ describe('RecoverStateCommand', () => { const result = await stateRecoveryCommand.verify(commandVerifyContext); expect(result.status).toBe(VerifyStatus.FAIL); - expect(result.error?.message).toInclude('Recovered store keys are not pairwise distinct.'); + expect(result.error?.message).toInclude('Recoverable store keys are not pairwise distinct.'); }); }); @@ -200,6 +202,12 @@ describe('RecoverStateCommand', () => { expect(invalidSMTVerificationEvent.error).toHaveBeenCalled(); }); + it(`should not throw error if recovery is available for "${moduleName}"`, async () => { + await expect(stateRecoveryCommand.execute(commandExecuteContext)).resolves.not.toThrow( + `Recovery failed for module: ${moduleName}`, + ); + }); + it(`should throw error if recovery not available for "${moduleName}"`, async () => { interoperableCCMethods.delete(moduleName); diff --git a/framework/test/unit/modules/interoperability/sidechain/commands/initialize_state_recovery.spec.ts b/framework/test/unit/modules/interoperability/sidechain/commands/initialize_state_recovery.spec.ts index ec3ef4bce0..ee8b5d2026 100644 --- a/framework/test/unit/modules/interoperability/sidechain/commands/initialize_state_recovery.spec.ts +++ b/framework/test/unit/modules/interoperability/sidechain/commands/initialize_state_recovery.spec.ts @@ -55,22 +55,17 @@ import { InvalidSMTVerificationEvent } from '../../../../../../src/modules/inter describe('Sidechain InitializeStateRecoveryCommand', () => { const interopMod = new SidechainInteroperabilityModule(); type StoreMock = Mocked; - const chainAccountStoreMock = { + const getSetHas = () => ({ get: jest.fn(), set: jest.fn(), has: jest.fn(), + }); + const chainAccountStoreMock = { + ...getSetHas(), key: Buffer.from('chainAccount', 'hex'), }; - const ownChainAccountStoreMock = { - get: jest.fn(), - set: jest.fn(), - has: jest.fn(), - }; - const terminatedStateAccountMock = { - get: jest.fn(), - set: jest.fn(), - has: jest.fn(), - }; + const ownChainAccountStoreMock = getSetHas(); + const terminatedStateAccountMock = getSetHas(); let stateRecoveryInitCommand: InitializeStateRecoveryCommand; let commandExecuteContext: CommandExecuteContext; let transaction: Transaction; @@ -85,10 +80,17 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { let commandVerifyContext: CommandVerifyContext; let stateStore: PrefixedStateReadWriter; let mainchainAccount: ChainAccount; + let ownChainAccount: OwnChainAccount; beforeEach(async () => { stateRecoveryInitCommand = interopMod['_stateRecoveryInitCommand']; + ownChainAccount = { + name: 'sidechain', + chainID: utils.intToBuffer(2, 4), + nonce: BigInt('0'), + }; + sidechainChainAccount = { name: 'sidechain1', lastCertificate: { @@ -179,7 +181,6 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { }); describe('verify', () => { - let ownChainAccount: OwnChainAccount; beforeEach(() => { mainchainAccount = { name: 'mainchain', @@ -191,17 +192,9 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { }, status: ChainStatus.ACTIVE, }; - ownChainAccount = { - name: 'sidechain', - chainID: utils.intToBuffer(2, 4), - nonce: BigInt('0'), - }; terminatedStateAccountMock.has.mockResolvedValue(true); ownChainAccountStoreMock.get.mockResolvedValue(ownChainAccount); chainAccountStoreMock.get.mockResolvedValue(mainchainAccount); - interopStoreMock = { - createTerminatedStateAccount: jest.fn(), - }; commandVerifyContext = transactionContext.createCommandVerifyContext( stateRecoveryInitParamsSchema, ); @@ -212,7 +205,15 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { expect(result.status).toBe(VerifyStatus.OK); }); - it('should return error if chain id is same as mainchain id or own chain account id', async () => { + it('should return error if chain id is same as mainchain id', async () => { + commandVerifyContext.params.chainID = getMainchainID(ownChainAccount.chainID); + + await expect(stateRecoveryInitCommand.verify(commandVerifyContext)).rejects.toThrow( + 'Chain ID is not valid.', + ); + }); + + it('should return error if chain id is same as own chain account id', async () => { commandVerifyContext.params.chainID = ownChainAccount.chainID; await expect(stateRecoveryInitCommand.verify(commandVerifyContext)).rejects.toThrow( @@ -220,6 +221,25 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { ); }); + it("should not return error if terminated state account doesn't exist", async () => { + await terminatedStateSubstore.del(createStoreGetter(stateStore), transactionParams.chainID); + + await expect(stateRecoveryInitCommand.verify(commandVerifyContext)).resolves.not.toThrow( + 'Sidechain is already terminated.', + ); + }); + + it('should not return error if terminated state account exists but not initialized', async () => { + await terminatedStateSubstore.set(createStoreGetter(stateStore), transactionParams.chainID, { + ...terminatedStateAccount, + initialized: false, + }); + + await expect(stateRecoveryInitCommand.verify(commandVerifyContext)).resolves.not.toThrow( + 'Sidechain is already terminated.', + ); + }); + it('should return error if terminated state account exists and is initialized', async () => { await terminatedStateSubstore.set(createStoreGetter(stateStore), transactionParams.chainID, { ...terminatedStateAccount, @@ -268,7 +288,7 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { ); }); - it('should return error if the sidechain is active on the mainchain and does not violate the liveness requirement', async () => { + it('should return error if the sidechain has ChainStatus.REGISTERED status', async () => { await terminatedStateSubstore.set(createStoreGetter(stateStore), transactionParams.chainID, { ...terminatedStateAccount, initialized: false, @@ -285,7 +305,7 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { timestamp: 100, validatorsHash: utils.getRandomBytes(32), }, - status: ChainStatus.ACTIVE, + status: ChainStatus.REGISTERED, }; sidechainChainAccountEncoded = codec.encode(chainDataSchema, sidechainChainAccount); transactionParams = { @@ -313,11 +333,11 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { ); await expect(stateRecoveryInitCommand.verify(commandVerifyContext)).rejects.toThrow( - 'Sidechain is still active and obeys the liveness requirement.', + 'Sidechain has status registered.', ); }); - it('should return error if the sidechain has ChainStatus.REGISTERED status', async () => { + it('should return error if the sidechain is active on the mainchain and does not violate the liveness requirement', async () => { await terminatedStateSubstore.set(createStoreGetter(stateStore), transactionParams.chainID, { ...terminatedStateAccount, initialized: false, @@ -334,7 +354,7 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { timestamp: 100, validatorsHash: utils.getRandomBytes(32), }, - status: ChainStatus.REGISTERED, + status: ChainStatus.ACTIVE, }; sidechainChainAccountEncoded = codec.encode(chainDataSchema, sidechainChainAccount); transactionParams = { @@ -362,7 +382,62 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { ); await expect(stateRecoveryInitCommand.verify(commandVerifyContext)).rejects.toThrow( - 'Sidechain has status registered.', + 'Sidechain is still active and obeys the liveness requirement.', + ); + }); + + it('should not return error if the sidechain is active on the mainchain and does violate the liveness requirement', async () => { + await terminatedStateSubstore.set(createStoreGetter(stateStore), transactionParams.chainID, { + ...terminatedStateAccount, + initialized: false, + }); + const mainchainID = getMainchainID(transactionParams.chainID); + when(chainAccountStoreMock.get) + .calledWith(expect.anything(), mainchainID) + .mockResolvedValue({ + ...mainchainAccount, + lastCertificate: { + ...mainchainAccount.lastCertificate, + timestamp: LIVENESS_LIMIT + 50, + }, + } as ChainAccount); + sidechainChainAccount = { + name: 'sidechain1', + lastCertificate: { + height: 10, + stateRoot: utils.getRandomBytes(32), + timestamp: 10, + validatorsHash: utils.getRandomBytes(32), + }, + status: ChainStatus.ACTIVE, + }; + sidechainChainAccountEncoded = codec.encode(chainDataSchema, sidechainChainAccount); + transactionParams = { + chainID: utils.intToBuffer(3, 4), + bitmap: Buffer.alloc(0), + siblingHashes: [], + sidechainAccount: sidechainChainAccountEncoded, + }; + encodedTransactionParams = codec.encode(stateRecoveryInitParamsSchema, transactionParams); + transaction = new Transaction({ + module: MODULE_NAME_INTEROPERABILITY, + command: COMMAND_NAME_STATE_RECOVERY_INIT, + fee: BigInt(100000000), + nonce: BigInt(0), + params: encodedTransactionParams, + senderPublicKey: utils.getRandomBytes(32), + signatures: [], + }); + transactionContext = createTransactionContext({ + transaction, + stateStore, + }); + commandVerifyContext = transactionContext.createCommandVerifyContext( + stateRecoveryInitParamsSchema, + ); + + await expect(stateRecoveryInitCommand.verify(commandVerifyContext)).resolves.not.toThrow( + 'Sidechain is still active and obeys the liveness requirement.', ); }); }); @@ -391,8 +466,9 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { jest.spyOn(SparseMerkleTree.prototype, 'verify').mockResolvedValue(false); jest.spyOn(invalidSMTVerificationEvent, 'error'); + const msg = `given chainID: ${commandExecuteContext.params.chainID.toString('hex')}.`; await expect(stateRecoveryInitCommand.execute(commandExecuteContext)).rejects.toThrow( - 'State recovery initialization proof of inclusion is not valid', + `State recovery initialization proof of inclusion is not valid for ${msg}.`, ); expect(interopStoreMock.createTerminatedStateAccount).not.toHaveBeenCalled(); expect(invalidSMTVerificationEvent.error).toHaveBeenCalled(); @@ -413,15 +489,24 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { transactionParams.chainID, ); + const msg = `mainchainID: ${mainchainID.toString('hex')}`; await expect(stateRecoveryInitCommand.execute(commandExecuteContext)).rejects.toThrow( - 'State recovery initialization proof of inclusion is not valid', + `State recovery initialization proof of inclusion is not valid for ${msg}.`, ); expect(interopStoreMock.createTerminatedStateAccount).not.toHaveBeenCalled(); expect(invalidSMTVerificationEvent.error).toHaveBeenCalled(); }); it('should create a terminated state account when there is none', async () => { - // Arrange & Assign & Act + const mainchainID = getMainchainID(commandExecuteContext.chainID); + + when(chainAccountStoreMock.get) + .calledWith(expect.anything(), mainchainID) + .mockResolvedValue(mainchainAccount); + + jest.spyOn(terminatedStateSubstore, 'has').mockResolvedValue(false); + jest.spyOn(stateRecoveryInitCommand['internalMethod'], 'createTerminatedStateAccount'); + await stateRecoveryInitCommand.execute(commandExecuteContext); const accountFromStore = await terminatedStateSubstore.get( @@ -429,29 +514,44 @@ describe('Sidechain InitializeStateRecoveryCommand', () => { transactionParams.chainID, ); - // Assert expect(accountFromStore).toEqual({ ...terminatedStateAccount, initialized: true }); - expect(interopStoreMock.createTerminatedStateAccount).not.toHaveBeenCalled(); + expect( + stateRecoveryInitCommand['internalMethod'].createTerminatedStateAccount, + ).toHaveBeenCalledTimes(1); }); it('should update the terminated state account when there is one', async () => { - // Arrange & Assign & Act - when(terminatedStateAccountMock.has) - .calledWith(expect.anything(), transactionParams.chainID) - .mockResolvedValue(false); - const terminatedStateStore = interopMod.stores.get(TerminatedStateStore); - terminatedStateStore.get = terminatedStateAccountMock.get; - terminatedStateAccountMock.get.mockResolvedValue(terminatedStateAccount); + + jest.spyOn(stateRecoveryInitCommand['internalMethod'], 'createTerminatedStateAccount'); + jest.spyOn(terminatedStateStore, 'get').mockResolvedValue(terminatedStateAccount); + jest.spyOn(terminatedStateStore, 'set'); + await stateRecoveryInitCommand.execute(commandExecuteContext); + const deserializedSidechainAccount = codec.decode( + chainDataSchema, + commandExecuteContext.params.sidechainAccount, + ); + expect(terminatedStateStore.set).toHaveBeenCalledWith( + commandExecuteContext, + commandExecuteContext.params.chainID, + { + stateRoot: deserializedSidechainAccount.lastCertificate.stateRoot, + mainchainStateRoot: EMPTY_HASH, + initialized: true, + }, + ); + const accountFromStore = await terminatedStateSubstore.get( commandExecuteContext, transactionParams.chainID, ); - - // Assert expect(accountFromStore).toEqual(terminatedStateAccount); + + expect( + stateRecoveryInitCommand['internalMethod'].createTerminatedStateAccount, + ).not.toHaveBeenCalled(); }); }); });