From ae33b05852cce7ab68ee0001b7bd717f3531c47e Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 6 May 2024 14:02:01 +0300 Subject: [PATCH] check bridge GRANDPA pallet call limits from signed extension --- 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 6fa62ec0cff4..98fbeaa30bba 100644 --- a/bridges/modules/grandpa/src/call_ext.rs +++ b/bridges/modules/grandpa/src/call_ext.rs @@ -148,8 +148,13 @@ impl, I: 'static> SubmitFinalityProofHelper { } } - // 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::::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::::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::::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::::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::::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 cb536eb07ff6..6ae6f093d8bd 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.