ban txs if they force to produce empty block or can't fit in a block

client/basic-authorship/src/basic_authorship.rs

@@ -33,7 +33,11 @@ use sc_client_api::backend;
 use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_INFO};
 use sc_transaction_pool_api::{InPoolTransaction, TransactionPool};
 use sp_api::{ApiExt, ProvideRuntimeApi};
-use sp_blockchain::{ApplyExtrinsicFailed::Validity, Error::ApplyExtrinsicFailed, HeaderBackend};
+use sp_blockchain::{
+	ApplyExtrinsicFailed::{TooBigStorageProof, Validity},
+	Error::ApplyExtrinsicFailed,
+	HeaderBackend,
+};
 use sp_consensus::{DisableProofRecording, EnableProofRecording, ProofRecording, Proposal};
 use sp_core::traits::SpawnNamed;
 use sp_inherents::InherentData;
@@ -400,6 +404,7 @@ where
 		let block_timer = time::Instant::now();
 		let mut skipped = 0;
 		let mut unqueue_invalid = Vec::new();
+		let mut suspicious_txs = Vec::new();
 
 		let mut t1 = self.transaction_pool.ready_at(self.parent_number).fuse();
 		let mut t2 =
@@ -438,12 +443,13 @@ where
 			}
 
 			let pending_tx_data = pending_tx.data().clone();
+			let pending_tx_encoded_size = pending_tx_data.encoded_size();
 			let pending_tx_hash = pending_tx.hash().clone();
 
 			let block_size =
 				block_builder.estimate_block_size(self.include_proof_in_block_size_estimation);
 			if let Some(remaining_size) =
-				block_size_limit.checked_sub(block_size + pending_tx_data.encoded_size())
+				block_size_limit.checked_sub(block_size + pending_tx_encoded_size)
 			{
 				// There is enough space left in the block, we push the transaction
 				trace!("[{:?}] Pushing to the block.", pending_tx_hash);
@@ -456,6 +462,18 @@ where
 						transaction_pushed = true;
 						debug!("[{:?}] Pushed to the block.", pending_tx_hash);
 					},
+					Err(ApplyExtrinsicFailed(TooBigStorageProof(proof_diff, _))) => {
+						pending_iterator.report_invalid(&pending_tx);
+						if pending_tx_encoded_size + proof_diff > block_size_limit {
+							// The transaction and its storage proof are too big to be included
+							// in a future block, we must ban the transaction right away
+							debug!("[{:?}] Invalid transaction: too big storage proof", pending_tx_hash);
+							unqueue_invalid.push(pending_tx_hash);
+						} else {
+							// The transaction is suspicious, but it could be a false positive
+							suspicious_txs.push(pending_tx_hash);
+						}
+					},
 					Err(ApplyExtrinsicFailed(Validity(e))) if e.exhausted_resources() => {
 						pending_iterator.report_invalid(&pending_tx);
 						if skipped < MAX_SKIPPED_TRANSACTIONS {
@@ -513,6 +531,12 @@ where
 			}
 		};
 
+		if matches!(end_reason, EndProposingReason::HitDeadline) && !transaction_pushed {
+			warn!("Hit deadline `{}` without including any transaction!", block_size_limit,);
+			// If we hits the hard deadline but the block still empty, we ban suspicious txs
+			self.transaction_pool.remove_invalid(&suspicious_txs);
+		}
+
 		if matches!(end_reason, EndProposingReason::HitBlockSizeLimit) && !transaction_pushed {
 			warn!(
 				"Hit block size limit of `{}` without including any transaction!",

client/block-builder/src/lib.rs

@@ -36,7 +36,6 @@ use sp_core::ExecutionContext;
 use sp_runtime::{
 	legacy,
 	traits::{Block as BlockT, Hash, HashFor, Header as HeaderT, NumberFor, One},
-	transaction_validity::{InvalidTransaction, TransactionValidityError},
 	Digest,
 };
 
@@ -242,9 +241,10 @@ where
 							// we should rollback and return the error
 							// `InvalidTransaction::ExhaustsResources`.
 							return TransactionOutcome::Rollback(Err(
-								ApplyExtrinsicFailed::Validity(TransactionValidityError::Invalid(
-									InvalidTransaction::ExhaustsResources,
-								))
+								ApplyExtrinsicFailed::TooBigStorageProof(
+									proof_diff,
+									proof_diff_limit,
+								)
 								.into(),
 							))
 						}

primitives/blockchain/src/error.rs

@@ -40,6 +40,9 @@ pub enum ApplyExtrinsicFailed {
 
 	#[error("Application specific error")]
 	Application(#[source] Box<dyn 'static + std::error::Error + Send + Sync>),
+
+	#[error("Extrinsic execution generated a too big storage proof: {0}/{1}")]
+	TooBigStorageProof(usize, usize),
 }
 
 /// Substrate Client error