Merge pull request #5897 from LiskHQ/5894_refactor_contradiction_check

Update to have headers contradicting check as pure function - Closes #5894

elements/lisk-bft/src/finality_manager.ts

@@ -20,12 +20,8 @@ import * as Debug from 'debug';
 import { EventEmitter } from 'events';
 import { dataStructures } from '@liskhq/lisk-utils';
 import { BFT_ROUND_THRESHOLD } from './constant';
-import {
-	BFTChainDisjointError,
-	BFTForkChoiceRuleError,
-	BFTInvalidAttributeError,
-	BFTLowerChainBranchError,
-} from './types';
+import { BFTInvalidAttributeError, BFTError } from './types';
+import { areHeadersContradicting } from './header_contradicting';
 
 // eslint-disable-next-line new-cap
 const debug = Debug('lisk:bft:consensus_manager');
@@ -365,42 +361,8 @@ export class FinalityManager extends EventEmitter {
 			return true;
 		}
 
-		// Order the two block headers such that earlierBlock must be forged first
-		let earlierBlock = validatorLastBlock;
-		let laterBlock = blockHeader;
-		const higherMaxHeightPreviouslyForged =
-			earlierBlock.asset.maxHeightPreviouslyForged > laterBlock.asset.maxHeightPreviouslyForged;
-		const sameMaxHeightPreviouslyForged =
-			earlierBlock.asset.maxHeightPreviouslyForged === laterBlock.asset.maxHeightPreviouslyForged;
-		const higherMaxHeightPrevoted =
-			earlierBlock.asset.maxHeightPrevoted > laterBlock.asset.maxHeightPrevoted;
-		const sameMaxHeightPrevoted =
-			earlierBlock.asset.maxHeightPrevoted === laterBlock.asset.maxHeightPrevoted;
-		const higherHeight = earlierBlock.height > laterBlock.height;
-		if (
-			higherMaxHeightPreviouslyForged ||
-			(sameMaxHeightPreviouslyForged && higherMaxHeightPrevoted) ||
-			(sameMaxHeightPreviouslyForged && sameMaxHeightPrevoted && higherHeight)
-		) {
-			[earlierBlock, laterBlock] = [laterBlock, earlierBlock];
-		}
-
-		if (
-			earlierBlock.asset.maxHeightPrevoted === laterBlock.asset.maxHeightPrevoted &&
-			earlierBlock.height >= laterBlock.height
-		) {
-			/* Violation of the fork choice rule as validator moved to different chain
-			 without strictly larger maxHeightPreviouslyForged or larger height as
-			 justification. This in particular happens, if a validator is double forging. */
-			throw new BFTForkChoiceRuleError();
-		}
-
-		if (earlierBlock.height > laterBlock.asset.maxHeightPreviouslyForged) {
-			throw new BFTChainDisjointError();
-		}
-
-		if (earlierBlock.asset.maxHeightPrevoted > laterBlock.asset.maxHeightPrevoted) {
-			throw new BFTLowerChainBranchError();
+		if (areHeadersContradicting(validatorLastBlock, blockHeader)) {
+			throw new BFTError();
 		}
 
 		return true;

elements/lisk-bft/src/header_contradicting.ts

@@ -0,0 +1,62 @@
+/*
+ * Copyright © 2020 Lisk Foundation
+ *
+ * See the LICENSE file at the top-level directory of this distribution
+ * for licensing information.
+ *
+ * Unless otherwise agreed in a custom licensing agreement with the Lisk Foundation,
+ * no part of this software, including this file, may be copied, modified,
+ * propagated, or distributed except according to the terms contained in the
+ * LICENSE file.
+ *
+ * Removal or modification of this copyright notice is prohibited.
+ */
+
+import { BlockHeader } from '@liskhq/lisk-chain';
+
+export const areHeadersContradicting = (b1: BlockHeader, b2: BlockHeader): boolean => {
+	let earlierBlock = b1;
+	let laterBlock = b2;
+	const higherMaxHeightPreviouslyForged =
+		earlierBlock.asset.maxHeightPreviouslyForged > laterBlock.asset.maxHeightPreviouslyForged;
+	const sameMaxHeightPreviouslyForged =
+		earlierBlock.asset.maxHeightPreviouslyForged === laterBlock.asset.maxHeightPreviouslyForged;
+	const higherMaxHeightPrevoted =
+		earlierBlock.asset.maxHeightPrevoted > laterBlock.asset.maxHeightPrevoted;
+	const sameMaxHeightPrevoted =
+		earlierBlock.asset.maxHeightPrevoted === laterBlock.asset.maxHeightPrevoted;
+	const higherHeight = earlierBlock.height > laterBlock.height;
+	if (
+		higherMaxHeightPreviouslyForged ||
+		(sameMaxHeightPreviouslyForged && higherMaxHeightPrevoted) ||
+		(sameMaxHeightPreviouslyForged && sameMaxHeightPrevoted && higherHeight)
+	) {
+		[earlierBlock, laterBlock] = [laterBlock, earlierBlock];
+	}
+	// Blocks by different delegates are never contradicting
+	if (!earlierBlock.generatorPublicKey.equals(laterBlock.generatorPublicKey)) {
+		return false;
+	}
+	// No contradiction, as block headers are the same
+	if (earlierBlock.id.equals(laterBlock.id)) {
+		return false;
+	}
+	if (
+		earlierBlock.asset.maxHeightPrevoted === laterBlock.asset.maxHeightPrevoted &&
+		earlierBlock.height >= laterBlock.height
+	) {
+		/* Violation of the fork choice rule as validator moved to different chain
+		 without strictly larger maxHeightPreviouslyForged or larger height as
+		 justification. This in particular happens, if a validator is double forging. */
+		return true;
+	}
+
+	if (earlierBlock.height > laterBlock.asset.maxHeightPreviouslyForged) {
+		return true;
+	}
+
+	if (earlierBlock.asset.maxHeightPrevoted > laterBlock.asset.maxHeightPrevoted) {
+		return true;
+	}
+	return false;
+};

elements/lisk-bft/src/index.ts

@@ -15,3 +15,4 @@
 export * from './bft';
 export * from './fork_choice_rule';
 export * from './types';
+export { areHeadersContradicting } from './header_contradicting';

elements/lisk-bft/src/types.ts

@@ -26,28 +26,6 @@ export enum ForkStatus {
 
 export class BFTError extends Error {}
 
-export class BFTChainDisjointError extends BFTError {
-	public constructor() {
-		super(
-			'Violation of disjointedness condition. If delegate forged a block of higher height earlier and later the block with lower height',
-		);
-	}
-}
-
-export class BFTLowerChainBranchError extends BFTError {
-	public constructor() {
-		super(
-			'Violation of the condition that delegate must choose the branch with largest maxHeightPrevoted',
-		);
-	}
-}
-
-export class BFTForkChoiceRuleError extends BFTError {
-	public constructor() {
-		super('Violation of fork choice rule, delegate moved to a different chain');
-	}
-}
-
 export class BFTInvalidAttributeError extends BFTError {}
 
 export interface BFTPersistedValues {

elements/lisk-bft/test/unit/finality_manager.spec.ts

@@ -28,11 +28,7 @@ import {
 	CONSENSUS_STATE_VALIDATOR_LEDGER_KEY,
 	BFTVotingLedgerSchema,
 } from '../../src/finality_manager';
-import {
-	BFTChainDisjointError,
-	BFTForkChoiceRuleError,
-	BFTLowerChainBranchError,
-} from '../../src/types';
+import { BFTError } from '../../src/types';
 import { createFakeBlockHeader } from '../fixtures/blocks';
 import { StateStoreMock } from '../utils/state_store_mock';
 import { BFTFinalizedHeightCodecSchema } from '../../src';
@@ -202,7 +198,7 @@ describe('finality_manager', () => {
 				) as unknown) as StateStore;
 
 				await expect(finalityManager.verifyBlockHeaders(currentBlock, stateStore)).rejects.toThrow(
-					BFTForkChoiceRuleError,
+					BFTError,
 				);
 			});
 
@@ -230,7 +226,7 @@ describe('finality_manager', () => {
 				) as unknown) as StateStore;
 
 				await expect(finalityManager.verifyBlockHeaders(currentBlock, stateStore)).rejects.toThrow(
-					BFTForkChoiceRuleError,
+					BFTError,
 				);
 			});
 
@@ -254,7 +250,7 @@ describe('finality_manager', () => {
 				) as unknown) as StateStore;
 
 				await expect(finalityManager.verifyBlockHeaders(currentBlock, stateStore)).rejects.toThrow(
-					BFTChainDisjointError,
+					BFTError,
 				);
 			});
 
@@ -283,7 +279,7 @@ describe('finality_manager', () => {
 				) as unknown) as StateStore;
 
 				await expect(finalityManager.verifyBlockHeaders(currentBlock, stateStore)).rejects.toThrow(
-					BFTLowerChainBranchError,
+					BFTError,
 				);
 			});
 

framework/src/modules/dpos/transaction_assets/pom_transaction_asset.ts

@@ -14,6 +14,7 @@
 
 import { BlockHeader } from '@liskhq/lisk-chain';
 import { getAddressFromPublicKey } from '@liskhq/lisk-cryptography';
+import { areHeadersContradicting } from '@liskhq/lisk-bft';
 import { codec } from '@liskhq/lisk-codec';
 import { BaseAsset } from '../../base_asset';
 import { ApplyAssetContext, ValidateAssetContext } from '../../../types';
@@ -109,34 +110,8 @@ export class PomTransactionAsset extends BaseAsset<PomTransactionAssetContext> {
 			throw new Error('BlockHeaders are identical. No contradiction detected.');
 		}
 
-		/*
-			Check for BFT violations:
-					1. Double forging
-					2. Disjointedness
-					3. Branch is not the one with largest maxHeightPrevoted
-		*/
-
-		let b1 = asset.header1;
-		let b2 = asset.header2;
-
-		// Order the two block headers such that b1 must be forged first
-		if (
-			b1.asset.maxHeightPreviouslyForged > b2.asset.maxHeightPreviouslyForged ||
-			(b1.asset.maxHeightPreviouslyForged === b2.asset.maxHeightPreviouslyForged &&
-				b1.asset.maxHeightPrevoted > b2.asset.maxHeightPrevoted) ||
-			(b1.asset.maxHeightPreviouslyForged === b2.asset.maxHeightPreviouslyForged &&
-				b1.asset.maxHeightPrevoted === b2.asset.maxHeightPrevoted &&
-				b1.height > b2.height)
-		) {
-			b1 = asset.header2;
-			b2 = asset.header1;
-		}
-
-		if (
-			!(b1.asset.maxHeightPrevoted === b2.asset.maxHeightPrevoted && b1.height >= b2.height) &&
-			!(b1.height > b2.asset.maxHeightPreviouslyForged) &&
-			!(b1.asset.maxHeightPrevoted > b2.asset.maxHeightPrevoted)
-		) {
+		// Check for BFT violations:
+		if (!areHeadersContradicting(asset.header1, asset.header2)) {
 			throw new Error('BlockHeaders are not contradicting as per BFT violation rules.');
 		}
 	}