From 795701608c22170bd4874bbc0ed7c89773ee1baf Mon Sep 17 00:00:00 2001
From: Gavin Wood <gavin@parity.io>
Date: Fri, 22 Nov 2019 13:05:52 +0100
Subject: [PATCH] grandpa: voting rules shouldn't restrict past round base
(#4155)
* grandpa: voting rules shouldn't restrict past round base
* grandpa: fix lower bound on vote restriction. add test
---
.../client/finality-grandpa/src/environment.rs | 12 +++++++++++-
substrate/client/finality-grandpa/src/tests.rs | 13 +++++++++++++
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/substrate/client/finality-grandpa/src/environment.rs b/substrate/client/finality-grandpa/src/environment.rs
index 883dc0e35c0..b0eaf298b5a 100644
--- a/substrate/client/finality-grandpa/src/environment.rs
+++ b/substrate/client/finality-grandpa/src/environment.rs
@@ -34,7 +34,7 @@ use client_api::{
utils::is_descendent_of,
};
use client::{
- apply_aux, Client,
+ apply_aux, Client,
};
use grandpa::{
BlockNumberOps, Equivocation, Error as GrandpaError, round::State as RoundState,
@@ -497,8 +497,18 @@ where
// note that we pass the original `best_header`, i.e. before the
// authority set limit filter, which can be considered a
// mandatory/implicit voting rule.
+ //
+ // we also make sure that the restricted vote is higher than the
+ // round base (i.e. last finalized), otherwise the value
+ // returned by the given voting rule is ignored and the original
+ // target is used instead.
self.voting_rule
.restrict_vote(&*self.client, &base_header, &best_header, target_header)
+ .filter(|(_, restricted_number)| {
+ // we can only restrict votes within the interval [base, target]
+ restricted_number >= base_header.number() &&
+ restricted_number < target_header.number()
+ })
.or(Some((target_header.hash(), *target_header.number())))
},
Ok(None) => {
diff --git a/substrate/client/finality-grandpa/src/tests.rs b/substrate/client/finality-grandpa/src/tests.rs
index 6d4d439a9fb..abcd44635cc 100644
--- a/substrate/client/finality-grandpa/src/tests.rs
+++ b/substrate/client/finality-grandpa/src/tests.rs
@@ -1660,6 +1660,19 @@ fn grandpa_environment_respects_voting_rules() {
).unwrap().1,
19,
);
+
+ // we finalize block 20 with block 20 being the best block
+ peer.client().finalize_block(BlockId::Number(20), 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).
+ assert_eq!(
+ default_env.best_chain_containing(
+ peer.client().info().chain.finalized_hash
+ ).unwrap().1,
+ 20,
+ );
}
#[test]
--
GitLab