From 0e784ec6494075a3222e26dc43a6346edad24845 Mon Sep 17 00:00:00 2001
From: Svyatoslav Nikolsky <svyatonik@gmail.com>
Date: Wed, 8 May 2024 11:26:57 +0300
Subject: [PATCH] Bridge: check bridge GRANDPA pallet call limits from signed
 extension (#4385)

silent, because it'll be deployed with the
https://github.com/paritytech/polkadot-sdk/pull/4102, where this code
has been introduced

I've planned originally to avoid doing that check in the runtime code,
because it **may be** checked offchain. But actually, the check is quite
cheap and we could do that onchain too.
---
 bridges/modules/grandpa/src/call_ext.rs | 67 ++++++++++++++++++++++++-
 bridges/modules/grandpa/src/lib.rs      |  3 ++
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/bridges/modules/grandpa/src/call_ext.rs b/bridges/modules/grandpa/src/call_ext.rs
index 6fa62ec0cff49..98fbeaa30bbac 100644
--- a/bridges/modules/grandpa/src/call_ext.rs
+++ b/bridges/modules/grandpa/src/call_ext.rs
@@ -148,8 +148,13 @@ impl<T: Config<I>, I: 'static> SubmitFinalityProofHelper<T, I> {
 			}
 		}
 
-		// we do not check whether the header matches free submission criteria here - it is the
-		// relayer responsibility to check that
+		// let's also check whether the header submission fits the hardcoded limits. A normal
+		// relayer would check that before submitting a transaction (since limits are constants
+		// and do not depend on a volatile runtime state), but the ckeck itself is cheap, so
+		// let's do it here too
+		if !call_info.fits_limits() {
+			return Err(Error::<T, I>::HeaderOverflowLimits);
+		}
 
 		Ok(improved_by)
 	}
@@ -468,6 +473,64 @@ mod tests {
 		})
 	}
 
+	#[test]
+	fn extension_rejects_new_header_if_it_overflow_size_limits() {
+		run_test(|| {
+			let mut large_finality_target = test_header(10 + FreeHeadersInterval::get() as u64);
+			large_finality_target
+				.digest_mut()
+				.push(DigestItem::Other(vec![42u8; 1024 * 1024]));
+			let justification_params = JustificationGeneratorParams {
+				header: large_finality_target.clone(),
+				..Default::default()
+			};
+			let large_justification = make_justification_for_header(justification_params);
+
+			let bridge_grandpa_call = crate::Call::<TestRuntime, ()>::submit_finality_proof_ex {
+				finality_target: Box::new(large_finality_target),
+				justification: large_justification,
+				current_set_id: 0,
+				is_free_execution_expected: true,
+			};
+			sync_to_header_10();
+
+			// if overflow size limits => Err
+			FreeHeadersRemaining::<TestRuntime, ()>::put(2);
+			assert!(RuntimeCall::check_obsolete_submit_finality_proof(&RuntimeCall::Grandpa(
+				bridge_grandpa_call.clone(),
+			),)
+			.is_err());
+		})
+	}
+
+	#[test]
+	fn extension_rejects_new_header_if_it_overflow_weight_limits() {
+		run_test(|| {
+			let finality_target = test_header(10 + FreeHeadersInterval::get() as u64);
+			let justification_params = JustificationGeneratorParams {
+				header: finality_target.clone(),
+				ancestors: TestBridgedChain::REASONABLE_HEADERS_IN_JUSTIFICATION_ANCESTRY,
+				..Default::default()
+			};
+			let justification = make_justification_for_header(justification_params);
+
+			let bridge_grandpa_call = crate::Call::<TestRuntime, ()>::submit_finality_proof_ex {
+				finality_target: Box::new(finality_target),
+				justification,
+				current_set_id: 0,
+				is_free_execution_expected: true,
+			};
+			sync_to_header_10();
+
+			// if overflow weight limits => Err
+			FreeHeadersRemaining::<TestRuntime, ()>::put(2);
+			assert!(RuntimeCall::check_obsolete_submit_finality_proof(&RuntimeCall::Grandpa(
+				bridge_grandpa_call.clone(),
+			),)
+			.is_err());
+		})
+	}
+
 	#[test]
 	fn extension_rejects_new_header_if_free_execution_is_requested_and_improved_by_is_below_expected(
 	) {
diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs
index efcbfb1654b34..a927882aaaa27 100644
--- a/bridges/modules/grandpa/src/lib.rs
+++ b/bridges/modules/grandpa/src/lib.rs
@@ -504,6 +504,9 @@ pub mod pallet {
 		/// The submitter wanted free execution, but the difference between best known and
 		/// bundled header numbers is below the `FreeHeadersInterval`.
 		BelowFreeHeaderInterval,
+		/// The header (and its finality) submission overflows hardcoded chain limits: size
+		/// and/or weight are larger than expected.
+		HeaderOverflowLimits,
 	}
 
 	/// Called when new free header is imported.