Merge branch 'master' into avoid_stack_protected

Author: cheme

.gitlab-ci.yml

@@ -158,6 +158,10 @@ cargo-audit:
 cargo-deny:
   stage:                           test
   <<:                              *docker-env
+  only:
+    - schedules
+    - tags
+    - web
   script:
     - cargo deny check --hide-inclusion-graph -c .maintain/deny.toml
   after_script:
@@ -259,7 +263,7 @@ test-wasmtime:
   variables:
     <<:                            *default-vars
     # Enable debug assertions since we are running optimized builds for testing
-    # but still want to have debug assertions.
+    # but still want to have debug assertions. 
     RUSTFLAGS: -Cdebug-assertions=y
     RUST_BACKTRACE: 1
   except:
@@ -285,7 +289,7 @@ test-runtime-benchmarks:
       - $DEPLOY_TAG
   script:
     - cd bin/node/cli
-    - WASM_BUILD_NO_COLOR=1 time cargo test --release --verbose --features runtime-benchmarks
+    - WASM_BUILD_NO_COLOR=1 time cargo test --workspace --release --verbose --features runtime-benchmarks
     - sccache -s
 
 test-linux-stable-int:

.maintain/monitoring/alerting-rules/alerting-rule-tests.yaml

@@ -18,14 +18,14 @@ tests:
               pod="polkadot-abcdef01234-abcdef",
               instance="polkadot-abcdef01234-abcdef",
             }'
-            values: '10+1x30' # 10 11 12 13 .. 40
+            values: '11+1x10 22+2x30 10043x5'
 
           - series: 'polkadot_sub_txpool_validations_finished{
               job="polkadot",
               pod="polkadot-abcdef01234-abcdef",
               instance="polkadot-abcdef01234-abcdef",
             }'
-            values: '0x30' # 0 0 0 0 .. 0
+            values: '0+1x42 42x5'
 
           - series: 'polkadot_block_height{
               status="best", job="polkadot",
@@ -161,43 +161,66 @@ tests:
           # Transaction queue
           ######################################################################
 
-          - eval_time: 10m
-            alertname: TransactionQueueSize
-            exp_alerts:
           - eval_time: 11m
-            alertname: TransactionQueueSize
+            alertname: TransactionQueueSizeIncreasing
+            # Number of validations scheduled and finished both grow at a rate
+            # of 1 in the first 10 minutes, thereby the queue is not increasing
+            # in size, thus don't expect an alert.
+            exp_alerts:
+          - eval_time: 22m
+            alertname: TransactionQueueSizeIncreasing
+            # Number of validations scheduled is growing twice as fast as the
+            # number of validations finished after minute 10. Thus expect
+            # warning alert after 20 minutes.
             exp_alerts:
                 - exp_labels:
                       severity: warning
                       pod: polkadot-abcdef01234-abcdef
                       instance: polkadot-abcdef01234-abcdef
                       job: polkadot
                   exp_annotations:
-                      message: "The node polkadot-abcdef01234-abcdef has more
-                      than 10 transactions in the queue for more than 10
-                      minutes"
-
-          - eval_time: 31m
-            alertname: TransactionQueueSize
+                      message: "The transaction pool size on node
+                      polkadot-abcdef01234-abcdef has been monotonically
+                      increasing for the last 10 minutes."
+          - eval_time: 43m
+            alertname: TransactionQueueSizeIncreasing
+            # Number of validations scheduled is growing twice as fast as the
+            # number of validations finished after minute 10. Thus expect
+            # both warning and critical alert after 40 minutes.
             exp_alerts:
                 - exp_labels:
                       severity: warning
                       pod: polkadot-abcdef01234-abcdef
                       instance: polkadot-abcdef01234-abcdef
                       job: polkadot
                   exp_annotations:
-                      message: "The node polkadot-abcdef01234-abcdef has more
-                      than 10 transactions in the queue for more than 10
-                      minutes"
+                      message: "The transaction pool size on node
+                      polkadot-abcdef01234-abcdef has been monotonically
+                      increasing for the last 10 minutes."
+                - exp_labels:
+                      severity: critical
+                      pod: polkadot-abcdef01234-abcdef
+                      instance: polkadot-abcdef01234-abcdef
+                      job: polkadot
+                  exp_annotations:
+                      message: "The transaction pool size on node
+                      polkadot-abcdef01234-abcdef has been monotonically
+                      increasing for the last 30 minutes."
+          - eval_time: 49m
+            alertname: TransactionQueueSizeHigh
+            # After minute 43 the number of validations scheduled jumps up
+            # drastically while the number of validations finished stays the
+            # same. Thus expect an alert.
+            exp_alerts:
                 - exp_labels:
                       severity: critical
                       pod: polkadot-abcdef01234-abcdef
                       instance: polkadot-abcdef01234-abcdef
                       job: polkadot
                   exp_annotations:
-                      message: "The node polkadot-abcdef01234-abcdef has more
-                      than 10 transactions in the queue for more than 30
-                      minutes"
+                      message: "The transaction pool size on node
+                      polkadot-abcdef01234-abcdef has been above 10_000 for the
+                      last 5 minutes."
 
           ######################################################################
           # Networking

.maintain/monitoring/alerting-rules/alerting-rules.yaml

@@ -73,24 +73,33 @@ groups:
   # Transaction queue
   ##############################################################################
 
-  - alert: TransactionQueueSize
-    expr: 'polkadot_sub_txpool_validations_scheduled -
-    polkadot_sub_txpool_validations_finished > 10'
+  - alert: TransactionQueueSizeIncreasing
+    expr: 'increase(polkadot_sub_txpool_validations_scheduled[5m]) -
+    increase(polkadot_sub_txpool_validations_finished[5m]) > 0'
     for: 10m
     labels:
       severity: warning
     annotations:
-      message: 'The node {{ $labels.instance }} has more than 10 transactions in
-      the queue for more than 10 minutes'
-  - alert: TransactionQueueSize
-    expr: 'polkadot_sub_txpool_validations_scheduled -
-    polkadot_sub_txpool_validations_finished > 10'
+      message: 'The transaction pool size on node {{ $labels.instance }} has
+      been monotonically increasing for the last 10 minutes.'
+  - alert: TransactionQueueSizeIncreasing
+    expr: 'increase(polkadot_sub_txpool_validations_scheduled[5m]) -
+    increase(polkadot_sub_txpool_validations_finished[5m]) > 0'
     for: 30m
     labels:
       severity: critical
     annotations:
-      message: 'The node {{ $labels.instance }} has more than 10 transactions in
-      the queue for more than 30 minutes'
+      message: 'The transaction pool size on node {{ $labels.instance }} has
+      been monotonically increasing for the last 30 minutes.'
+  - alert: TransactionQueueSizeHigh
+    expr: 'polkadot_sub_txpool_validations_scheduled -
+    polkadot_sub_txpool_validations_finished > 10000'
+    for: 5m
+    labels:
+      severity: critical
+    annotations:
+      message: 'The transaction pool size on node {{ $labels.instance }} has
+      been above 10_000 for the last 5 minutes.'
 
   ##############################################################################
   # Networking

Cargo.lock

@@ -497,9 +497,7 @@ fn offchain_http_should_work(wasm_method: WasmExecutionMethod) {
 	let mut ext = TestExternalities::default();
 	let (offchain, state) = testing::TestOffchainExt::new();
 	ext.register_extension(OffchainExt::new(offchain));
-	state.write().expect_request(
-		0,
-		testing::PendingRequest {
+	state.write().expect_request(testing::PendingRequest {
 			method: "POST".into(),
 			uri: "http://localhost:12345".into(),
 			body: vec![1, 2, 3, 4],

client/executor/src/integration_tests/mod.rs

@@ -750,7 +750,11 @@ impl<Block: BlockT> Inner<Block> {
 					Round(1),
 				)),
 				Some(ref mut v) => if v.set_id == set_id {
-					if self.authorities != authorities {
+					let diff_authorities =
+						self.authorities.iter().collect::<HashSet<_>>() !=
+						authorities.iter().collect();
+
+					if diff_authorities {
 						debug!(target: "afg",
 							"Gossip validator noted set {:?} twice with different authorities. \
 							Was the authority set hard forked?",
@@ -829,7 +833,7 @@ impl<Block: BlockT> Inner<Block> {
 			return Action::Discard(cost::UNKNOWN_VOTER);
 		}
 
-		if let Err(()) = sp_finality_grandpa::check_message_signature(
+		if !sp_finality_grandpa::check_message_signature(
 			&full.message.message,
 			&full.message.id,
 			&full.message.signature,
@@ -2620,12 +2624,12 @@ mod tests {
 	fn allow_noting_different_authorities_for_same_set() {
 		let (val, _) = GossipValidator::<Block>::new(config(), voter_set_state(), None);
 
-		let a1 = vec![AuthorityId::default()];
+		let a1 = vec![AuthorityId::from_slice(&[0; 32])];
 		val.note_set(SetId(1), a1.clone(), |_, _| {});
 
 		assert_eq!(val.inner().read().authorities, a1);
 
-		let a2 = vec![AuthorityId::default(), AuthorityId::default()];
+		let a2 = vec![AuthorityId::from_slice(&[1; 32]), AuthorityId::from_slice(&[2; 32])];
 		val.note_set(SetId(1), a2.clone(), |_, _| {});
 
 		assert_eq!(val.inner().read().authorities, a2);

client/finality-grandpa/src/communication/gossip.rs

@@ -105,6 +105,34 @@ mod benefit {
 	pub(super) const PER_EQUIVOCATION: i32 = 10;
 }
 
+/// A type that ties together our local authority id and a keystore where it is
+/// available for signing.
+pub struct LocalIdKeystore((AuthorityId, BareCryptoStorePtr));
+
+impl LocalIdKeystore {
+	/// Returns a reference to our local authority id.
+	fn local_id(&self) -> &AuthorityId {
+		&(self.0).0
+	}
+
+	/// Returns a reference to the keystore.
+	fn keystore(&self) -> &BareCryptoStorePtr {
+		&(self.0).1
+	}
+}
+
+impl AsRef<BareCryptoStorePtr> for LocalIdKeystore {
+	fn as_ref(&self) -> &BareCryptoStorePtr {
+		self.keystore()
+	}
+}
+
+impl From<(AuthorityId, BareCryptoStorePtr)> for LocalIdKeystore {
+	fn from(inner: (AuthorityId, BareCryptoStorePtr)) -> LocalIdKeystore {
+		LocalIdKeystore(inner)
+	}
+}
+
 /// If the voter set is larger than this value some telemetry events are not
 /// sent to avoid increasing usage resource on the node and flooding the
 /// telemetry server (e.g. received votes, received commits.)
@@ -272,11 +300,10 @@ impl<B: BlockT, N: Network<B>> NetworkBridge<B, N> {
 	/// network all within the current set.
 	pub(crate) fn round_communication(
 		&self,
-		keystore: Option<BareCryptoStorePtr>,
+		keystore: Option<LocalIdKeystore>,
 		round: Round,
 		set_id: SetId,
 		voters: Arc<VoterSet<AuthorityId>>,
-		local_key: Option<AuthorityId>,
 		has_voted: HasVoted<B>,
 	) -> (
 		impl Stream<Item = SignedMessage<B>> + Unpin,
@@ -288,9 +315,10 @@ impl<B: BlockT, N: Network<B>> NetworkBridge<B, N> {
 			&*voters,
 		);
 
-		let local_id = local_key.and_then(|id| {
-			if voters.contains(&id) {
-				Some(id)
+		let keystore = keystore.and_then(|ks| {
+			let id = ks.local_id();
+			if voters.contains(id) {
+				Some(ks)
 			} else {
 				None
 			}
@@ -350,11 +378,10 @@ impl<B: BlockT, N: Network<B>> NetworkBridge<B, N> {
 
 		let (tx, out_rx) = mpsc::channel(0);
 		let outgoing = OutgoingMessages::<B> {
-			keystore: keystore.clone(),
+			keystore,
 			round: round.0,
 			set_id: set_id.0,
 			network: self.gossip_engine.clone(),
-			local_id,
 			sender: tx,
 			has_voted,
 		};
@@ -629,11 +656,10 @@ pub struct SetId(pub SetIdNumber);
 pub(crate) struct OutgoingMessages<Block: BlockT> {
 	round: RoundNumber,
 	set_id: SetIdNumber,
-	local_id: Option<AuthorityId>,
+	keystore: Option<LocalIdKeystore>,
 	sender: mpsc::Sender<SignedMessage<Block>>,
 	network: Arc<Mutex<GossipEngine<Block>>>,
 	has_voted: HasVoted<Block>,
-	keystore: Option<BareCryptoStorePtr>,
 }
 
 impl<B: BlockT> Unpin for OutgoingMessages<B> {}
@@ -667,19 +693,12 @@ impl<Block: BlockT> Sink<Message<Block>> for OutgoingMessages<Block>
 		}
 
 		// when locals exist, sign messages on import
-		if let Some(ref public) = self.local_id {
-			let keystore = match &self.keystore {
-				Some(keystore) => keystore.clone(),
-				None => {
-					return Err(Error::Signing("Cannot sign without a keystore".to_string()))
-				}
-			};
-
+		if let Some(ref keystore) = self.keystore {
 			let target_hash = *(msg.target().0);
 			let signed = sp_finality_grandpa::sign_message(
-				keystore,
+				keystore.as_ref(),
 				msg,
-				public.clone(),
+				keystore.local_id().clone(),
 				self.round,
 				self.set_id,
 			).ok_or(
@@ -774,7 +793,7 @@ fn check_compact_commit<Block: BlockT>(
 		use crate::communication::gossip::Misbehavior;
 		use finality_grandpa::Message as GrandpaMessage;
 
-		if let Err(()) = sp_finality_grandpa::check_message_signature_with_buffer(
+		if !sp_finality_grandpa::check_message_signature_with_buffer(
 			&GrandpaMessage::Precommit(precommit.clone()),
 			id,
 			sig,
@@ -862,7 +881,7 @@ fn check_catch_up<Block: BlockT>(
 		for (msg, id, sig) in messages {
 			signatures_checked += 1;
 
-			if let Err(()) = sp_finality_grandpa::check_message_signature_with_buffer(
+			if !sp_finality_grandpa::check_message_signature_with_buffer(
 				&msg,
 				id,
 				sig,

client/finality-grandpa/src/communication/mod.rs

@@ -716,12 +716,18 @@ where
 			HasVoted::No => HasVoted::No,
 		};
 
+		// we can only sign when we have a local key in the authority set
+		// and we have a reference to the keystore.
+		let keystore = match (local_key.as_ref(), self.config.keystore.as_ref()) {
+			(Some(id), Some(keystore)) => Some((id.clone(), keystore.clone()).into()),
+			_ => None,
+		};
+
 		let (incoming, outgoing) = self.network.round_communication(
-			self.config.keystore.clone(),
+			keystore,
 			crate::communication::Round(round),
 			crate::communication::SetId(self.set_id),
 			self.voters.clone(),
-			local_key.clone(),
 			has_voted,
 		);
 

client/finality-grandpa/src/environment.rs

@@ -133,14 +133,14 @@ impl<Block: BlockT> GrandpaJustification<Block> {
 		let mut buf = Vec::new();
 		let mut visited_hashes = HashSet::new();
 		for signed in self.commit.precommits.iter() {
-			if sp_finality_grandpa::check_message_signature_with_buffer(
+			if !sp_finality_grandpa::check_message_signature_with_buffer(
 				&finality_grandpa::Message::Precommit(signed.precommit.clone()),
 				&signed.id,
 				&signed.signature,
 				self.round,
 				set_id,
 				&mut buf,
-			).is_err() {
+			) {
 				return Err(ClientError::BadJustification(
 					"invalid signature for precommit in grandpa justification".to_string()));
 			}