Repository: paritytech/substrate
File: client/service/test/src/client/mod.rs (Rust)

Code before:

		// build B'[1]
		let mut block_not_ok = client
			.new_block_at(client.chain_info().genesis_hash, Default::default(), false)
			.unwrap();
		block_not_ok.push_storage_change(vec![0], Some(vec![1])).unwrap();
		let block_not_ok = block_not_ok.build().unwrap().block;

		let params = BlockCheckParams {
			hash: block_not_ok.hash(),
			number: 1,
			parent_hash: *block_not_ok.header().parent_hash(),
			allow_missing_state: false,
			allow_missing_parent: false,
			import_existing: false,
		};
		if record_only {
			known_bad.insert(block_not_ok.hash());
		} else {
			assert_eq!(block_on(client.check_block(params)).unwrap(), ImportResult::KnownBad);
		}

		// Now going to the fork
		block_on(client.import_as_final(BlockOrigin::Own, block_ok)).unwrap();

		// And check good fork (build B[2])
		let mut block_ok = client.new_block_at(block_ok_1_hash, Default::default(), false).unwrap();
		block_ok.push_storage_change(vec![0], Some(vec![2])).unwrap();
		let block_ok = block_ok.build().unwrap().block;
		assert_eq!(*block_ok.header().number(), 2);

		let params = BlockCheckParams {
			hash: block_ok.hash(),
			number: 2,
			parent_hash: *block_ok.header().parent_hash(),
			allow_missing_state: false,
			allow_missing_parent: false,
			import_existing: false,
		};
		if record_only {
			fork_rules.push((2, block_ok.hash()));
		}
		assert_eq!(block_on(client.check_block(params)).unwrap(), ImportResult::imported(false));

		// And now try bad fork (build B'[2])
		let mut block_not_ok =
			client.new_block_at(block_ok_1_hash, Default::default(), false).unwrap();
		block_not_ok.push_storage_change(vec![0], Some(vec![3])).unwrap();
		let block_not_ok = block_not_ok.build().unwrap().block;
		assert_eq!(*block_not_ok.header().number(), 2);


Review comment (suggestion):
_note:_ Checking fork at block height 2 is more readable and less confusing, this is inline with actual block number, and also with params passed to `check_block`.

Code after:
		};
		if record_only {
			known_bad.insert(block_not_ok.hash());
		} else {
			assert_eq!(block_on(client.check_block(params)).unwrap(), ImportResult::KnownBad);
		}

		// Now going to the fork
		block_on(client.import_as_final(BlockOrigin::Own, block_ok)).unwrap();

		// And check good fork (build B[2])
		let mut block_ok = client.new_block_at(block_ok_1_hash, Default::default(), false).unwrap();
		block_ok.push_storage_change(vec![0], Some(vec![2])).unwrap();
		let block_ok = block_ok.build().unwrap().block;
		assert_eq!(*block_ok.header().number(), 2);

		let params = BlockCheckParams {
			hash: block_ok.hash(),
			number: 2,
			parent_hash: *block_ok.header().parent_hash(),
			allow_missing_state: false,
			allow_missing_parent: false,
			import_existing: false,
		};
		if record_only {
			fork_rules.push((2, block_ok.hash()));
		}
		assert_eq!(block_on(client.check_block(params)).unwrap(), ImportResult::imported(false));

		// And now try bad fork (build B'[2])
		let mut block_not_ok =
			client.new_block_at(block_ok_1_hash, Default::default(), false).unwrap();
		block_not_ok.push_storage_change(vec![0], Some(vec![3])).unwrap();
		let block_not_ok = block_not_ok.build().unwrap().block;
		assert_eq!(*block_not_ok.header().number(), 2);

		let params = BlockCheckParams {
			hash: block_not_ok.hash(),
			number: 2,
			parent_hash: *block_not_ok.header().parent_hash(),
			allow_missing_state: false,
			allow_missing_parent: false,
			import_existing: false,
		};

		if !record_only {
			assert_eq!(block_on(client.check_block(params)).unwrap(), ImportResult::KnownBad);
		}
	}

	let mut known_bad = HashSet::new();