From 554f0ff9567248db13a38de06e606ded2c1c1b9a Mon Sep 17 00:00:00 2001 From: shuse2 Date: Wed, 27 Sep 2023 15:11:11 +0200 Subject: [PATCH 1/3] :bug: Fix genesis block aggregate commit --- elements/lisk-chain/src/block_header.ts | 6 +-- elements/lisk-chain/test/unit/block.spec.ts | 2 +- .../lisk-chain/test/unit/block_header.spec.ts | 4 +- framework/src/engine/consensus/consensus.ts | 2 +- framework/src/genesis_block.ts | 2 +- .../unit/engine/consensus/consensus.spec.ts | 45 +++++++++++++++++++ framework/test/unit/genesis_block.spec.ts | 1 + 7 files changed, 54 insertions(+), 8 deletions(-) diff --git a/elements/lisk-chain/src/block_header.ts b/elements/lisk-chain/src/block_header.ts index d7a3c21ae26..591dac7b3ae 100644 --- a/elements/lisk-chain/src/block_header.ts +++ b/elements/lisk-chain/src/block_header.ts @@ -261,13 +261,13 @@ export class BlockHeader { }); } - if (header.aggregateCommit.height !== 0) { + if (header.aggregateCommit.height !== header.height) { errors.push({ - message: 'Genesis block header aggregateCommit.height must equal 0', + message: 'Genesis block header aggregateCommit.height must equal to the height', keyword: 'const', dataPath: 'aggregateCommit.height', schemaPath: 'properties.aggregateCommit.height', - params: { allowedValue: 0 }, + params: { allowedValue: header.height }, }); } diff --git a/elements/lisk-chain/test/unit/block.spec.ts b/elements/lisk-chain/test/unit/block.spec.ts index b1c25337bee..5bac9da83ca 100644 --- a/elements/lisk-chain/test/unit/block.spec.ts +++ b/elements/lisk-chain/test/unit/block.spec.ts @@ -124,7 +124,7 @@ describe('block', () => { maxHeightGenerated: 0, validatorsHash: utils.hash(Buffer.alloc(0)), aggregateCommit: { - height: 0, + height: 1009988, aggregationBits: Buffer.alloc(0), certificateSignature: EMPTY_BUFFER, }, diff --git a/elements/lisk-chain/test/unit/block_header.spec.ts b/elements/lisk-chain/test/unit/block_header.spec.ts index a60ac0fcce8..76494e9da6a 100644 --- a/elements/lisk-chain/test/unit/block_header.spec.ts +++ b/elements/lisk-chain/test/unit/block_header.spec.ts @@ -257,7 +257,7 @@ describe('block_header', () => { ); }); - it('should throw error if aggregateCommit.height is not equal to 0', () => { + it('should throw error if aggregateCommit.height is not equal to the height', () => { const block = getGenesisBlockAttrs(); const blockHeader = new BlockHeader({ ...block, @@ -265,7 +265,7 @@ describe('block_header', () => { }); expect(() => blockHeader.validateGenesis()).toThrow( - 'Genesis block header aggregateCommit.height must equal 0', + 'Genesis block header aggregateCommit.height must equal to the height', ); }); diff --git a/framework/src/engine/consensus/consensus.ts b/framework/src/engine/consensus/consensus.ts index 87726032971..49b412a7f18 100644 --- a/framework/src/engine/consensus/consensus.ts +++ b/framework/src/engine/consensus/consensus.ts @@ -399,7 +399,7 @@ export class Consensus { const finalizedBlockHeader = await this._chain.dataAccess.getBlockHeaderByHeight( this._chain.finalizedHeight, ); - return finalizedBlockHeader.aggregateCommit.height; + return Math.max(finalizedBlockHeader.aggregateCommit.height, this._chain.genesisHeight); } private async _execute(block: Block, peerID: string): Promise { diff --git a/framework/src/genesis_block.ts b/framework/src/genesis_block.ts index afebea4d925..f698d4a2524 100644 --- a/framework/src/genesis_block.ts +++ b/framework/src/genesis_block.ts @@ -68,7 +68,7 @@ export const generateGenesisBlock = async ( impliesMaxPrevotes: true, assetRoot, aggregateCommit: { - height: 0, + height, aggregationBits: EMPTY_BUFFER, certificateSignature: EMPTY_BUFFER, }, diff --git a/framework/test/unit/engine/consensus/consensus.spec.ts b/framework/test/unit/engine/consensus/consensus.spec.ts index 53350492fa6..35dc97eecea 100644 --- a/framework/test/unit/engine/consensus/consensus.spec.ts +++ b/framework/test/unit/engine/consensus/consensus.spec.ts @@ -73,6 +73,7 @@ describe('consensus', () => { dataAccess: { getBlockHeaderByHeight: jest.fn().mockResolvedValue(lastBlock.header), }, + genesisHeight: 0, } as unknown as Chain; network = { registerEndpoint: jest.fn(), @@ -221,6 +222,50 @@ describe('consensus', () => { }); }); + describe('getMaxRemovalHeight', () => { + it('should return genesis height if the finalizedBlock.aggregateCommit.height is smaller', async () => { + const finalizedBlock = await createValidDefaultBlock({ + header: { + height: 1, + aggregateCommit: { + height: 0, + aggregationBits: Buffer.alloc(0), + certificateSignature: Buffer.alloc(0), + }, + }, + }); + (chain.dataAccess.getBlockHeaderByHeight as jest.Mock).mockResolvedValue( + finalizedBlock.header, + ); + const genesisHeight = 25519; + (chain as any).genesisHeight = genesisHeight; + + await expect(consensus.getMaxRemovalHeight()).resolves.toEqual(genesisHeight); + }); + + it('should return finalizedBlock.aggregateCommit.height if the genesis height is smaller', async () => { + const finalizedBlock = await createValidDefaultBlock({ + header: { + height: 1, + aggregateCommit: { + height: 25520, + aggregationBits: Buffer.alloc(0), + certificateSignature: Buffer.alloc(0), + }, + }, + }); + (chain.dataAccess.getBlockHeaderByHeight as jest.Mock).mockResolvedValue( + finalizedBlock.header, + ); + const genesisHeight = 25519; + (chain as any).genesisHeight = genesisHeight; + + await expect(consensus.getMaxRemovalHeight()).resolves.toEqual( + finalizedBlock.header.aggregateCommit.height, + ); + }); + }); + describe('certifySingleCommit', () => { const passphrase = Mnemonic.generateMnemonic(256); const { publicKey } = legacy.getPrivateAndPublicKeyFromPassphrase(passphrase); diff --git a/framework/test/unit/genesis_block.spec.ts b/framework/test/unit/genesis_block.spec.ts index 2ee294300df..9a287c8f99a 100644 --- a/framework/test/unit/genesis_block.spec.ts +++ b/framework/test/unit/genesis_block.spec.ts @@ -33,6 +33,7 @@ describe('generateGenesisBlock', () => { expect(result.header.validatorsHash).toHaveLength(32); expect(result.header.eventRoot).toHaveLength(32); expect(result.header.version).toBe(0); + expect(result.header.aggregateCommit.height).toBe(30); expect(stateMachine.executeGenesisBlock).toHaveBeenCalledTimes(1); }); From 4cd4053d67d7d97baf5775ed064e46fb581b4b4c Mon Sep 17 00:00:00 2001 From: shuse2 Date: Thu, 28 Sep 2023 13:30:53 +0200 Subject: [PATCH 2/3] Update elements/lisk-chain/src/block_header.ts Co-authored-by: !shan --- elements/lisk-chain/src/block_header.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elements/lisk-chain/src/block_header.ts b/elements/lisk-chain/src/block_header.ts index 591dac7b3ae..2523b95d193 100644 --- a/elements/lisk-chain/src/block_header.ts +++ b/elements/lisk-chain/src/block_header.ts @@ -263,7 +263,7 @@ export class BlockHeader { if (header.aggregateCommit.height !== header.height) { errors.push({ - message: 'Genesis block header aggregateCommit.height must equal to the height', + message: 'Genesis block header aggregateCommit.height must equal to the genesis height', keyword: 'const', dataPath: 'aggregateCommit.height', schemaPath: 'properties.aggregateCommit.height', From b047ec377b56ed86935d5c7eee44b32764aed4cb Mon Sep 17 00:00:00 2001 From: shuse2 Date: Fri, 29 Sep 2023 09:41:25 +0200 Subject: [PATCH 3/3] :white_check_mark: Fix unit test --- elements/lisk-chain/test/unit/block_header.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elements/lisk-chain/test/unit/block_header.spec.ts b/elements/lisk-chain/test/unit/block_header.spec.ts index 76494e9da6a..1580adff18f 100644 --- a/elements/lisk-chain/test/unit/block_header.spec.ts +++ b/elements/lisk-chain/test/unit/block_header.spec.ts @@ -265,7 +265,7 @@ describe('block_header', () => { }); expect(() => blockHeader.validateGenesis()).toThrow( - 'Genesis block header aggregateCommit.height must equal to the height', + 'Genesis block header aggregateCommit.height must equal to the genesis height', ); });