Unverified Commit 5c4dcfb2 authored by André Silva's avatar André Silva Committed by GitHub
Browse files

grandpa: add voting rule to pause new votes for a period (#904)

* grandpa: add voting rule to pause new votes for a period

* grandpa: increase delay

* grandpa: parse custom pause delay from cli

* grandpa: log scheduled pause on startup

* grandpa: rename parameter to grandpa_pause

* grandpa: make pause voting rule generic on block

* grandpa: add test for pause voting rule

* grandpa: add hardcoded pause

* collator: fix test compilation
parent 96682d36
Pipeline #83238 failed with stages
in 11 minutes and 37 seconds
......@@ -4071,8 +4071,10 @@ dependencies = [
"polkadot-primitives 0.7.26-pre1",
"polkadot-rpc 0.7.26-pre1",
"polkadot-runtime 0.7.26-pre1",
"polkadot-test-runtime-client 2.0.0",
"polkadot-validation 0.7.26-pre1",
"sc-authority-discovery 0.8.0-alpha.3 (git+https://github.com/paritytech/substrate?branch=polkadot-master)",
"sc-block-builder 0.8.0-alpha.3 (git+https://github.com/paritytech/substrate?branch=polkadot-master)",
"sc-chain-spec 2.0.0-alpha.3 (git+https://github.com/paritytech/substrate?branch=polkadot-master)",
"sc-client 0.8.0-alpha.3 (git+https://github.com/paritytech/substrate?branch=polkadot-master)",
"sc-client-api 2.0.0-alpha.3 (git+https://github.com/paritytech/substrate?branch=polkadot-master)",
......
......@@ -75,4 +75,13 @@ pub struct Cli {
#[allow(missing_docs)]
#[structopt(long = "enable-authority-discovery")]
pub authority_discovery_enabled: bool,
/// Setup a GRANDPA scheduled voting pause.
///
/// This parameter takes two values, namely a block number and a delay (in
/// blocks). After the given block number is finalized the GRANDPA voter
/// will temporarily stop voting for new blocks until the given delay has
/// elapsed (i.e. until a block at height `pause_block + delay` is imported).
#[structopt(long = "grandpa-pause", number_of_values(2))]
pub grandpa_pause: Vec<u32>,
}
......@@ -31,6 +31,14 @@ pub fn run(version: VersionInfo) -> sc_cli::Result<()> {
config.impl_name = "parity-polkadot";
let force_kusama = opt.run.force_kusama;
let grandpa_pause = if opt.grandpa_pause.is_empty() {
None
} else {
// should be enforced by cli parsing
assert_eq!(opt.grandpa_pause.len(), 2);
Some((opt.grandpa_pause[0], opt.grandpa_pause[1]))
};
match opt.subcommand {
None => {
opt.run.base.init(&version)?;
......@@ -61,7 +69,7 @@ pub fn run(version: VersionInfo) -> sc_cli::Result<()> {
service::kusama_runtime::RuntimeApi,
service::KusamaExecutor,
service::kusama_runtime::UncheckedExtrinsic,
>(config, opt.authority_discovery_enabled)
>(config, opt.authority_discovery_enabled, grandpa_pause)
} else {
info!("Native runtime: {}", service::PolkadotExecutor::native_version().runtime_version);
......@@ -69,7 +77,7 @@ pub fn run(version: VersionInfo) -> sc_cli::Result<()> {
service::polkadot_runtime::RuntimeApi,
service::PolkadotExecutor,
service::polkadot_runtime::UncheckedExtrinsic,
>(config, opt.authority_discovery_enabled)
>(config, opt.authority_discovery_enabled, grandpa_pause)
}
},
Some(Subcommand::Base(cmd)) => {
......@@ -123,6 +131,7 @@ pub fn run(version: VersionInfo) -> sc_cli::Result<()> {
fn run_service_until_exit<R, D, E>(
config: service::Configuration,
authority_discovery_enabled: bool,
grandpa_pause: Option<(u32, u32)>,
) -> sc_cli::Result<()>
where
R: ConstructRuntimeApi<Block, service::TFullClient<Block, R, D>>
......@@ -151,7 +160,14 @@ where
_ =>
sc_cli::run_service_until_exit(
config,
|config| service::new_full::<R, D, E>(config, None, None, authority_discovery_enabled, 6000)
|config| service::new_full::<R, D, E>(
config,
None,
None,
authority_discovery_enabled,
6000,
grandpa_pause,
)
.map(|(s, _)| s),
),
}
......
......@@ -349,14 +349,14 @@ where
).into(),
(true, _) =>
build_collator_service(
service::kusama_new_full(config, Some((key.public(), para_id)), None, false, 6000)?,
service::kusama_new_full(config, Some((key.public(), para_id)), None, false, 6000, None)?,
para_id,
key,
build_parachain_context,
)?.await,
(false, _) =>
build_collator_service(
service::polkadot_new_full(config, Some((key.public(), para_id)), None, false, 6000)?,
service::polkadot_new_full(config, Some((key.public(), para_id)), None, false, 6000, None)?,
para_id,
key,
build_parachain_context,
......@@ -395,7 +395,7 @@ pub fn run_collator<P>(
(true, _) =>
sc_cli::run_service_until_exit(config, |config| {
build_collator_service(
service::kusama_new_full(config, Some((key.public(), para_id)), None, false, 6000)?,
service::kusama_new_full(config, Some((key.public(), para_id)), None, false, 6000, None)?,
para_id,
key,
build_parachain_context,
......@@ -404,7 +404,7 @@ pub fn run_collator<P>(
(false, _) =>
sc_cli::run_service_until_exit(config, |config| {
build_collator_service(
service::polkadot_new_full(config, Some((key.public(), para_id)), None, false, 6000)?,
service::polkadot_new_full(config, Some((key.public(), para_id)), None, false, 6000, None)?,
para_id,
key,
build_parachain_context,
......
......@@ -55,6 +55,10 @@ sp-offchain = { package = "sp-offchain", git = "https://github.com/paritytech/su
prometheus-endpoint = { package = "substrate-prometheus-endpoint", git = "https://github.com/paritytech/substrate", branch = "polkadot-master" }
frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch = "polkadot-master" }
[dev-dependencies]
polkadot-test-runtime-client = { path = "../runtime/test-runtime/client" }
sc-block-builder = { git = "https://github.com/paritytech/substrate", branch = "polkadot-master" }
[features]
default = ["rocksdb", "full-node"]
rocksdb = ["service/rocksdb"]
......
......@@ -39,7 +39,7 @@ pub use sc_executor::NativeExecutionDispatch;
pub use sc_client::{ExecutionStrategy, CallExecutor, Client};
pub use sc_client_api::backend::Backend;
pub use sp_api::{Core as CoreApi, ConstructRuntimeApi, ProvideRuntimeApi, StateBackend};
pub use sp_runtime::traits::HashFor;
pub use sp_runtime::traits::{HashFor, NumberFor};
pub use consensus_common::SelectChain;
pub use polkadot_primitives::parachain::{CollatorId, ParachainHost};
pub use polkadot_primitives::Block;
......@@ -208,6 +208,7 @@ pub fn polkadot_new_full(
max_block_data_size: Option<u64>,
authority_discovery_enabled: bool,
slot_duration: u64,
grandpa_pause: Option<(u32, u32)>,
)
-> Result<(
impl AbstractService<
......@@ -220,7 +221,14 @@ pub fn polkadot_new_full(
FullNodeHandles,
), ServiceError>
{
new_full(config, collating_for, max_block_data_size, authority_discovery_enabled, slot_duration)
new_full(
config,
collating_for,
max_block_data_size,
authority_discovery_enabled,
slot_duration,
grandpa_pause,
)
}
/// Create a new Kusama service for a full node.
......@@ -231,6 +239,7 @@ pub fn kusama_new_full(
max_block_data_size: Option<u64>,
authority_discovery_enabled: bool,
slot_duration: u64,
grandpa_pause: Option<(u32, u32)>,
)
-> Result<(
impl AbstractService<
......@@ -243,7 +252,14 @@ pub fn kusama_new_full(
FullNodeHandles,
), ServiceError>
{
new_full(config, collating_for, max_block_data_size, authority_discovery_enabled, slot_duration)
new_full(
config,
collating_for,
max_block_data_size,
authority_discovery_enabled,
slot_duration,
grandpa_pause,
)
}
/// Handles to other sub-services that full nodes instantiate, which consumers
......@@ -262,6 +278,7 @@ pub fn new_full<Runtime, Dispatch, Extrinsic>(
max_block_data_size: Option<u64>,
authority_discovery_enabled: bool,
slot_duration: u64,
grandpa_pause: Option<(u32, u32)>,
)
-> Result<(
impl AbstractService<
......@@ -478,13 +495,37 @@ pub fn new_full<Runtime, Dispatch, Extrinsic>(
// they're validators or not). at this point the full voter should
// provide better guarantees of block and vote data availability than
// the observer.
// add a custom voting rule to temporarily stop voting for new blocks
// after the given pause block is finalized and restarting after the
// given delay.
// temporarily hardcode a GRANDPA pause for the upcoming runtime upgrade
let grandpa_pause = grandpa_pause.or(Some((1491586, 6000)));
let voting_rule = match grandpa_pause {
Some((block, delay)) => {
info!("GRANDPA scheduled voting pause set for block #{} with a duration of {} blocks.",
block,
delay,
);
grandpa::VotingRulesBuilder::default()
.add(PauseAfterBlockFor(block, delay))
.build()
},
None =>
grandpa::VotingRulesBuilder::default()
.build(),
};
let grandpa_config = grandpa::GrandpaParams {
config,
link: link_half,
network: service.network(),
inherent_data_providers: inherent_data_providers.clone(),
telemetry_on_connect: Some(service.telemetry_on_connect_stream()),
voting_rule: grandpa::VotingRulesBuilder::default().build(),
voting_rule,
prometheus_registry: service.prometheus_registry(),
};
......@@ -638,3 +679,197 @@ where
})?
.build()
}
/// A custom GRANDPA voting rule that "pauses" voting (i.e. keeps voting for the
/// same last finalized block) after a given block at height `N` has been
/// finalized and for a delay of `M` blocks, i.e. until the best block reaches
/// `N` + `M`, the voter will keep voting for block `N`.
struct PauseAfterBlockFor<N>(N, N);
impl<Block, B> grandpa::VotingRule<Block, B> for PauseAfterBlockFor<NumberFor<Block>> where
Block: BlockT,
B: sp_blockchain::HeaderBackend<Block>,
{
fn restrict_vote(
&self,
backend: &B,
base: &Block::Header,
best_target: &Block::Header,
current_target: &Block::Header,
) -> Option<(Block::Hash, NumberFor<Block>)> {
use sp_runtime::generic::BlockId;
use sp_runtime::traits::Header as _;
// walk backwards until we find the target block
let find_target = |
target_number: NumberFor<Block>,
current_header: &Block::Header
| {
let mut target_hash = current_header.hash();
let mut target_header = current_header.clone();
loop {
if *target_header.number() < target_number {
unreachable!(
"we are traversing backwards from a known block; \
blocks are stored contiguously; \
qed"
);
}
if *target_header.number() == target_number {
return Some((target_hash, target_number));
}
target_hash = *target_header.parent_hash();
target_header = backend.header(BlockId::Hash(target_hash)).ok()?
.expect("Header known to exist due to the existence of one of its descendents; qed");
}
};
// only restrict votes targeting a block higher than the block
// we've set for the pause
if *current_target.number() > self.0 {
// if we're past the pause period (i.e. `self.0 + self.1`)
// then we no longer need to restrict any votes
if *best_target.number() > self.0 + self.1 {
return None;
}
// if we've finalized the pause block, just keep returning it
// until best number increases enough to pass the condition above
if *base.number() >= self.0 {
return Some((base.hash(), *base.number()));
}
// otherwise find the target header at the pause block
// to vote on
return find_target(self.0, current_target);
}
None
}
}
#[cfg(test)]
mod tests {
use polkadot_test_runtime_client::prelude::*;
use polkadot_test_runtime_client::sp_consensus::BlockOrigin;
use sc_block_builder::BlockBuilderProvider;
use grandpa::VotingRule;
use sp_blockchain::HeaderBackend;
use sp_runtime::generic::BlockId;
use sp_runtime::traits::Header;
use std::sync::Arc;
#[test]
fn grandpa_pause_voting_rule_works() {
let client = Arc::new(polkadot_test_runtime_client::new());
let mut push_blocks = {
let mut client = client.clone();
move |n| {
for _ in 0..n {
let block = client.new_block(Default::default()).unwrap().build().unwrap().block;
client.import(BlockOrigin::Own, block).unwrap();
}
}
};
let get_header = {
let client = client.clone();
move |n| client.header(&BlockId::Number(n)).unwrap().unwrap()
};
// the rule should filter all votes after block #20
// is finalized until block #50 is imported.
let voting_rule = super::PauseAfterBlockFor(20, 30);
// add 10 blocks
push_blocks(10);
assert_eq!(
client.info().best_number,
10,
);
// we have not reached the pause block
// therefore nothing should be restricted
assert_eq!(
voting_rule.restrict_vote(
&*client,
&get_header(0),
&get_header(10),
&get_header(10),
),
None,
);
// add 15 more blocks
// best block: #25
push_blocks(15);
// we are targeting the pause block,
// the vote should not be restricted
assert_eq!(
voting_rule.restrict_vote(
&*client,
&get_header(10),
&get_header(20),
&get_header(20),
),
None,
);
// we are past the pause block, votes should
// be limited to the pause block.
let pause_block = get_header(20);
assert_eq!(
voting_rule.restrict_vote(
&*client,
&get_header(10),
&get_header(21),
&get_header(21),
),
Some((pause_block.hash(), *pause_block.number())),
);
// we've finalized the pause block, so we'll keep
// restricting our votes to it.
assert_eq!(
voting_rule.restrict_vote(
&*client,
&pause_block, // #20
&get_header(21),
&get_header(21),
),
Some((pause_block.hash(), *pause_block.number())),
);
// add 30 more blocks
// best block: #55
push_blocks(30);
// we're at the last block of the pause, this block
// should still be considered in the pause period
assert_eq!(
voting_rule.restrict_vote(
&*client,
&pause_block, // #20
&get_header(50),
&get_header(50),
),
Some((pause_block.hash(), *pause_block.number())),
);
// we're past the pause period, no votes should be filtered
assert_eq!(
voting_rule.restrict_vote(
&*client,
&pause_block, // #20
&get_header(51),
&get_header(51),
),
None,
);
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment