This repository has been archived by the owner on Jun 11, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 458
Missing test(s) for message recovery #9137
Merged
Merged
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
6d15984
Update message recovery code & tests
sitetester d1e24f0
Add missing test for schema verification
sitetester 142e547
Revert check, since this check would be needed only in case message r…
sitetester 62fe774
Revert test for valid params
sitetester fd5c2ce
Rearrange expects per code & cover recoveredCCMs part
sitetester ebefc84
Simplify tests
sitetester 9b50638
Better use `.toHaveBeenCalledWith(ctx)`
sitetester bc3dc83
Merge branch 'release/6.1.0' into 8193-verify-message-recovery
ishantiw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ | |
import { codec } from '@liskhq/lisk-codec'; | ||
import { Transaction } from '@liskhq/lisk-chain'; | ||
import { utils } from '@liskhq/lisk-cryptography'; | ||
import { MerkleTree } from '@liskhq/lisk-tree'; | ||
import { MerkleTree, regularMerkleTree } from '@liskhq/lisk-tree'; | ||
import { Proof } from '@liskhq/lisk-tree/dist-node/merkle_tree/types'; | ||
import { | ||
CROSS_CHAIN_COMMAND_NAME_TRANSFER, | ||
|
@@ -286,7 +286,7 @@ describe('MessageRecoveryCommand', () => { | |
expect(result.error?.message).toInclude(`Cross-chain message does not have a valid index.`); | ||
}); | ||
|
||
it('should return error if idxs[0] <= 1', async () => { | ||
it('should return error if idxs[0] === 1', async () => { | ||
transactionParams.idxs = [1]; | ||
ccms = [ccms[0]]; | ||
ccmsEncoded = ccms.map(ccm => codec.encode(ccmSchema, ccm)); | ||
|
@@ -366,6 +366,42 @@ describe('MessageRecoveryCommand', () => { | |
expect(result.error?.message).toInclude(`Cross-chain message was never in the outbox.`); | ||
}); | ||
|
||
it('should return error if ccm has invalid schema', async () => { | ||
ccms = [ | ||
{ | ||
nonce: BigInt(0), | ||
module: MODULE_NAME_INTEROPERABILITY, | ||
crossChainCommand: CROSS_CHAIN_COMMAND_REGISTRATION, | ||
sendingChainID: utils.intToBuffer(0, 2), // *** | ||
receivingChainID: utils.intToBuffer(3, 4), | ||
fee: BigInt(1), | ||
status: CCMStatusCode.FAILED_CCM, | ||
params: Buffer.alloc(0), | ||
}, | ||
]; | ||
ccmsEncoded = ccms.map(ccm => codec.encode(ccmSchema, ccm)); | ||
transactionParams.crossChainMessages = [...ccmsEncoded]; | ||
transactionParams.idxs = appendPrecedingToIndices([1], terminatedChainOutboxSize); | ||
|
||
commandVerifyContext = createCommandVerifyContext(transaction, transactionParams); | ||
|
||
await interopModule.stores | ||
.get(TerminatedOutboxStore) | ||
.set(createStoreGetter(commandVerifyContext.stateStore as any), chainID, { | ||
outboxRoot, | ||
outboxSize: terminatedChainOutboxSize, | ||
partnerChainInboxSize: 0, | ||
}); | ||
|
||
try { | ||
await command.verify(commandVerifyContext); | ||
} catch (err: any) { | ||
expect((err as Error).message).toInclude( | ||
`Property '.sendingChainID' minLength not satisfied`, | ||
); | ||
} | ||
}); | ||
|
||
it('should return error if ccm.status !== CCMStatusCode.OK', async () => { | ||
ccms = [ | ||
{ | ||
|
@@ -396,7 +432,9 @@ describe('MessageRecoveryCommand', () => { | |
const result = await command.verify(commandVerifyContext); | ||
|
||
expect(result.status).toBe(VerifyStatus.FAIL); | ||
expect(result.error?.message).toInclude(`Cross-chain message status is not valid.`); | ||
expect(result.error?.message).toInclude( | ||
`Cross-chain message status must be equal to value ${CCMStatusCode.OK}.`, | ||
); | ||
}); | ||
|
||
it('should return error if cross-chain message receiving chain ID is not valid', async () => { | ||
|
@@ -536,6 +574,13 @@ describe('MessageRecoveryCommand', () => { | |
|
||
await expect(command.execute(commandExecuteContext)).resolves.toBeUndefined(); | ||
|
||
expect(commandExecuteContext.contextStore.set).toHaveBeenNthCalledWith( | ||
1, | ||
CONTEXT_STORE_KEY_CCM_PROCESSING, | ||
true, | ||
); | ||
|
||
const recoveredCCMs: Buffer[] = []; | ||
for (const crossChainMessage of commandExecuteContext.params.crossChainMessages) { | ||
const ccm = codec.decode<CCMsg>(ccmSchema, crossChainMessage); | ||
const ctx: CrossChainMessageContext = { | ||
|
@@ -546,25 +591,53 @@ describe('MessageRecoveryCommand', () => { | |
), | ||
}; | ||
|
||
const recoveredCCM = await command['_applyRecovery'](ctx); | ||
recoveredCCMs.push(codec.encode(ccmSchema, recoveredCCM)); | ||
|
||
expect(command['_applyRecovery']).toHaveBeenCalledWith(ctx); | ||
expect(commandExecuteContext.contextStore.set).toHaveBeenNthCalledWith( | ||
1, | ||
CONTEXT_STORE_KEY_CCM_PROCESSING, | ||
true, | ||
); | ||
expect(commandExecuteContext.contextStore.set).toHaveBeenNthCalledWith( | ||
2, | ||
CONTEXT_STORE_KEY_CCM_PROCESSING, | ||
false, | ||
); | ||
} | ||
|
||
expect(interopModule.stores.get(TerminatedOutboxStore).set).toHaveBeenCalledTimes(1); | ||
expect(commandExecuteContext.contextStore.set).toHaveBeenNthCalledWith( | ||
2, | ||
CONTEXT_STORE_KEY_CCM_PROCESSING, | ||
false, | ||
); | ||
|
||
const terminatedOutboxSubstore = interopModule.stores.get(TerminatedOutboxStore); | ||
jest.spyOn(terminatedOutboxSubstore, 'set'); | ||
|
||
expect(terminatedOutboxSubstore.set).toHaveBeenCalledTimes(1); | ||
|
||
const terminatedOutboxAccount = await terminatedOutboxSubstore.get( | ||
commandExecuteContext, | ||
commandExecuteContext.params.chainID, | ||
); | ||
const proofLocal = { | ||
size: terminatedOutboxAccount.outboxSize, | ||
idxs: commandExecuteContext.params.idxs, | ||
siblingHashes: commandExecuteContext.params.siblingHashes, | ||
}; | ||
terminatedOutboxAccount.outboxRoot = regularMerkleTree.calculateRootFromUpdateData( | ||
recoveredCCMs.map(ccm => utils.hash(ccm)), | ||
{ ...proofLocal, indexes: proofLocal.idxs }, | ||
); | ||
expect(terminatedOutboxSubstore.set).toHaveBeenCalledWith( | ||
commandExecuteContext, | ||
commandExecuteContext.params.chainID, | ||
terminatedOutboxAccount, | ||
); | ||
}); | ||
|
||
it('should call forwardRecovery when sending chain is not mainchain', async () => { | ||
await expect(command.execute(commandExecuteContext)).resolves.toBeUndefined(); | ||
|
||
expect(commandExecuteContext.contextStore.set).toHaveBeenNthCalledWith( | ||
1, | ||
CONTEXT_STORE_KEY_CCM_PROCESSING, | ||
true, | ||
); | ||
|
||
const recoveredCCMs: Buffer[] = []; | ||
for (const crossChainMessage of commandExecuteContext.params.crossChainMessages) { | ||
const ccm = codec.decode<CCMsg>(ccmSchema, crossChainMessage); | ||
const ctx: CrossChainMessageContext = { | ||
|
@@ -575,23 +648,41 @@ describe('MessageRecoveryCommand', () => { | |
), | ||
}; | ||
|
||
const recoveredCCM = await command['_forwardRecovery'](ctx); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above |
||
recoveredCCMs.push(codec.encode(ccmSchema, recoveredCCM)); | ||
|
||
expect(command['_forwardRecovery']).toHaveBeenCalledWith(ctx); | ||
} | ||
|
||
const terminatedOutboxStore = command['stores'].get(TerminatedOutboxStore); | ||
jest.spyOn(terminatedOutboxStore, 'set'); | ||
|
||
expect(terminatedOutboxStore.set).toHaveBeenCalledTimes(1); | ||
expect(commandExecuteContext.contextStore.set).toHaveBeenNthCalledWith( | ||
1, | ||
CONTEXT_STORE_KEY_CCM_PROCESSING, | ||
true, | ||
); | ||
expect(commandExecuteContext.contextStore.set).toHaveBeenNthCalledWith( | ||
2, | ||
CONTEXT_STORE_KEY_CCM_PROCESSING, | ||
false, | ||
); | ||
|
||
const terminatedOutboxSubstore = command['stores'].get(TerminatedOutboxStore); | ||
const terminatedOutboxAccount = await terminatedOutboxSubstore.get( | ||
commandExecuteContext, | ||
commandExecuteContext.params.chainID, | ||
); | ||
jest.spyOn(terminatedOutboxSubstore, 'set'); | ||
|
||
expect(terminatedOutboxSubstore.set).toHaveBeenCalledTimes(1); | ||
|
||
const proofLocal = { | ||
size: terminatedOutboxAccount.outboxSize, | ||
idxs: commandExecuteContext.params.idxs, | ||
siblingHashes: commandExecuteContext.params.siblingHashes, | ||
}; | ||
terminatedOutboxAccount.outboxRoot = regularMerkleTree.calculateRootFromUpdateData( | ||
recoveredCCMs.map(ccm => utils.hash(ccm)), | ||
{ ...proofLocal, indexes: proofLocal.idxs }, | ||
); | ||
expect(terminatedOutboxSubstore.set).toHaveBeenCalledWith( | ||
commandExecuteContext, | ||
commandExecuteContext.params.chainID, | ||
terminatedOutboxAccount, | ||
); | ||
}); | ||
}); | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
due to this, the expectation in line 597 will be met, even though it was not triggered from the function you are testing, right? Alternatively, you could create a copy of
ctx
, change the status ofccm
toCCM_STATUS_CODE_RECOVERED
and swapccm.sendingChainID
andccm.receivingChainID
.