From a199a989b84e8457ace0ae2073b92071dd8bcabc Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Fri, 5 Jul 2019 09:40:26 +0200 Subject: [PATCH] on_start now returns the precise elements to request (#3003) * on_start now returns the precise elements to request * Fix test --- substrate/core/consensus/common/src/block_import.rs | 10 ++++++---- substrate/core/consensus/common/src/import_queue.rs | 9 +++++++-- substrate/core/finality-grandpa/src/import.rs | 7 +++++-- substrate/core/finality-grandpa/src/light_import.rs | 11 +++++++---- 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/substrate/core/consensus/common/src/block_import.rs b/substrate/core/consensus/common/src/block_import.rs index 6ce4acdf39..5213a5c73d 100644 --- a/substrate/core/consensus/common/src/block_import.rs +++ b/substrate/core/consensus/common/src/block_import.rs @@ -194,8 +194,9 @@ pub trait BlockImport { pub trait JustificationImport { type Error: ::std::error::Error + Send + 'static; - /// Called by the import queue when it is started. - fn on_start(&self, _link: &mut dyn crate::import_queue::Link) { } + /// Called by the import queue when it is started. Returns a list of justifications to request + /// from the network. + fn on_start(&self) -> Vec<(B::Hash, NumberFor)> { Vec::new() } /// Import a Block justification and finalize the given block. fn import_justification( @@ -210,8 +211,9 @@ pub trait JustificationImport { pub trait FinalityProofImport { type Error: std::error::Error + Send + 'static; - /// Called by the import queue when it is started. - fn on_start(&self, _link: &mut dyn crate::import_queue::Link) { } + /// Called by the import queue when it is started. Returns a list of finality proofs to request + /// from the network. + fn on_start(&self) -> Vec<(B::Hash, NumberFor)> { Vec::new() } /// Import a Block justification and finalize the given block. Returns finalized block or error. fn import_finality_proof( diff --git a/substrate/core/consensus/common/src/import_queue.rs b/substrate/core/consensus/common/src/import_queue.rs index 68a39a9d9c..e0a576d7fa 100644 --- a/substrate/core/consensus/common/src/import_queue.rs +++ b/substrate/core/consensus/common/src/import_queue.rs @@ -267,10 +267,15 @@ impl> BlockImportWorker { }; if let Some(justification_import) = worker.justification_import.as_ref() { - justification_import.on_start(&mut worker.result_sender); + for (hash, number) in justification_import.on_start() { + worker.result_sender.request_justification(&hash, number); + } } + if let Some(finality_proof_import) = worker.finality_proof_import.as_ref() { - finality_proof_import.on_start(&mut worker.result_sender); + for (hash, number) in finality_proof_import.on_start() { + worker.result_sender.request_finality_proof(&hash, number); + } } let future = futures::future::poll_fn(move || { diff --git a/substrate/core/finality-grandpa/src/import.rs b/substrate/core/finality-grandpa/src/import.rs index 227daff552..d447d04ebf 100644 --- a/substrate/core/finality-grandpa/src/import.rs +++ b/substrate/core/finality-grandpa/src/import.rs @@ -76,7 +76,8 @@ impl, RA, PRA, SC> JustificationImport { type Error = ConsensusError; - fn on_start(&self, link: &mut dyn consensus_common::import_queue::Link) { + fn on_start(&self) -> Vec<(Block::Hash, NumberFor)> { + let mut out = Vec::new(); let chain_info = self.inner.info().chain; // request justifications for all pending changes for which change blocks have already been imported @@ -94,12 +95,14 @@ impl, RA, PRA, SC> JustificationImport if let Ok(Some(hash)) = effective_block_hash { if let Ok(Some(header)) = self.inner.header(&BlockId::Hash(hash)) { if *header.number() == pending_change.effective_number() { - link.request_justification(&header.hash(), *header.number()); + out.push((header.hash(), *header.number())); } } } } } + + out } fn import_justification( diff --git a/substrate/core/finality-grandpa/src/light_import.rs b/substrate/core/finality-grandpa/src/light_import.rs index 25a3f84f6d..fcb575ec1a 100644 --- a/substrate/core/finality-grandpa/src/light_import.rs +++ b/substrate/core/finality-grandpa/src/light_import.rs @@ -144,15 +144,18 @@ impl, RA> FinalityProofImport { type Error = ConsensusError; - fn on_start(&self, link: &mut dyn consensus_common::import_queue::Link) { + fn on_start(&self) -> Vec<(Block::Hash, NumberFor)> { + let mut out = Vec::new(); let chain_info = self.client.info().chain; let data = self.data.read(); for (pending_number, pending_hash) in data.consensus_changes.pending_changes() { if *pending_number > chain_info.finalized_number && *pending_number <= chain_info.best_number { - link.request_finality_proof(pending_hash, *pending_number); + out.push((pending_hash.clone(), *pending_number)); } } + + out } fn import_finality_proof( @@ -572,8 +575,8 @@ pub mod tests { { type Error = ConsensusError; - fn on_start(&self, link: &mut dyn consensus_common::import_queue::Link) { - self.0.on_start(link) + fn on_start(&self) -> Vec<(Block::Hash, NumberFor)> { + self.0.on_start() } fn import_finality_proof(