Skip to content

Commit 1d22f76

Browse files
rphmeierMTDK1
authored andcommitted
Offline fallback for GRANDPA (paritytech#1619)
Co-authored-by: André Silva <[email protected]> * skeleton for finality tracker * dispatch events when nothing finalized for a long time * begin integrating finality tracker into grandpa * add delay field to pending change * add has_api_with function to sr_version for querying APIs * partially integrate new force changes into grandpa * implement forced changes * get srml-grandpa compiling * Update core/finality-grandpa/src/authorities.rs Co-Authored-By: rphmeier <[email protected]> * Update core/finality-grandpa/src/authorities.rs Co-Authored-By: rphmeier <[email protected]> * Update core/finality-grandpa/src/authorities.rs Co-Authored-By: rphmeier <[email protected]> * remove explicit dependence on CoreApi * increase node runtime version * integrate grandpa forced changes into node runtime * add some tests to finality-tracker * integrate finality tracking into node-runtime * test forced-change logic * test forced changes in the authority-set handler * kill some unneeded bounds in client * test forced-changes in finality-grandpa and fix logic * build wasm and finality-tracker is no-std * restart voter on forced change * allow returning custom error type from lock_import_and_run * extract out most DB logic to aux_schema and use atomic client ops * unify authority set writing * implement set pausing * bump runtime version * note on DB when we pause. * core: grandpa: integrate forced changes with multiple pending standard changes * core: grandpa: fix AuthoritySet tests * runtime: bump impl_version * core: clear pending justification requests after forced change import * srml: finality-tracker: use FinalizedInherentData * core: log requests for clearing justification requests * core, node: update runtimes * core: grandpa: fix tests * core: grandpa: remove todos and add comments * core: grandpa: use has_api_with from ApiExt * core: fix tests * core: grandpa: remove unnecessary mut modifier * core: replace PostImportActions bitflags with struct * core: grandpa: restrict genesis on forced authority set change * core: grandpa: add more docs * core: grandpa: prevent safety violations in Environment::finalize_block * core: grandpa: register finality tracker inherent data provider * core: grandpa: fix tests * node: update runtime blobs * core: grandpa: remove outdated todo * core: aura: fix typo in log message * core: grandpa: check re-finalization is on canonical chain * srml: finality-tracker: fix initialization * node: update runtime wasm * srml: finality-tracker: don't re-initialize config keys
1 parent d5504f6 commit 1d22f76

File tree

31 files changed

+2215
-514
lines changed

31 files changed

+2215
-514
lines changed

Cargo.lock

Lines changed: 25 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ members = [
6767
"srml/example",
6868
"srml/executive",
6969
"srml/fees",
70+
"srml/finality-tracker",
7071
"srml/grandpa",
7172
"srml/indices",
7273
"srml/metadata",

core/client/src/client.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use runtime_primitives::{
2727
};
2828
use consensus::{
2929
Error as ConsensusError, ErrorKind as ConsensusErrorKind, ImportBlock, ImportResult,
30-
BlockOrigin, ForkChoiceStrategy
30+
BlockOrigin, ForkChoiceStrategy,
3131
};
3232
use runtime_primitives::traits::{
3333
Block as BlockT, Header as HeaderT, Zero, As, NumberFor, CurrentHeight, BlockNumberToHash,
@@ -619,9 +619,10 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
619619
}
620620

621621
/// Lock the import lock, and run operations inside.
622-
pub fn lock_import_and_run<R, F: FnOnce(&mut ClientImportOperation<Block, Blake2Hasher, B>) -> error::Result<R>>(
623-
&self, f: F
624-
) -> error::Result<R> {
622+
pub fn lock_import_and_run<R, Err, F>(&self, f: F) -> Result<R, Err> where
623+
F: FnOnce(&mut ClientImportOperation<Block, Blake2Hasher, B>) -> Result<R, Err>,
624+
Err: From<error::Error>,
625+
{
625626
let inner = || {
626627
let _import_lock = self.import_lock.lock();
627628

@@ -827,7 +828,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
827828
operation.notify_imported = Some((hash, origin, import_headers.into_post(), is_new_best, storage_changes));
828829
}
829830

830-
Ok(ImportResult::Queued)
831+
Ok(ImportResult::imported())
831832
}
832833

833834
fn block_execution(
@@ -1391,13 +1392,15 @@ impl<B, E, Block, RA> consensus::BlockImport<Block> for Client<B, E, Block, RA>
13911392
blockchain::BlockStatus::InChain => {},
13921393
blockchain::BlockStatus::Unknown => return Ok(ImportResult::UnknownParent),
13931394
}
1395+
13941396
match self.backend.blockchain().status(BlockId::Hash(hash))
13951397
.map_err(|e| ConsensusError::from(ConsensusErrorKind::ClientImport(e.to_string())))?
13961398
{
13971399
blockchain::BlockStatus::InChain => return Ok(ImportResult::AlreadyInChain),
13981400
blockchain::BlockStatus::Unknown => {},
13991401
}
1400-
Ok(ImportResult::Queued)
1402+
1403+
Ok(ImportResult::imported())
14011404
}
14021405
}
14031406

@@ -1414,7 +1417,7 @@ impl<B, E, Block, RA> consensus::Authorities<Block> for Client<B, E, Block, RA>
14141417

14151418
impl<B, E, Block, RA> CurrentHeight for Client<B, E, Block, RA> where
14161419
B: backend::Backend<Block, Blake2Hasher>,
1417-
E: CallExecutor<Block, Blake2Hasher> + Clone,
1420+
E: CallExecutor<Block, Blake2Hasher>,
14181421
Block: BlockT<Hash=H256>,
14191422
{
14201423
type BlockNumber = <Block::Header as HeaderT>::Number;
@@ -1425,7 +1428,7 @@ impl<B, E, Block, RA> CurrentHeight for Client<B, E, Block, RA> where
14251428

14261429
impl<B, E, Block, RA> BlockNumberToHash for Client<B, E, Block, RA> where
14271430
B: backend::Backend<Block, Blake2Hasher>,
1428-
E: CallExecutor<Block, Blake2Hasher> + Clone,
1431+
E: CallExecutor<Block, Blake2Hasher>,
14291432
Block: BlockT<Hash=H256>,
14301433
{
14311434
type BlockNumber = <Block::Header as HeaderT>::Number;

core/consensus/common/src/block_import.rs

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,40 @@ use std::borrow::Cow;
2323
/// Block import result.
2424
#[derive(Debug, PartialEq, Eq)]
2525
pub enum ImportResult {
26-
/// Added to the import queue.
27-
Queued,
28-
/// Already in the import queue.
29-
AlreadyQueued,
26+
/// Block imported.
27+
Imported(ImportedAux),
3028
/// Already in the blockchain.
3129
AlreadyInChain,
3230
/// Block or parent is known to be bad.
3331
KnownBad,
3432
/// Block parent is not in the chain.
3533
UnknownParent,
36-
/// Added to the import queue but must be justified
37-
/// (usually required to safely enact consensus changes).
38-
NeedsJustification,
34+
}
35+
36+
/// Auxiliary data associated with an imported block result.
37+
#[derive(Debug, PartialEq, Eq)]
38+
pub struct ImportedAux {
39+
/// Clear all pending justification requests.
40+
pub clear_justification_requests: bool,
41+
/// Request a justification for the given block.
42+
pub needs_justification: bool,
43+
}
44+
45+
impl Default for ImportedAux {
46+
fn default() -> ImportedAux {
47+
ImportedAux {
48+
clear_justification_requests: false,
49+
needs_justification: false,
50+
}
51+
}
52+
}
53+
54+
impl ImportResult {
55+
/// Returns default value for `ImportResult::Imported` with both
56+
/// `clear_justification_requests` and `needs_justification` set to false.
57+
pub fn imported() -> ImportResult {
58+
ImportResult::Imported(ImportedAux::default())
59+
}
3960
}
4061

4162
/// Block data origin.

core/consensus/common/src/import_queue.rs

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@
2424
//! The `BasicQueue` and `BasicVerifier` traits allow serial queues to be
2525
//! instantiated simply.
2626
27-
use crate::block_import::{ImportBlock, BlockImport, JustificationImport, ImportResult, BlockOrigin};
27+
use crate::block_import::{
28+
BlockImport, BlockOrigin, ImportBlock, ImportedAux, ImportResult, JustificationImport,
29+
};
2830
use crossbeam_channel::{self as channel, Receiver, Sender};
2931

3032
use std::sync::Arc;
@@ -307,31 +309,37 @@ impl<B: BlockT> BlockImporter<B> {
307309

308310
match result {
309311
Ok(BlockImportResult::ImportedKnown(number)) => link.block_imported(&hash, number),
310-
Ok(BlockImportResult::ImportedUnknown(number)) => {
311-
link.block_imported(&hash, number)
312-
}
313-
Ok(BlockImportResult::ImportedUnjustified(number)) => {
312+
Ok(BlockImportResult::ImportedUnknown(number, aux)) => {
314313
link.block_imported(&hash, number);
315-
link.request_justification(&hash, number);
314+
315+
if aux.clear_justification_requests {
316+
trace!(target: "sync", "Block imported clears all pending justification requests {}: {:?}", number, hash);
317+
link.clear_justification_requests();
318+
}
319+
320+
if aux.needs_justification {
321+
trace!(target: "sync", "Block imported but requires justification {}: {:?}", number, hash);
322+
link.request_justification(&hash, number);
323+
}
316324
},
317325
Err(BlockImportError::IncompleteHeader(who)) => {
318326
if let Some(peer) = who {
319327
link.note_useless_and_restart_sync(peer, "Sent block with incomplete header to import");
320328
}
321-
}
329+
},
322330
Err(BlockImportError::VerificationFailed(who, e)) => {
323331
if let Some(peer) = who {
324332
link.note_useless_and_restart_sync(peer, &format!("Verification failed: {}", e));
325333
}
326-
}
334+
},
327335
Err(BlockImportError::BadBlock(who)) => {
328336
if let Some(peer) = who {
329337
link.note_useless_and_restart_sync(peer, "Sent us a bad block");
330338
}
331-
}
339+
},
332340
Err(BlockImportError::UnknownParent) | Err(BlockImportError::Error) => {
333-
link.restart()
334-
}
341+
link.restart();
342+
},
335343
};
336344
}
337345
if let Some(link) = self.link.as_ref() {
@@ -448,13 +456,15 @@ impl<B: BlockT, V: 'static + Verifier<B>> BlockImportWorker<B, V> {
448456
/// algorithm.
449457
pub trait Link<B: BlockT>: Send {
450458
/// Block imported.
451-
fn block_imported(&self, _hash: &B::Hash, _number: NumberFor<B>) { }
459+
fn block_imported(&self, _hash: &B::Hash, _number: NumberFor<B>) {}
452460
/// Batch of blocks imported, with or without error.
453461
fn blocks_processed(&self, _processed_blocks: Vec<B::Hash>, _has_error: bool) {}
454462
/// Justification import result.
455-
fn justification_imported(&self, _who: Origin, _hash: &B::Hash, _number: NumberFor<B>, _success: bool) { }
463+
fn justification_imported(&self, _who: Origin, _hash: &B::Hash, _number: NumberFor<B>, _success: bool) {}
464+
/// Clear all pending justification requests.
465+
fn clear_justification_requests(&self) {}
456466
/// Request a justification for the given block.
457-
fn request_justification(&self, _hash: &B::Hash, _number: NumberFor<B>) { }
467+
fn request_justification(&self, _hash: &B::Hash, _number: NumberFor<B>) {}
458468
/// Disconnect from peer.
459469
fn useless_peer(&self, _who: Origin, _reason: &str) {}
460470
/// Disconnect from peer and restart sync.
@@ -469,9 +479,7 @@ pub enum BlockImportResult<N: ::std::fmt::Debug + PartialEq> {
469479
/// Imported known block.
470480
ImportedKnown(N),
471481
/// Imported unknown block.
472-
ImportedUnknown(N),
473-
/// Imported unjustified block that requires one.
474-
ImportedUnjustified(N),
482+
ImportedUnknown(N, ImportedAux),
475483
}
476484

477485
/// Block import error.
@@ -520,17 +528,7 @@ pub fn import_single_block<B: BlockT, V: Verifier<B>>(
520528
trace!(target: "sync", "Block already in chain {}: {:?}", number, hash);
521529
Ok(BlockImportResult::ImportedKnown(number))
522530
},
523-
Ok(ImportResult::AlreadyQueued) => {
524-
trace!(target: "sync", "Block already queued {}: {:?}", number, hash);
525-
Ok(BlockImportResult::ImportedKnown(number))
526-
},
527-
Ok(ImportResult::Queued) => {
528-
Ok(BlockImportResult::ImportedUnknown(number))
529-
},
530-
Ok(ImportResult::NeedsJustification) => {
531-
trace!(target: "sync", "Block queued but requires justification {}: {:?}", number, hash);
532-
Ok(BlockImportResult::ImportedUnjustified(number))
533-
},
531+
Ok(ImportResult::Imported(aux)) => Ok(BlockImportResult::ImportedUnknown(number, aux)),
534532
Ok(ImportResult::UnknownParent) => {
535533
debug!(target: "sync", "Block with unknown parent {}: {:?}, parent: {:?}", number, hash, parent);
536534
Err(BlockImportError::UnknownParent)
@@ -547,7 +545,7 @@ pub fn import_single_block<B: BlockT, V: Verifier<B>>(
547545
};
548546

549547
match import_error(import_handle.check_block(hash, parent))? {
550-
BlockImportResult::ImportedUnknown(_) => (),
548+
BlockImportResult::ImportedUnknown { .. } => (),
551549
r @ _ => return Ok(r), // Any other successfull result means that the block is already imported.
552550
}
553551

@@ -629,7 +627,7 @@ mod tests {
629627
assert_eq!(link_port.recv(), Ok(LinkMsg::BlockImported));
630628

631629
// Send an unknown
632-
let results = vec![(Ok(BlockImportResult::ImportedUnknown(Default::default())), Default::default())];
630+
let results = vec![(Ok(BlockImportResult::ImportedUnknown(Default::default(), Default::default())), Default::default())];
633631
let _ = result_sender.send(BlockImportWorkerMsg::Imported(results)).ok().unwrap();
634632
assert_eq!(link_port.recv(), Ok(LinkMsg::BlockImported));
635633

core/consensus/common/src/lib.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ pub mod evaluation;
4747
const MAX_BLOCK_SIZE: usize = 4 * 1024 * 1024 + 512;
4848

4949
pub use self::error::{Error, ErrorKind};
50-
pub use block_import::{BlockImport, JustificationImport, ImportBlock, BlockOrigin, ImportResult, ForkChoiceStrategy};
50+
pub use block_import::{
51+
BlockImport, BlockOrigin, ForkChoiceStrategy, ImportedAux, ImportBlock, ImportResult, JustificationImport,
52+
};
5153

5254
/// Trait for getting the authorities at a given block.
5355
pub trait Authorities<B: Block> {

core/finality-grandpa/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@ consensus_common = { package = "substrate-consensus-common", path = "../consensu
1717
substrate-primitives = { path = "../primitives" }
1818
substrate-telemetry = { path = "../telemetry" }
1919
client = { package = "substrate-client", path = "../client" }
20+
inherents = { package = "substrate-inherents", path = "../../core/inherents" }
2021
network = { package = "substrate-network", path = "../network" }
2122
service = { package = "substrate-service", path = "../service", optional = true }
23+
srml-finality-tracker = { path = "../../srml/finality-tracker" }
2224
fg_primitives = { package = "substrate-finality-grandpa-primitives", path = "primitives" }
2325
grandpa = { package = "finality-grandpa", version = "0.6.0", features = ["derive-codec"] }
2426

core/finality-grandpa/primitives/src/lib.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ decl_runtime_apis! {
6161
/// applied in the runtime after those N blocks have passed.
6262
///
6363
/// The consensus protocol will coordinate the handoff externally.
64+
#[api_version(2)]
6465
pub trait GrandpaApi {
6566
/// Check a digest for pending changes.
6667
/// Return `None` if there are no pending changes.
@@ -77,6 +78,28 @@ decl_runtime_apis! {
7778
fn grandpa_pending_change(digest: &DigestFor<Block>)
7879
-> Option<ScheduledChange<NumberFor<Block>>>;
7980

81+
/// Check a digest for forced changes.
82+
/// Return `None` if there are no forced changes. Otherwise, return a
83+
/// tuple containing the pending change and the median last finalized
84+
/// block number at the time the change was signalled.
85+
///
86+
/// Added in version 2.
87+
///
88+
/// Forced changes are applied after a delay of _imported_ blocks,
89+
/// while pending changes are applied after a delay of _finalized_ blocks.
90+
///
91+
/// Precedence towards earlier or later digest items can be given
92+
/// based on the rules of the chain.
93+
///
94+
/// No change should be scheduled if one is already and the delay has not
95+
/// passed completely.
96+
///
97+
/// This should be a pure function: i.e. as long as the runtime can interpret
98+
/// the digest type it should return the same result regardless of the current
99+
/// state.
100+
fn grandpa_forced_change(digest: &DigestFor<Block>)
101+
-> Option<(NumberFor<Block>, ScheduledChange<NumberFor<Block>>)>;
102+
80103
/// Get the current GRANDPA authorities and weights. This should not change except
81104
/// for when changes are scheduled and the corresponding delay has passed.
82105
///

0 commit comments

Comments
 (0)