From 5359206aae3412ec92a29f7aa6c4dc40b34cecfd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Silva?= <andre.beat@gmail.com>
Date: Thu, 22 Aug 2019 10:35:45 +0200
Subject: [PATCH] grandpa: validate honest out of scope catch ups are for the
previous set (#3453)
---
.../src/communication/gossip.rs | 84 ++++++++++++++++++-
1 file changed, 83 insertions(+), 1 deletion(-)
diff --git a/substrate/core/finality-grandpa/src/communication/gossip.rs b/substrate/core/finality-grandpa/src/communication/gossip.rs
index 20a629b6ae3..5fd09620e92 100644
--- a/substrate/core/finality-grandpa/src/communication/gossip.rs
+++ b/substrate/core/finality-grandpa/src/communication/gossip.rs
@@ -729,7 +729,9 @@ impl<Block: BlockT> Inner<Block> {
// race where the peer sent us the request before it observed that
// we had transitioned to a new set. In this case we charge a lower
// cost.
- if local_view.round.0.saturating_sub(CATCH_UP_THRESHOLD) == 0 {
+ if request.set_id.0.saturating_add(1) == local_view.set_id.0 &&
+ local_view.round.0.saturating_sub(CATCH_UP_THRESHOLD) == 0
+ {
return (None, Action::Discard(cost::HONEST_OUT_OF_SCOPE_CATCH_UP));
}
@@ -1600,4 +1602,84 @@ mod tests {
_ => panic!("expected catch up message"),
};
}
+
+ #[test]
+ fn detects_honest_out_of_scope_catch_requests() {
+ let set_state = voter_set_state();
+ let (val, _) = GossipValidator::<Block>::new(
+ config(),
+ set_state.clone(),
+ );
+
+ // the validator starts at set id 2
+ val.note_set(SetId(2), Vec::new(), |_, _| {});
+
+ // add the peer making the request to the validator,
+ // otherwise it is discarded
+ let peer = PeerId::random();
+ val.inner.write().peers.new_peer(peer.clone());
+
+ let send_request = |set_id, round| {
+ let mut inner = val.inner.write();
+ inner.handle_catch_up_request(
+ &peer,
+ CatchUpRequestMessage {
+ set_id: SetId(set_id),
+ round: Round(round),
+ },
+ &set_state,
+ )
+ };
+
+ let assert_res = |res: (Option<_>, Action<_>), honest| {
+ assert!(res.0.is_none());
+ assert_eq!(
+ res.1,
+ if honest {
+ Action::Discard(cost::HONEST_OUT_OF_SCOPE_CATCH_UP)
+ } else {
+ Action::Discard(Misbehavior::OutOfScopeMessage.cost())
+ },
+ );
+ };
+
+ // the validator is at set id 2 and round 0. requests for set id 1
+ // should not be answered but they should be considered an honest
+ // mistake
+ assert_res(
+ send_request(1, 1),
+ true,
+ );
+
+ assert_res(
+ send_request(1, 10),
+ true,
+ );
+
+ // requests for set id 0 should be considered out of scope
+ assert_res(
+ send_request(0, 1),
+ false,
+ );
+
+ assert_res(
+ send_request(0, 10),
+ false,
+ );
+
+ // after the validator progresses further than CATCH_UP_THRESHOLD in set
+ // id 2, any request for set id 1 should no longer be considered an
+ // honest mistake.
+ val.note_round(Round(3), |_, _| {});
+
+ assert_res(
+ send_request(1, 1),
+ false,
+ );
+
+ assert_res(
+ send_request(1, 2),
+ false,
+ );
+ }
}
--
GitLab