Commit 6830cfce authored by André Silva's avatar André Silva Committed by Aaro Perämaa
Browse files

grandpa: make the VotingRule API async (#8101)



* grandpa: make the VotingRule api async

* grandpa: add docs to VotingRuleResult

* grandpa: formatting

* grandpa: use async blocks
Co-authored-by: default avatarBastian Köcher <bkchr@users.noreply.github.com>

* grandpa: expose VotingRuleResult

* grandpa: revert some broken changes to async syntax

* grandpa: use finality-grandpa v0.14.0

* grandpa: bump impl_version
Co-authored-by: default avatarBastian Köcher <bkchr@users.noreply.github.com>
parent d6390305
Pipeline #2322 failed with stage
......@@ -1637,9 +1637,9 @@ dependencies = [
[[package]]
name = "finality-grandpa"
version = "0.13.0"
version = "0.14.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2cd795898c348a8ec9edc66ec9e014031c764d4c88cc26d09b492cd93eb41339"
checksum = "c6447e2f8178843749e8c8003206def83ec124a7859475395777a28b5338647c"
dependencies = [
"either",
"futures 0.3.12",
......@@ -7193,6 +7193,7 @@ version = "0.9.0"
dependencies = [
"assert_matches",
"derive_more",
"dyn-clone",
"finality-grandpa",
"fork-tree",
"futures 0.3.12",
......
......@@ -109,17 +109,17 @@ pub fn wasm_binary_unwrap() -> &'static [u8] {
/// Runtime version.
pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("node"),
impl_name: create_runtime_str!("substrate-node"),
authoring_version: 10,
// Per convention: if the runtime behavior changes, increment spec_version
// and set impl_version to 0. If only runtime
// implementation changes and behavior does not, then leave spec_version as
// is and increment impl_version.
spec_version: 264,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 2,
spec_name: create_runtime_str!("node"),
impl_name: create_runtime_str!("substrate-node"),
authoring_version: 10,
// Per convention: if the runtime behavior changes, increment spec_version
// and set impl_version to 0. If only runtime
// implementation changes and behavior does not, then leave spec_version as
// is and increment impl_version.
spec_version: 264,
impl_version: 1,
apis: RUNTIME_API_VERSIONS,
transaction_version: 2,
};
/// Native version.
......
......@@ -16,6 +16,7 @@ targets = ["x86_64-unknown-linux-gnu"]
[dependencies]
derive_more = "0.99.2"
dyn-clone = "1.0"
fork-tree = { version = "3.0.0", path = "../../utils/fork-tree" }
futures = "0.3.9"
futures-timer = "3.0.1"
......@@ -43,13 +44,13 @@ sc-network-gossip = { version = "0.9.0", path = "../network-gossip" }
sp-finality-grandpa = { version = "3.0.0", path = "../../primitives/finality-grandpa" }
prometheus-endpoint = { package = "substrate-prometheus-endpoint", path = "../../utils/prometheus", version = "0.9.0"}
sc-block-builder = { version = "0.9.0", path = "../block-builder" }
finality-grandpa = { version = "0.13.0", features = ["derive-codec"] }
finality-grandpa = { version = "0.14.0", features = ["derive-codec"] }
pin-project = "1.0.4"
linked-hash-map = "0.5.2"
[dev-dependencies]
assert_matches = "1.3.0"
finality-grandpa = { version = "0.13.0", features = ["derive-codec", "test-helpers"] }
finality-grandpa = { version = "0.14.0", features = ["derive-codec", "test-helpers"] }
sc-network = { version = "0.9.0", path = "../network" }
sc-network-test = { version = "0.8.0", path = "../network/test" }
sp-keyring = { version = "3.0.0", path = "../../primitives/keyring" }
......
......@@ -14,7 +14,7 @@ sc-rpc = { version = "3.0.0", path = "../../rpc" }
sp-blockchain = { version = "3.0.0", path = "../../../primitives/blockchain" }
sp-core = { version = "3.0.0", path = "../../../primitives/core" }
sp-runtime = { version = "3.0.0", path = "../../../primitives/runtime" }
finality-grandpa = { version = "0.13.0", features = ["derive-codec"] }
finality-grandpa = { version = "0.14.0", features = ["derive-codec"] }
jsonrpc-core = "15.1.0"
jsonrpc-core-client = "15.1.0"
jsonrpc-derive = "15.1.0"
......
This diff is collapsed.
......@@ -217,34 +217,21 @@ impl<Block: BlockT> finality_grandpa::Chain<Block::Hash, NumberFor<Block>> for A
where
NumberFor<Block>: finality_grandpa::BlockNumberOps,
{
fn ancestry(
&self,
base: Block::Hash,
block: Block::Hash,
) -> Result<Vec<Block::Hash>, GrandpaError> {
let mut route = Vec::new();
let mut current_hash = block;
loop {
if current_hash == base {
break;
}
match self.ancestry.get(&current_hash) {
Some(current_header) => {
current_hash = *current_header.parent_hash();
route.push(current_hash);
}
_ => return Err(GrandpaError::NotDescendent),
}
}
route.pop(); // remove the base
Ok(route)
}
fn best_chain_containing(
&self,
_block: Block::Hash,
) -> Option<(Block::Hash, NumberFor<Block>)> {
None
}
fn ancestry(&self, base: Block::Hash, block: Block::Hash) -> Result<Vec<Block::Hash>, GrandpaError> {
let mut route = Vec::new();
let mut current_hash = block;
loop {
if current_hash == base { break; }
match self.ancestry.get(&current_hash) {
Some(current_header) => {
current_hash = *current_header.parent_hash();
route.push(current_hash);
},
_ => return Err(GrandpaError::NotDescendent),
}
}
route.pop(); // remove the base
Ok(route)
}
}
......@@ -124,7 +124,8 @@ pub use import::GrandpaBlockImport;
pub use justification::GrandpaJustification;
pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream};
pub use voting_rule::{
BeforeBestBlockBy, ThreeQuartersOfTheUnfinalizedChain, VotingRule, VotingRulesBuilder,
BeforeBestBlockBy, ThreeQuartersOfTheUnfinalizedChain, VotingRule, VotingRuleResult,
VotingRulesBuilder,
};
use aux_schema::PersistentData;
......
......@@ -53,21 +53,9 @@ where
Client: HeaderMetadata<Block, Error = sp_blockchain::Error>,
NumberFor<Block>: BlockNumberOps,
{
fn ancestry(
&self,
base: Block::Hash,
block: Block::Hash,
) -> Result<Vec<Block::Hash>, GrandpaError> {
environment::ancestry(&self.client, base, block)
}
fn best_chain_containing(
&self,
_block: Block::Hash,
) -> Option<(Block::Hash, NumberFor<Block>)> {
// only used by voter
None
}
fn ancestry(&self, base: Block::Hash, block: Block::Hash) -> Result<Vec<Block::Hash>, GrandpaError> {
environment::ancestry(&self.client, base, block)
}
}
fn grandpa_observer<BE, Block: BlockT, Client, S, F>(
......
......@@ -1568,105 +1568,95 @@ where
#[test]
fn grandpa_environment_respects_voting_rules() {
use finality_grandpa::Chain;
let peers = &[Ed25519Keyring::Alice];
let voters = make_ids(peers);
let mut net = GrandpaTestNet::new(TestApi::new(voters), 1);
let peer = net.peer(0);
let network_service = peer.network_service().clone();
let link = peer.data.lock().take().unwrap();
// add 21 blocks
peer.push_blocks(21, false);
// create an environment with no voting rule restrictions
let unrestricted_env = test_environment(&link, None, network_service.clone(), ());
// another with 3/4 unfinalized chain voting rule restriction
let three_quarters_env = test_environment(
&link,
None,
network_service.clone(),
voting_rule::ThreeQuartersOfTheUnfinalizedChain,
);
// and another restricted with the default voting rules: i.e. 3/4 rule and
// always below best block
let default_env = test_environment(
&link,
None,
network_service.clone(),
VotingRulesBuilder::default().build(),
);
// the unrestricted environment should just return the best block
assert_eq!(
unrestricted_env
.best_chain_containing(peer.client().info().finalized_hash)
.unwrap()
.1,
21,
);
// 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,
16,
);
assert_eq!(
default_env
.best_chain_containing(peer.client().info().finalized_hash)
.unwrap()
.1,
16,
);
// 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 21 for voting
assert_eq!(
three_quarters_env
.best_chain_containing(peer.client().info().finalized_hash)
.unwrap()
.1,
21,
);
// while the default environment will always still make sure we don't vote
// on the best block (2 behind)
assert_eq!(
default_env
.best_chain_containing(peer.client().info().finalized_hash)
.unwrap()
.1,
19,
);
// 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 (#21).
assert_eq!(
default_env
.best_chain_containing(peer.client().info().finalized_hash)
.unwrap()
.1,
21,
);
use finality_grandpa::voter::Environment;
let peers = &[Ed25519Keyring::Alice];
let voters = make_ids(peers);
let mut net = GrandpaTestNet::new(TestApi::new(voters), 1);
let peer = net.peer(0);
let network_service = peer.network_service().clone();
let link = peer.data.lock().take().unwrap();
// add 21 blocks
peer.push_blocks(21, false);
// create an environment with no voting rule restrictions
let unrestricted_env = test_environment(&link, None, network_service.clone(), ());
// another with 3/4 unfinalized chain voting rule restriction
let three_quarters_env = test_environment(
&link,
None,
network_service.clone(),
voting_rule::ThreeQuartersOfTheUnfinalizedChain,
);
// and another restricted with the default voting rules: i.e. 3/4 rule and
// always below best block
let default_env = test_environment(
&link,
None,
network_service.clone(),
VotingRulesBuilder::default().build(),
);
// the unrestricted environment should just return the best block
assert_eq!(
futures::executor::block_on(unrestricted_env.best_chain_containing(
peer.client().info().finalized_hash
)).unwrap().unwrap().1,
21,
);
// both the other environments should return block 16, which is 3/4 of the
// way in the unfinalized chain
assert_eq!(
futures::executor::block_on(three_quarters_env.best_chain_containing(
peer.client().info().finalized_hash
)).unwrap().unwrap().1,
16,
);
assert_eq!(
futures::executor::block_on(default_env.best_chain_containing(
peer.client().info().finalized_hash
)).unwrap().unwrap().1,
16,
);
// 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 21 for voting
assert_eq!(
futures::executor::block_on(three_quarters_env.best_chain_containing(
peer.client().info().finalized_hash
)).unwrap().unwrap().1,
21,
);
// while the default environment will always still make sure we don't vote
// on the best block (2 behind)
assert_eq!(
futures::executor::block_on(default_env.best_chain_containing(
peer.client().info().finalized_hash
)).unwrap().unwrap().1,
19,
);
// 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 (#21).
assert_eq!(
futures::executor::block_on(default_env.best_chain_containing(
peer.client().info().finalized_hash
)).unwrap().unwrap().1,
21,
);
}
#[test]
......
......@@ -22,37 +22,44 @@
//! restrictions that are taken into account by the GRANDPA environment when
//! selecting a finality target to vote on.
use std::future::Future;
use std::sync::Arc;
use std::pin::Pin;
use dyn_clone::DynClone;
use sc_client_api::blockchain::HeaderBackend;
use sp_runtime::generic::BlockId;
use sp_runtime::traits::{Block as BlockT, Header, NumberFor, One, Zero};
/// A future returned by a `VotingRule` to restrict a given vote, if any restriction is necessary.
pub type VotingRuleResult<Block> =
Pin<Box<dyn Future<Output = Option<(<Block as BlockT>::Hash, NumberFor<Block>)>> + Send + Sync>>;
/// A trait for custom voting rules in GRANDPA.
pub trait VotingRule<Block, B>: Send + Sync
where
Block: BlockT,
B: HeaderBackend<Block>,
pub trait VotingRule<Block, B>: DynClone + Send + Sync where
Block: BlockT,
B: HeaderBackend<Block>,
{
/// Restrict the given `current_target` vote, returning the block hash and
/// number of the block to vote on, and `None` in case the vote should not
/// be restricted. `base` is the block that we're basing our votes on in
/// order to pick our target (e.g. last round estimate), and `best_target`
/// is the initial best vote target before any vote rules were applied. When
/// applying multiple `VotingRule`s both `base` and `best_target` should
/// remain unchanged.
///
/// The contract of this interface requires that when restricting a vote, the
/// returned value **must** be an ancestor of the given `current_target`,
/// this also means that a variant must be maintained throughout the
/// execution of voting rules wherein `current_target <= best_target`.
fn restrict_vote(
&self,
backend: &B,
base: &Block::Header,
best_target: &Block::Header,
current_target: &Block::Header,
) -> Option<(Block::Hash, NumberFor<Block>)>;
/// Restrict the given `current_target` vote, returning the block hash and
/// number of the block to vote on, and `None` in case the vote should not
/// be restricted. `base` is the block that we're basing our votes on in
/// order to pick our target (e.g. last round estimate), and `best_target`
/// is the initial best vote target before any vote rules were applied. When
/// applying multiple `VotingRule`s both `base` and `best_target` should
/// remain unchanged.
///
/// The contract of this interface requires that when restricting a vote, the
/// returned value **must** be an ancestor of the given `current_target`,
/// this also means that a variant must be maintained throughout the
/// execution of voting rules wherein `current_target <= best_target`.
fn restrict_vote(
&self,
backend: Arc<B>,
base: &Block::Header,
best_target: &Block::Header,
current_target: &Block::Header,
) -> VotingRuleResult<Block>;
}
impl<Block, B> VotingRule<Block, B> for ()
......@@ -60,15 +67,15 @@ where
Block: BlockT,
B: HeaderBackend<Block>,
{
fn restrict_vote(
&self,
_backend: &B,
_base: &Block::Header,
_best_target: &Block::Header,
_current_target: &Block::Header,
) -> Option<(Block::Hash, NumberFor<Block>)> {
None
}
fn restrict_vote(
&self,
_backend: Arc<B>,
_base: &Block::Header,
_best_target: &Block::Header,
_current_target: &Block::Header,
) -> VotingRuleResult<Block> {
Box::pin(async { None })
}
}
/// A custom voting rule that guarantees that our vote is always behind the best
......@@ -81,35 +88,42 @@ where
Block: BlockT,
B: 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_arithmetic::traits::Saturating;
if current_target.number().is_zero() {
return None;
}
// 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;
}
// find the block at the given target height
find_target(backend, target_number, current_target)
}
fn restrict_vote(
&self,
backend: Arc<B>,
_base: &Block::Header,
best_target: &Block::Header,
current_target: &Block::Header,
) -> VotingRuleResult<Block> {
use sp_arithmetic::traits::Saturating;
if current_target.number().is_zero() {
return Box::pin(async { None });
}
// 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 Box::pin(async { None });
}
let current_target = current_target.clone();
// find the block at the given target height
Box::pin(std::future::ready(find_target(
&*backend,
target_number.clone(),
&current_target,
)))
}
}
/// A custom voting rule that limits votes towards 3/4 of the unfinalized chain,
/// using the given `base` and `best_target` to figure where the 3/4 target
/// should fall.
#[derive(Clone)]
pub struct ThreeQuartersOfTheUnfinalizedChain;
impl<Block, B> VotingRule<Block, B> for ThreeQuartersOfTheUnfinalizedChain
......@@ -117,33 +131,37 @@ where
Block: BlockT,
B: HeaderBackend<Block>,
{
fn restrict_vote(
&self,
backend: &B,
base: &Block::Header,
best_target: &Block::Header,
current_target: &Block::Header,
) -> Option<(Block::Hash, NumberFor<Block>)> {
// target a vote towards 3/4 of the unfinalized chain (rounding up)
let target_number = {
let two = NumberFor::<Block>::one() + One::one();
let three = two + One::one();
let four = three + One::one();
let diff = *best_target.number() - *base.number();
let diff = ((diff * three) + two) / four;
*base.number() + diff
};
// our current target is already lower than this rule would restrict
if target_number >= *current_target.number() {
return None;
}
// find the block at the given target height
find_target(backend, target_number, current_target)
}
fn restrict_vote(
&self,
backend: Arc<B>,
base: &Block::Header,
best_target: &Block::Header,
current_target: &Block::Header,
) -> VotingRuleResult<Block> {
// target a vote towards 3/4 of the unfinalized chain (rounding up)
let target_number = {
let two = NumberFor::<Block>::one() + One::one();
let three = two + One::one();
let four = three + One::one();
let diff = *best_target.number() - *base.number();
let diff = ((diff * three) + two) / four;
*base.number() + diff
};
// our current target is already lower than this rule would restrict
if target_number >= *current_target.number() {
return Box::pin(async { None });
}
// find the block at the given target height
Box::pin(std::future::ready(find_target(
&*backend,
target_number,
current_target,
)))
}
}
// walk backwards until we find the target block
......@@ -192,36 +210,45 @@ impl<B, Block> Clone for VotingRules<B, Block> {
}
}