diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock
index 2888776ab61443f15c735a12cb8259868c616e33..7a4ae1beea5afc888836e4db37e6e2cbf52e8b15 100644
--- a/substrate/Cargo.lock
+++ b/substrate/Cargo.lock
@@ -5482,6 +5482,7 @@ dependencies = [
"sc-telemetry 2.0.0",
"serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)",
"sp-api 2.0.0",
+ "sp-arithmetic 2.0.0",
"sp-blockchain 2.0.0",
"sp-consensus 0.8.0",
"sp-consensus-babe 0.8.0",
diff --git a/substrate/client/finality-grandpa/Cargo.toml b/substrate/client/finality-grandpa/Cargo.toml
index ac0501c3c31d9fc7d85084cbc657adc9cc9defbf..46268ee44ab0da3dd71364be6ffa38ba5c420003 100644
--- a/substrate/client/finality-grandpa/Cargo.toml
+++ b/substrate/client/finality-grandpa/Cargo.toml
@@ -13,6 +13,7 @@ log = "0.4.8"
parking_lot = "0.9.0"
rand = "0.7.2"
parity-scale-codec = { version = "1.0.0", features = ["derive"] }
+sp-arithmetic = { version = "2.0.0", path = "../../primitives/arithmetic" }
sp-runtime = { version = "2.0.0", path = "../../primitives/runtime" }
sp-consensus = { version = "0.8", path = "../../primitives/consensus/common" }
sp-core = { version = "2.0.0", path = "../../primitives/core" }
diff --git a/substrate/client/finality-grandpa/src/lib.rs b/substrate/client/finality-grandpa/src/lib.rs
index 78e803269d8ce823aafb27d1b6614b950fa7b6ea..c0d83ba0196a982404fd69e807c5dc9e7cb94700 100644
--- a/substrate/client/finality-grandpa/src/lib.rs
+++ b/substrate/client/finality-grandpa/src/lib.rs
@@ -96,7 +96,7 @@ pub use justification::GrandpaJustification;
pub use light_import::light_block_import;
pub use observer::run_grandpa_observer;
pub use voting_rule::{
- BeforeBestBlock, ThreeQuartersOfTheUnfinalizedChain, VotingRule, VotingRulesBuilder
+ BeforeBestBlockBy, ThreeQuartersOfTheUnfinalizedChain, VotingRule, VotingRulesBuilder
};
use aux_schema::PersistentData;
diff --git a/substrate/client/finality-grandpa/src/tests.rs b/substrate/client/finality-grandpa/src/tests.rs
index 4ad08c8868f274e250cdb6c548bbc47718e50d92..05b8f9689bbd5b98d4ec7752f5cb8aaa12a5634a 100644
--- a/substrate/client/finality-grandpa/src/tests.rs
+++ b/substrate/client/finality-grandpa/src/tests.rs
@@ -1694,8 +1694,8 @@ fn grandpa_environment_respects_voting_rules() {
}
};
- // add 20 blocks
- peer.push_blocks(20, false);
+ // add 21 blocks
+ peer.push_blocks(21, false);
// create an environment with no voting rule restrictions
let unrestricted_env = environment(Box::new(()));
@@ -1716,38 +1716,38 @@ fn grandpa_environment_respects_voting_rules() {
unrestricted_env.best_chain_containing(
peer.client().info().finalized_hash
).unwrap().1,
- 20,
+ 21,
);
- // both the other environments should return block 15, which is 3/4 of the
+ // both the other environments should return block 16, which is 3/4 of the
// way in the unfinalized chain
assert_eq!(
three_quarters_env.best_chain_containing(
peer.client().info().finalized_hash
).unwrap().1,
- 15,
+ 16,
);
assert_eq!(
default_env.best_chain_containing(
peer.client().info().finalized_hash
).unwrap().1,
- 15,
+ 16,
);
- // we finalize block 19 with block 20 being the best block
+ // we finalize block 19 with block 21 being the best block
peer.client().finalize_block(BlockId::Number(19), None, false).unwrap();
- // the 3/4 environment should propose block 20 for voting
+ // the 3/4 environment should propose block 21 for voting
assert_eq!(
three_quarters_env.best_chain_containing(
peer.client().info().finalized_hash
).unwrap().1,
- 20,
+ 21,
);
// while the default environment will always still make sure we don't vote
- // on the best block
+ // on the best block (2 behind)
assert_eq!(
default_env.best_chain_containing(
peer.client().info().finalized_hash
@@ -1755,17 +1755,17 @@ fn grandpa_environment_respects_voting_rules() {
19,
);
- // we finalize block 20 with block 20 being the best block
- peer.client().finalize_block(BlockId::Number(20), None, false).unwrap();
+ // we finalize block 21 with block 21 being the best block
+ peer.client().finalize_block(BlockId::Number(21), 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
- // the given base (#20).
+ // the given base (#21).
assert_eq!(
default_env.best_chain_containing(
peer.client().info().finalized_hash
).unwrap().1,
- 20,
+ 21,
);
}
diff --git a/substrate/client/finality-grandpa/src/voting_rule.rs b/substrate/client/finality-grandpa/src/voting_rule.rs
index 8ba52f30f7b0fb76ad9dd24845b683c4629596b3..523a1b05cd488645c557b6dfdef69d5c3ed1f7ac 100644
--- a/substrate/client/finality-grandpa/src/voting_rule.rs
+++ b/substrate/client/finality-grandpa/src/voting_rule.rs
@@ -70,30 +70,38 @@ impl<Block, B> VotingRule<Block, B> for () where
/// A custom voting rule that guarantees that our vote is always behind the best
/// block, in the best case exactly one block behind it.
#[derive(Clone)]
-pub struct BeforeBestBlock;
-impl<Block, B> VotingRule<Block, B> for BeforeBestBlock where
+pub struct BeforeBestBlockBy<N>(N);
+impl<Block, B> VotingRule<Block, B> for BeforeBestBlockBy<NumberFor<Block>> where
Block: BlockT,
B: HeaderBackend<Block>,
{
fn restrict_vote(
&self,
- _backend: &B,
+ backend: &B,
_base: &Block::Header,
best_target: &Block::Header,
current_target: &Block::Header,
) -> Option<(Block::Hash, NumberFor<Block>)> {
+ use sp_arithmetic::traits::Saturating;
+
if current_target.number().is_zero() {
return None;
}
- if current_target.number() == best_target.number() {
- return Some((
- current_target.parent_hash().clone(),
- *current_target.number() - One::one(),
- ));
+ // find the target number restricted by this rule
+ let target_number = best_target.number().saturating_sub(self.0);
+
+ // our current target is already lower than this rule would restrict
+ if target_number >= *current_target.number() {
+ return None;
}
- None
+ // find the block at the given target height
+ find_target(
+ backend,
+ target_number,
+ current_target,
+ )
}
}
@@ -130,26 +138,43 @@ impl<Block, B> VotingRule<Block, B> for ThreeQuartersOfTheUnfinalizedChain where
return None;
}
- let mut target_header = current_target.clone();
- let mut target_hash = current_target.hash();
-
- // walk backwards until we find the target block
- 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");
+ // find the block at the given target height
+ find_target(
+ backend,
+ target_number,
+ current_target,
+ )
+ }
+}
+
+// walk backwards until we find the target block
+fn find_target<Block, B>(
+ backend: &B,
+ target_number: NumberFor<Block>,
+ current_header: &Block::Header,
+) -> Option<(Block::Hash, NumberFor<Block>)> where
+ Block: BlockT,
+ B: HeaderBackend<Block>,
+{
+ 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");
}
}
@@ -213,7 +238,7 @@ impl<Block, B> Default for VotingRulesBuilder<Block, B> where
{
fn default() -> Self {
VotingRulesBuilder::new()
- .add(BeforeBestBlock)
+ .add(BeforeBestBlockBy(2.into()))
.add(ThreeQuartersOfTheUnfinalizedChain)
}
}