From 7d5b7f67a0d96449c3cbb0d7306d1b82463d3de5 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Tue, 18 Oct 2022 14:05:34 +0200 Subject: [PATCH] BlockId removal: refactor: Finalizer It changes the arguments of methods of `Finalizer` trait from: block: `BlockId` to: hash: `&Block::Hash` This PR is part of BlockId::Number refactoring analysis (paritytech/substrate#11292) --- client/api/src/backend.rs | 4 +- client/beefy/src/tests.rs | 75 ++++++++++++------- client/beefy/src/worker.rs | 8 +- client/consensus/babe/src/tests.rs | 10 +-- .../manual-seal/src/finalize_block.rs | 4 +- client/finality-grandpa/src/environment.rs | 2 +- client/finality-grandpa/src/finality_proof.rs | 5 +- client/finality-grandpa/src/tests.rs | 14 +++- client/finality-grandpa/src/warp_proof.rs | 7 +- client/network/sync/src/lib.rs | 8 +- client/network/test/src/lib.rs | 6 +- client/network/test/src/sync.rs | 42 ++++++----- client/rpc/src/chain/tests.rs | 8 +- client/service/src/client/client.rs | 23 ++---- client/service/test/src/client/mod.rs | 12 +-- test-utils/client/src/client_ext.rs | 8 +- 16 files changed, 128 insertions(+), 108 deletions(-) diff --git a/client/api/src/backend.rs b/client/api/src/backend.rs index f358385acd708..5b4acb0be8bda 100644 --- a/client/api/src/backend.rs +++ b/client/api/src/backend.rs @@ -252,7 +252,7 @@ pub trait Finalizer> { fn apply_finality( &self, operation: &mut ClientImportOperation, - id: BlockId, + block: &Block::Hash, justification: Option, notify: bool, ) -> sp_blockchain::Result<()>; @@ -272,7 +272,7 @@ pub trait Finalizer> { /// while performing major synchronization work. fn finalize_block( &self, - id: BlockId, + block: &Block::Hash, justification: Option, notify: bool, ) -> sp_blockchain::Result<()>; diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index 89be1cac4f886..4509a36d4757b 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -20,6 +20,7 @@ use futures::{future, stream::FuturesUnordered, Future, StreamExt}; use parking_lot::Mutex; +use sc_client_api::backend::Backend; use serde::{Deserialize, Serialize}; use std::{collections::HashMap, marker::PhantomData, sync::Arc, task::Poll}; use tokio::{runtime::Runtime, time::Duration}; @@ -509,13 +510,19 @@ fn finalize_block_and_wait_for_beefy( let (best_blocks, versioned_finality_proof) = get_beefy_streams(&mut net.lock(), peers.clone()); for block in finalize_targets { - let finalize = BlockId::number(*block); + let finalize = net + .lock() + .peer(0) + .client() + .as_client() + .expect_block_hash_from_id(&BlockId::number(*block)) + .unwrap(); peers.clone().for_each(|(index, _)| { net.lock() .peer(index) .client() .as_client() - .finalize_block(finalize, None) + .finalize_block(&finalize, None) .unwrap(); }) } @@ -604,9 +611,16 @@ fn lagging_validators() { ); // Alice finalizes #25, Bob lags behind - let finalize = BlockId::number(25); + let finalize = net + .lock() + .peer(0) + .client() + .as_client() + .block_hash_from_id(&BlockId::number(25)) + .unwrap() + .unwrap(); let (best_blocks, versioned_finality_proof) = get_beefy_streams(&mut net.lock(), peers.clone()); - net.lock().peer(0).client().as_client().finalize_block(finalize, None).unwrap(); + net.lock().peer(0).client().as_client().finalize_block(&finalize, None).unwrap(); // verify nothing gets finalized by BEEFY let timeout = Some(Duration::from_millis(250)); streams_empty_after_timeout(best_blocks, &net, &mut runtime, timeout); @@ -614,7 +628,7 @@ fn lagging_validators() { // Bob catches up and also finalizes #25 let (best_blocks, versioned_finality_proof) = get_beefy_streams(&mut net.lock(), peers.clone()); - net.lock().peer(1).client().as_client().finalize_block(finalize, None).unwrap(); + net.lock().peer(1).client().as_client().finalize_block(&finalize, None).unwrap(); // expected beefy finalizes block #17 from diff-power-of-two wait_for_best_beefy_blocks(best_blocks, &net, &mut runtime, &[23, 24, 25]); wait_for_beefy_signed_commitments(versioned_finality_proof, &net, &mut runtime, &[23, 24, 25]); @@ -628,8 +642,15 @@ fn lagging_validators() { // Alice finalizes session-boundary mandatory block #60, Bob lags behind let (best_blocks, versioned_finality_proof) = get_beefy_streams(&mut net.lock(), peers.clone()); - let finalize = BlockId::number(60); - net.lock().peer(0).client().as_client().finalize_block(finalize, None).unwrap(); + let finalize = net + .lock() + .peer(0) + .client() + .as_client() + .block_hash_from_id(&BlockId::number(60)) + .unwrap() + .unwrap(); + net.lock().peer(0).client().as_client().finalize_block(&finalize, None).unwrap(); // verify nothing gets finalized by BEEFY let timeout = Some(Duration::from_millis(250)); streams_empty_after_timeout(best_blocks, &net, &mut runtime, timeout); @@ -637,7 +658,7 @@ fn lagging_validators() { // Bob catches up and also finalizes #60 (and should have buffered Alice's vote on #60) let (best_blocks, versioned_finality_proof) = get_beefy_streams(&mut net.lock(), peers); - net.lock().peer(1).client().as_client().finalize_block(finalize, None).unwrap(); + net.lock().peer(1).client().as_client().finalize_block(&finalize, None).unwrap(); // verify beefy skips intermediary votes, and successfully finalizes mandatory block #60 wait_for_best_beefy_blocks(best_blocks, &net, &mut runtime, &[60]); wait_for_beefy_signed_commitments(versioned_finality_proof, &net, &mut runtime, &[60]); @@ -681,24 +702,17 @@ fn correct_beefy_payload() { get_beefy_streams(&mut net.lock(), [(0, BeefyKeyring::Alice)].into_iter()); // now 2 good validators and 1 bad one are voting - net.lock() + let hashof11 = net + .lock() .peer(0) .client() .as_client() - .finalize_block(BlockId::number(11), None) - .unwrap(); - net.lock() - .peer(1) - .client() - .as_client() - .finalize_block(BlockId::number(11), None) - .unwrap(); - net.lock() - .peer(3) - .client() - .as_client() - .finalize_block(BlockId::number(11), None) + .block_hash_from_id(&BlockId::number(11)) + .unwrap() .unwrap(); + net.lock().peer(0).client().as_client().finalize_block(&hashof11, None).unwrap(); + net.lock().peer(1).client().as_client().finalize_block(&hashof11, None).unwrap(); + net.lock().peer(3).client().as_client().finalize_block(&hashof11, None).unwrap(); // verify consensus is _not_ reached let timeout = Some(Duration::from_millis(250)); @@ -708,12 +722,7 @@ fn correct_beefy_payload() { // 3rd good validator catches up and votes as well let (best_blocks, versioned_finality_proof) = get_beefy_streams(&mut net.lock(), [(0, BeefyKeyring::Alice)].into_iter()); - net.lock() - .peer(2) - .client() - .as_client() - .finalize_block(BlockId::number(11), None) - .unwrap(); + net.lock().peer(2).client().as_client().finalize_block(&hashof11, None).unwrap(); // verify consensus is reached wait_for_best_beefy_blocks(best_blocks, &net, &mut runtime, &[11]); @@ -923,11 +932,19 @@ fn on_demand_beefy_justification_sync() { let (dave_best_blocks, _) = get_beefy_streams(&mut net.lock(), [(dave_index, BeefyKeyring::Dave)].into_iter()); + let hashof1 = net + .lock() + .peer(dave_index) + .client() + .as_client() + .block_hash_from_id(&BlockId::number(1)) + .unwrap() + .unwrap(); net.lock() .peer(dave_index) .client() .as_client() - .finalize_block(BlockId::number(1), None) + .finalize_block(&hashof1, None) .unwrap(); // Give Dave task some cpu cycles to process the finality notification, run_for(Duration::from_millis(100), &net, &mut runtime); diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 4381081f74ebd..6efebd131d6da 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -1506,11 +1506,9 @@ pub(crate) mod tests { // push 15 blocks with `AuthorityChange` digests every 10 blocks net.generate_blocks_and_sync(15, 10, &validator_set, false); // finalize 13 without justifications - net.peer(0) - .client() - .as_client() - .finalize_block(BlockId::number(13), None) - .unwrap(); + let hashof13 = + backend.blockchain().expect_block_hash_from_id(&BlockId::Number(13)).unwrap(); + net.peer(0).client().as_client().finalize_block(&hashof13, None).unwrap(); // Test initialization at session boundary. { diff --git a/client/consensus/babe/src/tests.rs b/client/consensus/babe/src/tests.rs index 58f5e7b8eb6d4..24185dbce6795 100644 --- a/client/consensus/babe/src/tests.rs +++ b/client/consensus/babe/src/tests.rs @@ -820,7 +820,7 @@ fn revert_not_allowed_for_finalized() { let canon = propose_and_import_blocks_wrap(BlockId::Number(0), 3); // Finalize best block - client.finalize_block(BlockId::Hash(canon[2]), None, false).unwrap(); + client.finalize_block(&canon[2], None, false).unwrap(); // Revert canon chain to last finalized block revert(client.clone(), backend, 100).expect("revert should work for baked test scenario"); @@ -882,7 +882,7 @@ fn importing_epoch_change_block_prunes_tree() { // We finalize block #13 from the canon chain, so on the next epoch // change the tree should be pruned, to not contain F (#7). - client.finalize_block(BlockId::Hash(canon_hashes[12]), None, false).unwrap(); + client.finalize_block(&canon_hashes[12], None, false).unwrap(); propose_and_import_blocks_wrap(BlockId::Hash(client.chain_info().best_hash), 7); // at this point no hashes from the first fork must exist on the tree @@ -909,7 +909,7 @@ fn importing_epoch_change_block_prunes_tree() { .any(|h| fork_3.contains(h)),); // finalizing block #25 from the canon chain should prune out the second fork - client.finalize_block(BlockId::Hash(canon_hashes[24]), None, false).unwrap(); + client.finalize_block(&canon_hashes[24], None, false).unwrap(); propose_and_import_blocks_wrap(BlockId::Hash(client.chain_info().best_hash), 8); // at this point no hashes from the second fork must exist on the tree @@ -1049,7 +1049,7 @@ fn obsolete_blocks_aux_data_cleanup() { assert!(aux_data_check(&fork3_hashes, true)); // Finalize A3 - client.finalize_block(BlockId::Number(3), None, true).unwrap(); + client.finalize_block(&fork1_hashes[2], None, true).unwrap(); // Wiped: A1, A2 assert!(aux_data_check(&fork1_hashes[..2], false)); @@ -1060,7 +1060,7 @@ fn obsolete_blocks_aux_data_cleanup() { // Present C4, C5 assert!(aux_data_check(&fork3_hashes, true)); - client.finalize_block(BlockId::Number(4), None, true).unwrap(); + client.finalize_block(&fork1_hashes[3], None, true).unwrap(); // Wiped: A3 assert!(aux_data_check(&fork1_hashes[2..3], false)); diff --git a/client/consensus/manual-seal/src/finalize_block.rs b/client/consensus/manual-seal/src/finalize_block.rs index d134ce7734571..e11353e2da611 100644 --- a/client/consensus/manual-seal/src/finalize_block.rs +++ b/client/consensus/manual-seal/src/finalize_block.rs @@ -20,7 +20,7 @@ use crate::rpc; use sc_client_api::backend::{Backend as ClientBackend, Finalizer}; -use sp_runtime::{generic::BlockId, traits::Block as BlockT, Justification}; +use sp_runtime::{traits::Block as BlockT, Justification}; use std::{marker::PhantomData, sync::Arc}; /// params for block finalization. @@ -46,7 +46,7 @@ where { let FinalizeBlockParams { hash, mut sender, justification, finalizer, .. } = params; - match finalizer.finalize_block(BlockId::Hash(hash), justification, true) { + match finalizer.finalize_block(&hash, justification, true) { Err(e) => { log::warn!("Failed to finalize block {}", e); rpc::send_result(&mut sender, Err(e.into())) diff --git a/client/finality-grandpa/src/environment.rs b/client/finality-grandpa/src/environment.rs index 3d708a95f41cb..60720494a9f9a 100644 --- a/client/finality-grandpa/src/environment.rs +++ b/client/finality-grandpa/src/environment.rs @@ -1352,7 +1352,7 @@ where // ideally some handle to a synchronization oracle would be used // to avoid unconditionally notifying. client - .apply_finality(import_op, BlockId::Hash(hash), persisted_justification, true) + .apply_finality(import_op, &hash, persisted_justification, true) .map_err(|e| { warn!(target: "afg", "Error applying finality to block {:?}: {}", (hash, number), e); e diff --git a/client/finality-grandpa/src/finality_proof.rs b/client/finality-grandpa/src/finality_proof.rs index e7578fa669463..a7042e26b1a71 100644 --- a/client/finality-grandpa/src/finality_proof.rs +++ b/client/finality-grandpa/src/finality_proof.rs @@ -309,7 +309,8 @@ mod tests { } for block in to_finalize { - client.finalize_block(BlockId::Number(*block), None).unwrap(); + let hash = blocks[*block as usize - 1].hash(); + client.finalize_block(&hash, None).unwrap(); } (client, backend, blocks) } @@ -489,7 +490,7 @@ mod tests { let grandpa_just8 = GrandpaJustification::from_commit(&client, round, commit).unwrap(); client - .finalize_block(BlockId::Number(8), Some((ID, grandpa_just8.encode().clone()))) + .finalize_block(&block8.hash(), Some((ID, grandpa_just8.encode().clone()))) .unwrap(); // Authority set change at block 8, so the justification stored there will be used in the diff --git a/client/finality-grandpa/src/tests.rs b/client/finality-grandpa/src/tests.rs index b1e46be5cabde..c04754411af1a 100644 --- a/client/finality-grandpa/src/tests.rs +++ b/client/finality-grandpa/src/tests.rs @@ -1457,7 +1457,12 @@ fn grandpa_environment_respects_voting_rules() { ); // we finalize block 19 with block 21 being the best block - peer.client().finalize_block(BlockId::Number(19), None, false).unwrap(); + let hashof19 = peer + .client() + .as_client() + .expect_block_hash_from_id(&BlockId::Number(19)) + .unwrap(); + peer.client().finalize_block(&hashof19, None, false).unwrap(); // the 3/4 environment should propose block 21 for voting assert_eq!( @@ -1479,7 +1484,12 @@ fn grandpa_environment_respects_voting_rules() { ); // we finalize block 21 with block 21 being the best block - peer.client().finalize_block(BlockId::Number(21), None, false).unwrap(); + let hashof21 = peer + .client() + .as_client() + .expect_block_hash_from_id(&BlockId::Number(21)) + .unwrap(); + peer.client().finalize_block(&hashof21, None, false).unwrap(); // even though the default environment will always try to not vote on the // best block, there's a hard rule that we can't cast any votes lower than diff --git a/client/finality-grandpa/src/warp_proof.rs b/client/finality-grandpa/src/warp_proof.rs index a31a0a8b91908..10d02f790a0dc 100644 --- a/client/finality-grandpa/src/warp_proof.rs +++ b/client/finality-grandpa/src/warp_proof.rs @@ -327,7 +327,7 @@ mod tests { use sp_consensus::BlockOrigin; use sp_finality_grandpa::GRANDPA_ENGINE_ID; use sp_keyring::Ed25519Keyring; - use sp_runtime::{generic::BlockId, traits::Header as _}; + use sp_runtime::traits::Header as _; use std::sync::Arc; use substrate_test_runtime_client::{ ClientBlockImportExt, ClientExt, DefaultTestClientBuilderExt, TestClientBuilder, @@ -412,10 +412,7 @@ mod tests { let justification = GrandpaJustification::from_commit(&client, 42, commit).unwrap(); client - .finalize_block( - BlockId::Hash(target_hash), - Some((GRANDPA_ENGINE_ID, justification.encode())), - ) + .finalize_block(&target_hash, Some((GRANDPA_ENGINE_ID, justification.encode()))) .unwrap(); authority_set_changes.push((current_set_id, n)); diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index e9c2b24b2ba95..76d7d624be523 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -3172,9 +3172,7 @@ mod test { let finalized_block = blocks[MAX_BLOCKS_TO_LOOK_BACKWARDS as usize * 2 - 1].clone(); let just = (*b"TEST", Vec::new()); - client - .finalize_block(BlockId::Hash(finalized_block.hash()), Some(just)) - .unwrap(); + client.finalize_block(&finalized_block.hash(), Some(just)).unwrap(); sync.update_chain_info(&info.best_hash, info.best_number); let peer_id1 = PeerId::random(); @@ -3303,9 +3301,7 @@ mod test { let finalized_block = blocks[MAX_BLOCKS_TO_LOOK_BACKWARDS as usize * 2 - 1].clone(); let just = (*b"TEST", Vec::new()); - client - .finalize_block(BlockId::Hash(finalized_block.hash()), Some(just)) - .unwrap(); + client.finalize_block(&finalized_block.hash(), Some(just)).unwrap(); sync.update_chain_info(&info.best_hash, info.best_number); let peer_id1 = PeerId::random(); diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index d4bee77b54aff..5460cc7d52461 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -190,11 +190,11 @@ impl PeersClient { pub fn finalize_block( &self, - id: BlockId, + hash: &::Hash, justification: Option, notify: bool, ) -> ClientResult<()> { - self.client.finalize_block(id, justification, notify) + self.client.finalize_block(hash, justification, notify) } } @@ -1113,7 +1113,7 @@ impl JustificationImport for ForceFinalized { justification: Justification, ) -> Result<(), Self::Error> { self.0 - .finalize_block(BlockId::Hash(hash), Some(justification), true) + .finalize_block(&hash, Some(justification), true) .map_err(|_| ConsensusError::InvalidJustification) } } diff --git a/client/network/test/src/sync.rs b/client/network/test/src/sync.rs index 6115e0e3b2c86..399062a88d269 100644 --- a/client/network/test/src/sync.rs +++ b/client/network/test/src/sync.rs @@ -251,18 +251,22 @@ fn sync_justifications() { assert_eq!(net.peer(1).client().justifications(&BlockId::Number(10)).unwrap(), None); // we finalize block #10, #15 and #20 for peer 0 with a justification + let backend = net.peer(0).client().as_backend(); + let hashof10 = backend.blockchain().expect_block_hash_from_id(&BlockId::Number(10)).unwrap(); + let hashof15 = backend.blockchain().expect_block_hash_from_id(&BlockId::Number(15)).unwrap(); + let hashof20 = backend.blockchain().expect_block_hash_from_id(&BlockId::Number(20)).unwrap(); let just = (*b"FRNK", Vec::new()); net.peer(0) .client() - .finalize_block(BlockId::Number(10), Some(just.clone()), true) + .finalize_block(&hashof10, Some(just.clone()), true) .unwrap(); net.peer(0) .client() - .finalize_block(BlockId::Number(15), Some(just.clone()), true) + .finalize_block(&hashof15, Some(just.clone()), true) .unwrap(); net.peer(0) .client() - .finalize_block(BlockId::Number(20), Some(just.clone()), true) + .finalize_block(&hashof20, Some(just.clone()), true) .unwrap(); let h1 = net.peer(1).client().header(&BlockId::Number(10)).unwrap().unwrap(); @@ -309,10 +313,7 @@ fn sync_justifications_across_forks() { net.block_until_sync(); let just = (*b"FRNK", Vec::new()); - net.peer(0) - .client() - .finalize_block(BlockId::Hash(f1_best), Some(just), true) - .unwrap(); + net.peer(0).client().finalize_block(&f1_best, Some(just), true).unwrap(); net.peer(1).request_justification(&f1_best, 10); net.peer(1).request_justification(&f2_best, 11); @@ -655,14 +656,8 @@ fn can_sync_to_peers_with_wrong_common_block() { // both peers re-org to the same fork without notifying each other let just = Some((*b"FRNK", Vec::new())); - net.peer(0) - .client() - .finalize_block(BlockId::Hash(fork_hash), just.clone(), true) - .unwrap(); - net.peer(1) - .client() - .finalize_block(BlockId::Hash(fork_hash), just, true) - .unwrap(); + net.peer(0).client().finalize_block(&fork_hash, just.clone(), true).unwrap(); + net.peer(1).client().finalize_block(&fork_hash, just, true).unwrap(); let final_hash = net.peer(0).push_blocks(1, false); net.block_until_sync(); @@ -976,10 +971,17 @@ fn multiple_requests_are_accepted_as_long_as_they_are_not_fulfilled() { assert_eq!(1, net.peer(0).num_peers()); } + let hashof10 = net + .peer(0) + .client() + .as_backend() + .blockchain() + .expect_block_hash_from_id(&BlockId::Number(10)) + .unwrap(); // Finalize the block and make the justification available. net.peer(0) .client() - .finalize_block(BlockId::Number(10), Some((*b"FRNK", Vec::new())), true) + .finalize_block(&hashof10, Some((*b"FRNK", Vec::new())), true) .unwrap(); block_on(futures::future::poll_fn::<(), _>(|cx| { @@ -1100,10 +1102,14 @@ fn syncs_state() { assert!(!net.peer(1).client().has_state_at(&BlockId::Number(64))); let just = (*b"FRNK", Vec::new()); - net.peer(1) + let hashof60 = net + .peer(0) .client() - .finalize_block(BlockId::Number(60), Some(just), true) + .as_backend() + .blockchain() + .expect_block_hash_from_id(&BlockId::Number(60)) .unwrap(); + net.peer(1).client().finalize_block(&hashof60, Some(just), true).unwrap(); // Wait for state sync. block_on(futures::future::poll_fn::<(), _>(|cx| { net.poll(cx); diff --git a/client/rpc/src/chain/tests.rs b/client/rpc/src/chain/tests.rs index 7d12458511cfd..1e2d6932a56de 100644 --- a/client/rpc/src/chain/tests.rs +++ b/client/rpc/src/chain/tests.rs @@ -199,15 +199,16 @@ async fn should_return_finalized_hash() { // import new block let block = client.new_block(Default::default()).unwrap().build().unwrap().block; client.import(BlockOrigin::Own, block).await.unwrap(); + let block_hash = client.block_hash(1).unwrap().unwrap(); // no finalization yet let res: H256 = api.call("chain_getFinalizedHead", EmptyParams::new()).await.unwrap(); assert_eq!(res, client.genesis_hash()); // finalize - client.finalize_block(BlockId::number(1), None).unwrap(); + client.finalize_block(&block_hash, None).unwrap(); let res: H256 = api.call("chain_getFinalizedHead", EmptyParams::new()).await.unwrap(); - assert_eq!(res, client.block_hash(1).unwrap().unwrap()); + assert_eq!(res, block_hash); } #[tokio::test] @@ -232,8 +233,9 @@ async fn test_head_subscription(method: &str) { let api = new_full(client.clone(), test_executor()).into_rpc(); let sub = api.subscribe(method, EmptyParams::new()).await.unwrap(); let block = client.new_block(Default::default()).unwrap().build().unwrap().block; + let block_hash = client.block_hash(1).unwrap().unwrap(); client.import(BlockOrigin::Own, block).await.unwrap(); - client.finalize_block(BlockId::number(1), None).unwrap(); + client.finalize_block(&block_hash, None).unwrap(); sub }; diff --git a/client/service/src/client/client.rs b/client/service/src/client/client.rs index a12b7177db47c..5d73ef4911dcb 100644 --- a/client/service/src/client/client.rs +++ b/client/service/src/client/client.rs @@ -1845,29 +1845,22 @@ where fn apply_finality( &self, operation: &mut ClientImportOperation, - id: BlockId, + hash: &Block::Hash, justification: Option, notify: bool, ) -> sp_blockchain::Result<()> { let last_best = self.backend.blockchain().info().best_hash; - let to_finalize_hash = self.backend.blockchain().expect_block_hash_from_id(&id)?; - self.apply_finality_with_block_hash( - operation, - to_finalize_hash, - justification, - last_best, - notify, - ) + self.apply_finality_with_block_hash(operation, *hash, justification, last_best, notify) } fn finalize_block( &self, - id: BlockId, + hash: &Block::Hash, justification: Option, notify: bool, ) -> sp_blockchain::Result<()> { self.lock_import_and_run(|operation| { - self.apply_finality(operation, id, justification, notify) + self.apply_finality(operation, hash, justification, notify) }) } } @@ -1881,20 +1874,20 @@ where fn apply_finality( &self, operation: &mut ClientImportOperation, - id: BlockId, + hash: &Block::Hash, justification: Option, notify: bool, ) -> sp_blockchain::Result<()> { - (**self).apply_finality(operation, id, justification, notify) + (**self).apply_finality(operation, hash, justification, notify) } fn finalize_block( &self, - id: BlockId, + hash: &Block::Hash, justification: Option, notify: bool, ) -> sp_blockchain::Result<()> { - (**self).finalize_block(id, justification, notify) + (**self).finalize_block(hash, justification, notify) } } diff --git a/client/service/test/src/client/mod.rs b/client/service/test/src/client/mod.rs index 4a5b56bd14006..9937c436a33ff 100644 --- a/client/service/test/src/client/mod.rs +++ b/client/service/test/src/client/mod.rs @@ -867,7 +867,7 @@ fn import_with_justification() { .unwrap() .block; block_on(client.import(BlockOrigin::Own, a2.clone())).unwrap(); - client.finalize_block(BlockId::hash(a2.hash()), None).unwrap(); + client.finalize_block(&a2.hash(), None).unwrap(); // A2 -> A3 let justification = Justifications::from((TEST_ENGINE_ID, vec![1, 2, 3])); @@ -996,7 +996,7 @@ fn finalizing_diverged_block_should_trigger_reorg() { // we finalize block B1 which is on a different branch from current best // which should trigger a re-org. - ClientExt::finalize_block(&client, BlockId::Hash(b1.hash()), None).unwrap(); + ClientExt::finalize_block(&client, &b1.hash(), None).unwrap(); // B1 should now be the latest finalized assert_eq!(client.chain_info().finalized_hash, b1.hash()); @@ -1020,7 +1020,7 @@ fn finalizing_diverged_block_should_trigger_reorg() { assert_eq!(client.chain_info().best_hash, b3.hash()); - ClientExt::finalize_block(&client, BlockId::Hash(b3.hash()), None).unwrap(); + ClientExt::finalize_block(&client, &b3.hash(), None).unwrap(); finality_notification_check(&mut finality_notifications, &[b1.hash()], &[]); finality_notification_check(&mut finality_notifications, &[b2.hash(), b3.hash()], &[a2.hash()]); @@ -1118,7 +1118,7 @@ fn finality_notifications_content() { // Postpone import to test behavior of import of finalized block. - ClientExt::finalize_block(&client, BlockId::Hash(a2.hash()), None).unwrap(); + ClientExt::finalize_block(&client, &a2.hash(), None).unwrap(); // Import and finalize D4 block_on(client.import_as_final(BlockOrigin::Own, d4.clone())).unwrap(); @@ -1274,7 +1274,7 @@ fn doesnt_import_blocks_that_revert_finality() { // we will finalize A2 which should make it impossible to import a new // B3 at the same height but that doesn't include it - ClientExt::finalize_block(&client, BlockId::Hash(a2.hash()), None).unwrap(); + ClientExt::finalize_block(&client, &a2.hash(), None).unwrap(); let import_err = block_on(client.import(BlockOrigin::Own, b3)).err().unwrap(); let expected_err = @@ -1309,7 +1309,7 @@ fn doesnt_import_blocks_that_revert_finality() { .unwrap() .block; block_on(client.import(BlockOrigin::Own, a3.clone())).unwrap(); - ClientExt::finalize_block(&client, BlockId::Hash(a3.hash()), None).unwrap(); + ClientExt::finalize_block(&client, &a3.hash(), None).unwrap(); finality_notification_check(&mut finality_notifications, &[a1.hash(), a2.hash()], &[]); diff --git a/test-utils/client/src/client_ext.rs b/test-utils/client/src/client_ext.rs index f2b99a5b355f0..dd416b9102fc0 100644 --- a/test-utils/client/src/client_ext.rs +++ b/test-utils/client/src/client_ext.rs @@ -22,14 +22,14 @@ use sc_client_api::{backend::Finalizer, client::BlockBackend}; use sc_consensus::{BlockImport, BlockImportParams, ForkChoiceStrategy}; use sc_service::client::Client; use sp_consensus::{BlockOrigin, Error as ConsensusError}; -use sp_runtime::{generic::BlockId, traits::Block as BlockT, Justification, Justifications}; +use sp_runtime::{traits::Block as BlockT, Justification, Justifications}; /// Extension trait for a test client. pub trait ClientExt: Sized { /// Finalize a block. fn finalize_block( &self, - id: BlockId, + hash: &Block::Hash, justification: Option, ) -> sp_blockchain::Result<()>; @@ -75,10 +75,10 @@ where { fn finalize_block( &self, - id: BlockId, + hash: &Block::Hash, justification: Option, ) -> sp_blockchain::Result<()> { - Finalizer::finalize_block(self, id, justification, true) + Finalizer::finalize_block(self, hash, justification, true) } fn genesis_hash(&self) -> ::Hash {