diff --git a/framework/src/engine/consensus/synchronizer/base_synchronizer.ts b/framework/src/engine/consensus/synchronizer/base_synchronizer.ts index 137d42c1fda..0ae83d15228 100644 --- a/framework/src/engine/consensus/synchronizer/base_synchronizer.ts +++ b/framework/src/engine/consensus/synchronizer/base_synchronizer.ts @@ -64,8 +64,9 @@ export abstract class BaseSynchronizer { protected async _getHighestCommonBlockFromNetwork( peerId: string, ids: Buffer[], - ): Promise { + ): Promise { const blockIds = codec.encode(getHighestCommonBlockRequestSchema, { ids }); + const { data } = (await this._network.requestFromPeer({ procedure: NETWORK_RPC_GET_HIGHEST_COMMON_BLOCK, peerId, @@ -79,11 +80,15 @@ export abstract class BaseSynchronizer { data, ); + if (!decodedResp.id.length) { + return undefined; + } try { validator.validate(getHighestCommonBlockResponseSchema, decodedResp); } catch { throw new ApplyPenaltyAndAbortError(peerId, 'Invalid common block response format'); } + return this._chain.dataAccess.getBlockHeaderByID(decodedResp.id); } diff --git a/framework/src/engine/consensus/synchronizer/block_synchronization_mechanism.ts b/framework/src/engine/consensus/synchronizer/block_synchronization_mechanism.ts index 80423114f23..e1b9e89e9e9 100644 --- a/framework/src/engine/consensus/synchronizer/block_synchronization_mechanism.ts +++ b/framework/src/engine/consensus/synchronizer/block_synchronization_mechanism.ts @@ -372,7 +372,6 @@ export class BlockSynchronizationMechanism extends BaseSynchronizer { const blockHeaders = await this._chain.dataAccess.getBlockHeadersWithHeights(heightList); let data: BlockHeader | undefined; - try { // Request the highest common block with the previously computed list // to the given peer diff --git a/framework/test/unit/engine/consensus/synchronizer/block_synchronization_mechanism/block_synchronization_mechanism.spec.ts b/framework/test/unit/engine/consensus/synchronizer/block_synchronization_mechanism/block_synchronization_mechanism.spec.ts index dfc1fda30be..12ffd2b4a81 100644 --- a/framework/test/unit/engine/consensus/synchronizer/block_synchronization_mechanism/block_synchronization_mechanism.spec.ts +++ b/framework/test/unit/engine/consensus/synchronizer/block_synchronization_mechanism/block_synchronization_mechanism.spec.ts @@ -509,7 +509,7 @@ describe('block_synchronization_mechanism', () => { describe('request and revert to last common block from peer', () => { describe('request the highest common block', () => { - it('should give up requesting the last common block after 3 tries, and then ban the peer and restart the mechanism', async () => { + it('should give up requesting the last common block after 3 tries if there is a network error, and then ban the peer and restart the mechanism', async () => { // Set last block to a high height const lastBlock = await createValidDefaultBlock({ header: { @@ -542,7 +542,7 @@ describe('block_synchronization_mechanism', () => { peerId, data: blockIds, }) - .mockResolvedValue({ + .mockRejectedValue({ data: codec.encode(getHighestCommonBlockResponseSchema, { id: Buffer.alloc(0) }), } as never); @@ -599,6 +599,187 @@ describe('block_synchronization_mechanism', () => { ).not.toHaveBeenCalled(); }); + it('should give up requesting the last common block after 3 tries if the common block response format is invalid, and then ban the peer and restart the mechanism', async () => { + // Set last block to a high height + const lastBlock = await createValidDefaultBlock({ + header: { + height: genesisBlock.header.height + 2000, + }, + }); + chainModule._lastBlock = lastBlock; + // Used in getHighestCommonBlock network action transactions + const blockHeightsList = computeBlockHeightsList( + finalizedHeight, + numberOfValidators, + 10, + Math.ceil(lastBlock.header.height / numberOfValidators), + ); + + const receivedBlock = await createValidDefaultBlock({ + header: { + height: lastBlock.header.height + 304, + }, + }); + + for (const expectedPeer of peersList.expectedSelection) { + const { peerId } = expectedPeer; + const blockIds = codec.encode(getHighestCommonBlockRequestSchema, { + ids: blockIdsList.map(id => id), + }); + when(networkMock.requestFromPeer) + .calledWith({ + procedure: 'getHighestCommonBlock', + peerId, + data: blockIds, + }) + .mockResolvedValue({ + data: codec.encode(getHighestCommonBlockResponseSchema, { + id: utils.getRandomBytes(37), + }), + } as never); + + when(networkMock.requestFromPeer) + .calledWith({ + procedure: 'getLastBlock', + peerId, + }) + .mockResolvedValue({ + data: receivedBlock.getBytes(), + } as never); + const encodedBlocks = requestedBlocks.map(block => block.getBytes()); + when(networkMock.requestFromPeer) + .calledWith({ + procedure: 'getBlocksFromId', + peerId, + data: codec.encode(getBlocksFromIdRequestSchema, { + blockId: highestCommonBlock.id, + }), + }) + .mockResolvedValue({ + data: codec.encode(getBlocksFromIdResponseSchema, { blocks: encodedBlocks }), + } as never); + } + + when(chainModule.dataAccess.getBlockHeadersWithHeights) + .calledWith(blockHeightsList) + .mockResolvedValue(blockList.map(b => b.header) as never); + + when(chainModule.dataAccess.getLastBlock) + .calledWith() + .mockResolvedValue(lastBlock as never); + + when(chainModule.dataAccess.getBlockHeadersByHeightBetween) + // If cache size initialization on chain changes this needs to be updated accordingly + .calledWith(1496, 2001) + .mockResolvedValue([] as never); + + // BFT loads blocks from storage and extracts their headers + when(chainModule.dataAccess.getBlockHeadersByHeightBetween) + // If cache size initialization on chain changes this needs to be updated accordingly + .calledWith(expect.any(Number), expect.any(Number)) + .mockResolvedValue([lastBlock] as never); + + await expect(blockSynchronizationMechanism.run(receivedBlock)).rejects.toThrow( + Errors.ApplyPenaltyAndRestartError, + ); + + expect(networkMock.requestFromPeer).toHaveBeenCalledTimes(3); + expect(networkMock.getConnectedPeers).toHaveBeenCalledTimes(1); + + expect( + blockSynchronizationMechanism['_requestAndApplyBlocksToCurrentChain'], + ).not.toHaveBeenCalled(); + }); + + it('should give up requesting the last common block if not found, and then ban the peer and restart the mechanism', async () => { + // Set last block to a high height + const lastBlock = await createValidDefaultBlock({ + header: { + height: genesisBlock.header.height + 2000, + }, + }); + chainModule._lastBlock = lastBlock; + // Used in getHighestCommonBlock network action transactions + const blockHeightsList = computeBlockHeightsList( + finalizedHeight, + numberOfValidators, + 10, + Math.ceil(lastBlock.header.height / numberOfValidators), + ); + + const receivedBlock = await createValidDefaultBlock({ + header: { + height: lastBlock.header.height + 304, + }, + }); + + for (const expectedPeer of peersList.expectedSelection) { + const { peerId } = expectedPeer; + const blockIds = codec.encode(getHighestCommonBlockRequestSchema, { + ids: blockIdsList.map(id => id), + }); + when(networkMock.requestFromPeer) + .calledWith({ + procedure: 'getHighestCommonBlock', + peerId, + data: blockIds, + }) + .mockResolvedValue({ + data: codec.encode(getHighestCommonBlockResponseSchema, { id: Buffer.alloc(0) }), + } as never); + + when(networkMock.requestFromPeer) + .calledWith({ + procedure: 'getLastBlock', + peerId, + }) + .mockResolvedValue({ + data: receivedBlock.getBytes(), + } as never); + const encodedBlocks = requestedBlocks.map(block => block.getBytes()); + when(networkMock.requestFromPeer) + .calledWith({ + procedure: 'getBlocksFromId', + peerId, + data: codec.encode(getBlocksFromIdRequestSchema, { + blockId: highestCommonBlock.id, + }), + }) + .mockResolvedValue({ + data: codec.encode(getBlocksFromIdResponseSchema, { blocks: encodedBlocks }), + } as never); + } + + when(chainModule.dataAccess.getBlockHeadersWithHeights) + .calledWith(blockHeightsList) + .mockResolvedValue(blockList.map(b => b.header) as never); + + when(chainModule.dataAccess.getLastBlock) + .calledWith() + .mockResolvedValue(lastBlock as never); + + when(chainModule.dataAccess.getBlockHeadersByHeightBetween) + // If cache size initialization on chain changes this needs to be updated accordingly + .calledWith(1496, 2001) + .mockResolvedValue([] as never); + + // BFT loads blocks from storage and extracts their headers + when(chainModule.dataAccess.getBlockHeadersByHeightBetween) + // If cache size initialization on chain changes this needs to be updated accordingly + .calledWith(expect.any(Number), expect.any(Number)) + .mockResolvedValue([lastBlock] as never); + + await expect(blockSynchronizationMechanism.run(receivedBlock)).rejects.toThrow( + Errors.ApplyPenaltyAndRestartError, + ); + + expect(networkMock.getConnectedPeers).toHaveBeenCalledTimes(1); + + expect( + blockSynchronizationMechanism['_requestAndApplyBlocksToCurrentChain'], + ).not.toHaveBeenCalled(); + }); + it('should ban the peer and restart the mechanism if the common block height is smaller than the finalized height', async () => { // Used in getHighestCommonBlock network action transactions const blockHeightsList = computeBlockHeightsList( diff --git a/framework/test/unit/engine/consensus/synchronizer/fast_chain_switching_mechanism/fast_chain_switching_mechanism.spec.ts b/framework/test/unit/engine/consensus/synchronizer/fast_chain_switching_mechanism/fast_chain_switching_mechanism.spec.ts index fc9d0647934..965f7ac3160 100644 --- a/framework/test/unit/engine/consensus/synchronizer/fast_chain_switching_mechanism/fast_chain_switching_mechanism.spec.ts +++ b/framework/test/unit/engine/consensus/synchronizer/fast_chain_switching_mechanism/fast_chain_switching_mechanism.spec.ts @@ -270,7 +270,7 @@ describe('fast_chain_switching_mechanism', () => { }); describe('when fail to request the common block', () => { - it('should give up after trying 10 times, apply penalty and restart the mechanism', async () => { + it('should give up after trying 10 times if there is a network error, apply penalty and restart the mechanism', async () => { // Arrange const storageReturnValue = [ { @@ -290,7 +290,7 @@ describe('fast_chain_switching_mechanism', () => { peerId: aPeerId, data: blockIds, }) - .mockResolvedValue({ + .mockRejectedValue({ data: codec.encode(getHighestCommonBlockResponseSchema, { id: Buffer.alloc(0) }), } as never); @@ -302,6 +302,74 @@ describe('fast_chain_switching_mechanism', () => { // Assert expect(networkMock.requestFromPeer).toHaveBeenCalledTimes(9); }); + + it('should give up after trying 10 times if the common block response format is invalid, apply penalty and restart the mechanism', async () => { + // Arrange + const storageReturnValue = [ + { + id: genesisBlock.header.id, + }, + { + id: chainModule.lastBlock.header.id, + }, + ]; + const blockIds = codec.encode(getHighestCommonBlockRequestSchema, { + ids: storageReturnValue.map(blocks => blocks.id), + }); + // Simulate peer not sending back a common block + when(networkMock.requestFromPeer) + .calledWith({ + procedure: 'getHighestCommonBlock', + peerId: aPeerId, + data: blockIds, + }) + .mockResolvedValue({ + data: codec.encode(getHighestCommonBlockResponseSchema, { + id: utils.getRandomBytes(37), + }), + } as never); + + // Act + await expect(fastChainSwitchingMechanism.run(aBlock, aPeerId)).rejects.toThrow( + new Errors.ApplyPenaltyAndAbortError(aPeerId, "Peer didn't return a common block"), + ); + + // Assert + expect(networkMock.requestFromPeer).toHaveBeenCalledTimes(9); + }); + + it('should give up if not found, apply penalty and restart the mechanism', async () => { + // Arrange + const storageReturnValue = [ + { + id: genesisBlock.header.id, + }, + { + id: chainModule.lastBlock.header.id, + }, + ]; + const blockIds = codec.encode(getHighestCommonBlockRequestSchema, { + ids: storageReturnValue.map(blocks => blocks.id), + }); + // Simulate peer not sending back a common block + when(networkMock.requestFromPeer) + .calledWith({ + procedure: 'getHighestCommonBlock', + peerId: aPeerId, + data: blockIds, + }) + .mockResolvedValue({ + data: codec.encode(getHighestCommonBlockResponseSchema, { id: Buffer.alloc(0) }), + } as never); + + // Act + await expect(fastChainSwitchingMechanism.run(aBlock, aPeerId)).rejects.toThrow( + new Errors.ApplyPenaltyAndAbortError(aPeerId, "Peer didn't return a common block"), + ); + + // Assert + expect(networkMock.requestFromPeer).toHaveBeenCalledTimes(1); + }); }); describe('given that the highest common block is found', () => {