Improve block processing tests

elements/lisk-chain/src/block_assets.ts

@@ -100,7 +100,9 @@ export class BlockAssets {
 				);
 			}
 			if (last.module > asset.module) {
-				throw new Error('Assets are not sorted in the increasing values of moduleID.');
+				throw new Error(
+					'Assets are not sorted by the module property value in lexicographical order.',
+				);
 			}
 			// Check for duplicates
 			if (i > 0 && asset.module === last.module) {
@@ -118,7 +120,9 @@ export class BlockAssets {
 			validator.validate(blockAssetSchema, asset);
 
 			if (last.module > asset.module) {
-				throw new Error('Assets are not sorted in the increasing values of moduleID.');
+				throw new Error(
+					'Assets are not sorted by the module property value in lexicographical order.',
+				);
 			}
 			if (i > 0 && asset.module === last.module) {
 				throw new Error(`Module with ID ${this._assets[i].module} has duplicate entries.`);

elements/lisk-chain/test/unit/block_assets.spec.ts

@@ -129,11 +129,11 @@ describe('block assets', () => {
 				assetList = [
 					{
 						module: 'auth',
-						data: utils.getRandomBytes(64),
+						data: utils.getRandomBytes(MAX_ASSET_DATA_SIZE_BYTES),
 					},
 					{
 						module: 'random',
-						data: utils.getRandomBytes(128),
+						data: utils.getRandomBytes(MAX_ASSET_DATA_SIZE_BYTES + 1),
 					},
 				];
 				assets = new BlockAssets(assetList);
@@ -146,20 +146,20 @@ describe('block assets', () => {
 				assetList = [
 					{
 						module: 'auth',
-						data: utils.getRandomBytes(64),
+						data: utils.getRandomBytes(MAX_ASSET_DATA_SIZE_BYTES / 2),
 					},
 					{
 						module: 'random',
-						data: utils.getRandomBytes(64),
+						data: utils.getRandomBytes(MAX_ASSET_DATA_SIZE_BYTES / 2),
 					},
 				];
 				assets = new BlockAssets(assetList);
 				expect(assets.validate()).toBeUndefined();
 			});
 		});
 
-		describe('when the assets are not sorted by moduleID', () => {
-			it('should throw error when assets are not sorted by moduleID', () => {
+		describe('when the assets are not sorted by module', () => {
+			it('should throw error when assets are not sorted by module', () => {
 				assetList = [
 					{
 						module: 'random',
@@ -172,7 +172,7 @@ describe('block assets', () => {
 				];
 				assets = new BlockAssets(assetList);
 				expect(() => assets.validate()).toThrow(
-					'Assets are not sorted in the increasing values of moduleID.',
+					'Assets are not sorted by the module property value in lexicographical order.',
 				);
 			});
 
@@ -300,7 +300,7 @@ describe('block assets', () => {
 				];
 				assets = new BlockAssets(assetList);
 				expect(() => assets.validateGenesis()).toThrow(
-					'Assets are not sorted in the increasing values of moduleID.',
+					'Assets are not sorted by the module property value in lexicographical order.',
 				);
 			});
 

elements/lisk-chain/test/unit/block_header.spec.ts

@@ -81,8 +81,10 @@ const blockHeaderProps = [
 	'previousBlockID',
 	'generatorAddress',
 	'transactionRoot',
+	'eventRoot',
 	'assetRoot',
 	'stateRoot',
+	'impliesMaxPrevotes',
 	'maxHeightPrevoted',
 	'maxHeightGenerated',
 	'validatorsHash',
@@ -143,6 +145,7 @@ describe('block_header', () => {
 				expect(blockHeader.validatorsHash).toEqual(data.validatorsHash);
 				expect(blockHeader.aggregateCommit).toEqual(data.aggregateCommit);
 				expect(blockHeader.maxHeightPrevoted).toEqual(data.maxHeightPrevoted);
+				expect(blockHeader.impliesMaxPrevotes).toEqual(data.impliesMaxPrevotes);
 				expect(blockHeader.maxHeightGenerated).toEqual(data.maxHeightGenerated);
 				expect(blockHeader.assetRoot).toEqual(data.assetRoot);
 				expect(blockHeader.transactionRoot).toEqual(data.transactionRoot);

elements/lisk-chain/test/unit/chain.spec.ts

@@ -33,6 +33,7 @@ import {
 	DEFAULT_MAX_BLOCK_HEADER_CACHE,
 	DEFAULT_MIN_BLOCK_HEADER_CACHE,
 } from '../../src/constants';
+import { BlockAssets, BlockHeader, Transaction } from '../../src';
 
 describe('chain', () => {
 	const constants = {
@@ -296,11 +297,19 @@ describe('chain', () => {
 
 		it('should not throw error with a valid block', async () => {
 			const txs = new Array(20).fill(0).map(() => getTransaction());
+			const totalSize = txs.reduce((prev, curr) => prev + curr.getBytes().length, 0);
+			(chainInstance as any).constants.maxTransactionsSize = totalSize;
 			block = await createValidDefaultBlock({
 				transactions: txs,
 			});
+			jest.spyOn(BlockHeader.prototype, 'validate');
+			jest.spyOn(BlockAssets.prototype, 'validate');
+			jest.spyOn(Transaction.prototype, 'validate');
 			// Act & assert
 			expect(() => chainInstance.validateBlock(block, { version: 2 })).not.toThrow();
+			expect(BlockHeader.prototype.validate).toHaveBeenCalledTimes(1);
+			expect(BlockAssets.prototype.validate).toHaveBeenCalledTimes(1);
+			expect(Transaction.prototype.validate).toHaveBeenCalledTimes(txs.length);
 		});
 
 		it('should throw error if transaction root does not match', async () => {
@@ -317,12 +326,13 @@ describe('chain', () => {
 
 		it('should throw error if transactions exceeds max transactions length', async () => {
 			// Arrange
-			(chainInstance as any).constants.maxTransactionsSize = 100;
 			const txs = new Array(200).fill(0).map(() => getTransaction());
+			const totalSize = txs.reduce((prev, curr) => prev + curr.getBytes().length, 0);
+			(chainInstance as any).constants.maxTransactionsSize = totalSize - 1;
 			block = await createValidDefaultBlock({ transactions: txs });
 			// Act & assert
 			expect(() => chainInstance.validateBlock(block, { version: 2 })).toThrow(
-				'Transactions length is longer than configured length: 100.',
+				`Transactions length is longer than configured length: ${totalSize - 1}.`,
 			);
 		});
 
@@ -337,5 +347,41 @@ describe('chain', () => {
 				'Block version must be 2.',
 			);
 		});
+
+		it('should throw error if block header validation fails', async () => {
+			const txs = new Array(20).fill(0).map(() => getTransaction());
+			block = await createValidDefaultBlock({
+				transactions: txs,
+			});
+			jest.spyOn(BlockHeader.prototype, 'validate').mockImplementation(() => {
+				throw new Error('invalid header');
+			});
+			// Act & assert
+			expect(() => chainInstance.validateBlock(block, { version: 2 })).toThrow('invalid header');
+		});
+
+		it('should throw error if block asset validation fails', async () => {
+			const txs = new Array(20).fill(0).map(() => getTransaction());
+			block = await createValidDefaultBlock({
+				transactions: txs,
+			});
+			jest.spyOn(BlockAssets.prototype, 'validate').mockImplementation(() => {
+				throw new Error('invalid assets');
+			});
+			// Act & assert
+			expect(() => chainInstance.validateBlock(block, { version: 2 })).toThrow('invalid assets');
+		});
+
+		it('should throw error if transaction validation fails', async () => {
+			const txs = new Array(20).fill(0).map(() => getTransaction());
+			block = await createValidDefaultBlock({
+				transactions: txs,
+			});
+			jest.spyOn(Transaction.prototype, 'validate').mockImplementation(() => {
+				throw new Error('invalid tx');
+			});
+			// Act & assert
+			expect(() => chainInstance.validateBlock(block, { version: 2 })).toThrow('invalid tx');
+		});
 	});
 });

elements/lisk-chain/test/unit/transactions.spec.ts

@@ -13,21 +13,26 @@
  */
 import { utils } from '@liskhq/lisk-cryptography';
 import { Transaction } from '../../src/transaction';
+import { TRANSACTION_MAX_PARAMS_SIZE } from '../../src/constants';
 
 describe('blocks/transactions', () => {
-	describe('transaction', () => {
-		it.todo('should have id');
-		it.todo('should have senderAddress');
-		it.todo('should throw when module is invalid');
-		it.todo('should throw when command is invalid');
-		it.todo('should throw when sender public key is invalid');
-		it.todo('should throw when nonce is invalid');
-		it.todo('should throw when fee is invalid');
-		it.todo('should throw when params is invalid');
-	});
 	describe('#validateTransaction', () => {
 		let transaction: Transaction;
 
+		it('should not throw when transaction is valid', () => {
+			transaction = new Transaction({
+				module: 'token',
+				command: 'transfer',
+				fee: BigInt(613000),
+				// 126 is the size of other properties
+				params: utils.getRandomBytes(TRANSACTION_MAX_PARAMS_SIZE),
+				nonce: BigInt(2),
+				senderPublicKey: utils.getRandomBytes(32),
+				signatures: [utils.getRandomBytes(64)],
+			});
+			expect(() => transaction.validate()).not.toThrow();
+		});
+
 		it('should throw when module name is invalid', () => {
 			transaction = new Transaction({
 				module: 'token_mod',
@@ -54,6 +59,20 @@ describe('blocks/transactions', () => {
 			expect(() => transaction.validate()).toThrow('Invalid command name');
 		});
 
+		it('should throw when transaction is too big', () => {
+			transaction = new Transaction({
+				module: 'token',
+				command: 'transfer',
+				fee: BigInt(613000),
+				// 126 is the size of other properties
+				params: utils.getRandomBytes(TRANSACTION_MAX_PARAMS_SIZE + 1),
+				nonce: BigInt(2),
+				senderPublicKey: utils.getRandomBytes(32),
+				signatures: [utils.getRandomBytes(64)],
+			});
+			expect(() => transaction.validate()).toThrow('Params exceeds max size allowed');
+		});
+
 		it('should throw when sender public key is not 32 bytes', () => {
 			transaction = new Transaction({
 				module: 'token',

framework/test/unit/engine/consensus/consensus.spec.ts

@@ -708,6 +708,14 @@ describe('consensus', () => {
 				expect(savingEvents).toHaveLength(3);
 				savingEvents.forEach((e: Event, i: number) => expect(e.toObject().index).toEqual(i));
 			});
+
+			it('should reject when ABI.commit fails and it should not store the block', async () => {
+				jest.spyOn(chain, 'saveBlock');
+				jest.spyOn(consensus['_abi'], 'commit').mockRejectedValue(new Error('fail to commit'));
+
+				await expect(consensus['_executeValidated'](block)).rejects.toThrow('fail to commit');
+				expect(chain.saveBlock).not.toHaveBeenCalled();
+			});
 		});
 
 		describe('block verification', () => {
@@ -738,9 +746,12 @@ describe('consensus', () => {
 				it('should throw error when block timestamp is from future', () => {
 					const invalidBlock = { ...block };
 
-					jest.spyOn(bft.method, 'getSlotNumber').mockReturnValue(Math.floor(Date.now() / 10));
-
-					(invalidBlock.header as any).timestamp = Math.floor((Date.now() + 10000) / 1000);
+					jest
+						.spyOn(bft.method, 'getSlotNumber')
+						// return blockSlotNumber in the future
+						.mockReturnValueOnce(Math.floor(Date.now() / 1000 / 10) + 10000)
+						// return blockSlotNumber for the currrent value
+						.mockReturnValueOnce(Math.floor(Date.now() / 1000 / 10));
 
 					expect(() => consensus['_verifyTimestamp'](invalidBlock as any)).toThrow(
 						`Invalid timestamp ${
@@ -876,6 +887,20 @@ describe('consensus', () => {
 					);
 				});
 
+				it('should throw error if the header impliesMaxPrevotes is not the same as the computed value', async () => {
+					when(consensus['_bft'].method.getBFTHeights as never)
+						.calledWith(stateStore)
+						.mockResolvedValue({ maxHeightPrevoted: block.header.maxHeightPrevoted } as never);
+
+					when(consensus['_bft'].method.impliesMaximalPrevotes as never)
+						.calledWith(stateStore, block.header)
+						.mockResolvedValue(false as never);
+
+					await expect(consensus['_verifyBFTProperties'](stateStore, block as any)).rejects.toThrow(
+						'Invalid imply max prevote',
+					);
+				});
+
 				it('should be success if maxHeightPrevoted is valid and header is not contradicting', async () => {
 					when(consensus['_bft'].method.getBFTHeights as never)
 						.calledWith(stateStore)
@@ -885,6 +910,10 @@ describe('consensus', () => {
 						.calledWith(stateStore, block.header)
 						.mockResolvedValue(false as never);
 
+					when(consensus['_bft'].method.impliesMaximalPrevotes as never)
+						.calledWith(stateStore, block.header)
+						.mockResolvedValue(true as never);
+
 					await expect(
 						consensus['_verifyBFTProperties'](stateStore, block as any),
 					).resolves.toBeUndefined();

framework/test/unit/state_machine/state_machine.spec.ts

@@ -184,6 +184,7 @@ describe('state_machine', () => {
 				getMethodContext: expect.any(Function),
 				getStore: expect.any(Function),
 			});
+			expect(mod.commands[0].execute).toHaveBeenCalledTimes(1);
 			expect(mod.afterCommandExecute).toHaveBeenCalledTimes(1);
 		});