Add another condition to the reject-obsolete-parachain-heads extension (

paritytech#1505)

* add another condition to the reject-obsolete-parachain-heads extension

* add tracing to obsolete-tx-extensions

* fix tests

* extension_rejects_header_from_new_relay_block_with_same_hash

* fmt

* fix benchmarks

bridges/bin/millau/runtime/src/lib.rs

@@ -1035,6 +1035,7 @@ impl_runtime_apis! {
  pallet_bridge_parachains::RelayBlockNumber,
  pallet_bridge_parachains::RelayBlockHash,
  bp_polkadot_core::parachains::ParaHeadsProof,
+ Vec<(bp_polkadot_core::parachains::ParaId, bp_polkadot_core::parachains::ParaHash)>,
  ) {
  bridge_runtime_common::parachains_benchmarking::prepare_parachain_heads_proof::<Runtime, WithRialtoParachainsInstance>(
  parachains,

bridges/bin/runtime-common/src/messages_extension.rs

@@ -70,6 +70,14 @@ macro_rules! declare_bridge_reject_obsolete_messages {
 
  let inbound_lane_data = pallet_bridge_messages::InboundLanes::<$runtime, $instance>::get(&proof.lane);
  if proof.nonces_end <= inbound_lane_data.last_delivered_nonce() {
+ log::trace!(
+ target: pallet_bridge_messages::LOG_TARGET,
+ "Rejecting obsolete messages delivery transaction: lane {:?}, bundled {:?}, best {:?}",
+ proof.lane,
+ proof.nonces_end,
+ inbound_lane_data.last_delivered_nonce(),
+ );
+
  return sp_runtime::transaction_validity::InvalidTransaction::Stale.into();
  }
 
@@ -84,6 +92,14 @@ macro_rules! declare_bridge_reject_obsolete_messages {
 
  let outbound_lane_data = pallet_bridge_messages::OutboundLanes::<$runtime, $instance>::get(&proof.lane);
  if latest_delivered_nonce <= outbound_lane_data.latest_received_nonce {
+ log::trace!(
+ target: pallet_bridge_messages::LOG_TARGET,
+ "Rejecting obsolete messages confirmation transaction: lane {:?}, bundled {:?}, best {:?}",
+ proof.lane,
+ latest_delivered_nonce,
+ outbound_lane_data.latest_received_nonce,
+ );
+
  return sp_runtime::transaction_validity::InvalidTransaction::Stale.into();
  }
 

bridges/bin/runtime-common/src/parachains_benchmarking.rs

@@ -21,7 +21,7 @@
 use crate::messages_benchmarking::{grow_trie, insert_header_to_grandpa_pallet};
 
 use bp_parachains::parachain_head_storage_key_at_source;
-use bp_polkadot_core::parachains::{ParaHead, ParaHeadsProof, ParaId};
+use bp_polkadot_core::parachains::{ParaHash, ParaHead, ParaHeadsProof, ParaId};
 use bp_runtime::StorageProofSize;
 use codec::Encode;
 use frame_support::traits::Get;
@@ -37,7 +37,7 @@ pub fn prepare_parachain_heads_proof<R, PI>(
  parachains: &[ParaId],
  parachain_head_size: u32,
  size: StorageProofSize,
-) -> (RelayBlockNumber, RelayBlockHash, ParaHeadsProof)
+) -> (RelayBlockNumber, RelayBlockHash, ParaHeadsProof, Vec<(ParaId, ParaHash)>)
 where
  R: pallet_bridge_parachains::Config<PI>
  + pallet_bridge_grandpa::Config<R::BridgesGrandpaPalletInstance>,
@@ -48,6 +48,7 @@ where
  let parachain_head = ParaHead(vec![0u8; parachain_head_size as usize]);
 
  // insert all heads to the trie
+ let mut parachain_heads = Vec::with_capacity(parachains.len());
  let mut storage_keys = Vec::with_capacity(parachains.len());
  let mut state_root = Default::default();
  let mut mdb = MemoryDB::default();
@@ -62,6 +63,7 @@ where
  .map_err(|_| "TrieMut::insert has failed")
  .expect("TrieMut::insert should not fail in benchmarks");
  storage_keys.push(storage_key);
+ parachain_heads.push((*parachain, parachain_head.hash()))
  }
  }
  state_root = grow_trie(state_root, &mut mdb, size);
@@ -76,5 +78,5 @@ where
  let (relay_block_number, relay_block_hash) =
  insert_header_to_grandpa_pallet::<R, R::BridgesGrandpaPalletInstance>(state_root);
 
- (relay_block_number, relay_block_hash, ParaHeadsProof(proof))
+ (relay_block_number, relay_block_hash, ParaHeadsProof(proof), parachain_heads)
 }

bridges/modules/grandpa/src/extension.rs

@@ -75,6 +75,13 @@ macro_rules! declare_bridge_reject_obsolete_grandpa_header {
  if best_finalized_number < bundled_block_number {
  Ok(sp_runtime::transaction_validity::ValidTransaction::default())
  } else {
+ log::trace!(
+ target: $crate::LOG_TARGET,
+ "Rejecting obsolete bridged header: bundled {:?}, best {:?}",
+ bundled_block_number,
+ best_finalized_number,
+ );
+
  sp_runtime::transaction_validity::InvalidTransaction::Stale.into()
  }
  },

bridges/modules/grandpa/src/lib.rs

@@ -60,7 +60,7 @@ pub use pallet::*;
 pub use weights::WeightInfo;
 
 /// The target that will be used when publishing logs related to this pallet.
-const LOG_TARGET: &str = "runtime::bridge-grandpa";
+pub const LOG_TARGET: &str = "runtime::bridge-grandpa";
 
 /// Block number of the bridged chain.
 pub type BridgedBlockNumber<T, I> = BlockNumberOf<<T as Config<I>>::BridgedChain>;

bridges/modules/messages/src/lib.rs

@@ -91,7 +91,7 @@ mod mock;
 pub use pallet::*;
 
 /// The target that will be used when publishing logs related to this pallet.
-const LOG_TARGET: &str = "runtime::bridge-messages";
+pub const LOG_TARGET: &str = "runtime::bridge-messages";
 
 #[frame_support::pallet]
 pub mod pallet {

bridges/modules/parachains/src/benchmarking.rs

@@ -21,7 +21,7 @@ use crate::{
  RelayBlockNumber,
 };
 
-use bp_polkadot_core::parachains::{ParaHeadsProof, ParaId};
+use bp_polkadot_core::parachains::{ParaHash, ParaHeadsProof, ParaId};
 use bp_runtime::StorageProofSize;
 use frame_benchmarking::{account, benchmarks_instance_pallet};
 use frame_system::RawOrigin;
@@ -37,7 +37,7 @@ pub trait Config<I: 'static>: crate::Config<I> {
  parachains: &[ParaId],
  parachain_head_size: u32,
  proof_size: StorageProofSize,
- ) -> (RelayBlockNumber, RelayBlockHash, ParaHeadsProof);
+ ) -> (RelayBlockNumber, RelayBlockHash, ParaHeadsProof, Vec<(ParaId, ParaHash)>);
 }
 
 benchmarks_instance_pallet! {
@@ -57,13 +57,13 @@ benchmarks_instance_pallet! {
 
  let sender = account("sender", 0, 0);
  let parachains = (1..=p).map(ParaId).collect::<Vec<_>>();
- let (relay_block_number, relay_block_hash, parachain_heads_proof) = T::prepare_parachain_heads_proof(
+ let (relay_block_number, relay_block_hash, parachain_heads_proof, parachains_heads) = T::prepare_parachain_heads_proof(
  &parachains,
  DEFAULT_PARACHAIN_HEAD_SIZE,
  StorageProofSize::Minimal(0),
  );
  let at_relay_block = (relay_block_number, relay_block_hash);
- }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains.clone(), parachain_heads_proof)
+ }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains_heads, parachain_heads_proof)
  verify {
  for parachain in parachains {
  assert!(crate::Pallet::<T, I>::best_parachain_head(parachain).is_some());
@@ -74,13 +74,13 @@ benchmarks_instance_pallet! {
  submit_parachain_heads_with_1kb_proof {
  let sender = account("sender", 0, 0);
  let parachains = vec![ParaId(1)];
- let (relay_block_number, relay_block_hash, parachain_heads_proof) = T::prepare_parachain_heads_proof(
+ let (relay_block_number, relay_block_hash, parachain_heads_proof, parachains_heads) = T::prepare_parachain_heads_proof(
  &parachains,
  DEFAULT_PARACHAIN_HEAD_SIZE,
  StorageProofSize::HasExtraNodes(1024),
  );
  let at_relay_block = (relay_block_number, relay_block_hash);
- }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains.clone(), parachain_heads_proof)
+ }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains_heads, parachain_heads_proof)
  verify {
  for parachain in parachains {
  assert!(crate::Pallet::<T, I>::best_parachain_head(parachain).is_some());
@@ -91,13 +91,13 @@ benchmarks_instance_pallet! {
  submit_parachain_heads_with_16kb_proof {
  let sender = account("sender", 0, 0);
  let parachains = vec![ParaId(1)];
- let (relay_block_number, relay_block_hash, parachain_heads_proof) = T::prepare_parachain_heads_proof(
+ let (relay_block_number, relay_block_hash, parachain_heads_proof, parachains_heads) = T::prepare_parachain_heads_proof(
  &parachains,
  DEFAULT_PARACHAIN_HEAD_SIZE,
  StorageProofSize::HasExtraNodes(16 * 1024),
  );
  let at_relay_block = (relay_block_number, relay_block_hash);
- }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains.clone(), parachain_heads_proof)
+ }: submit_parachain_heads(RawOrigin::Signed(sender), at_relay_block, parachains_heads, parachain_heads_proof)
  verify {
  for parachain in parachains {
  assert!(crate::Pallet::<T, I>::best_parachain_head(parachain).is_some());

bridges/modules/parachains/src/extension.rs

@@ -72,15 +72,34 @@ macro_rules! declare_bridge_reject_obsolete_parachain_header {
  ref parachains,
  ..
  }) if parachains.len() == 1 => {
- let parachain = parachains.get(0).expect("verified by match condition; qed");
+ let (parachain, parachain_head_hash) = parachains.get(0).expect("verified by match condition; qed");
 
  let bundled_relay_block_number = at_relay_block.0;
 
  let best_parachain_head = $crate::BestParaHeads::<$runtime, $instance>::get(parachain);
+
  match best_parachain_head {
  Some(best_parachain_head) if best_parachain_head.at_relay_block_number
- >= bundled_relay_block_number =>
- sp_runtime::transaction_validity::InvalidTransaction::Stale.into(),
+ >= bundled_relay_block_number => {
+ log::trace!(
+ target: $crate::LOG_TARGET,
+ "Rejecting obsolete parachain-head {:?} transaction: bundled relay block number: \
+ {:?} best relay block number: {:?}",
+ parachain,
+ bundled_relay_block_number,
+ best_parachain_head.at_relay_block_number,
+ );
+ sp_runtime::transaction_validity::InvalidTransaction::Stale.into()
+ }
+ Some(best_parachain_head) if best_parachain_head.head_hash == *parachain_head_hash => {
+ log::trace!(
+ target: $crate::LOG_TARGET,
+ "Rejecting obsolete parachain-head {:?} transaction: head hash {:?}",
+ parachain,
+ best_parachain_head.head_hash,
+ );
+ sp_runtime::transaction_validity::InvalidTransaction::Stale.into()
+ }
  _ => Ok(sp_runtime::transaction_validity::ValidTransaction::default()),
  }
  },
@@ -118,7 +137,7 @@ mod tests {
  mock::{run_test, Call, TestRuntime},
  BestParaHead, BestParaHeads, RelayBlockNumber,
  };
- use bp_polkadot_core::parachains::{ParaHeadsProof, ParaId};
+ use bp_polkadot_core::parachains::{ParaHash, ParaHeadsProof, ParaId};
  use frame_support::weights::{DispatchClass, DispatchInfo, Pays};
  use sp_runtime::traits::SignedExtension;
 
@@ -127,7 +146,10 @@ mod tests {
  Call::Parachains => ()
  }
 
- fn validate_submit_parachain_heads(num: RelayBlockNumber, parachains: Vec<ParaId>) -> bool {
+ fn validate_submit_parachain_heads(
+ num: RelayBlockNumber,
+ parachains: Vec<(ParaId, ParaHash)>,
+ ) -> bool {
  BridgeRejectObsoleteParachainHeader
  .validate(
  &42,
@@ -147,29 +169,39 @@ mod tests {
  ParaId(1),
  BestParaHead {
  at_relay_block_number: 10,
- head_hash: Default::default(),
+ head_hash: [1u8; 32].into(),
  next_imported_hash_position: 0,
  },
  );
  }
 
  #[test]
- fn extension_rejects_obsolete_header() {
+ fn extension_rejects_header_from_the_obsolete_relay_block() {
  run_test(|| {
  // when current best finalized is #10 and we're trying to import header#5 => tx is
  // rejected
  sync_to_relay_header_10();
- assert!(!validate_submit_parachain_heads(5, vec![ParaId(1)]));
+ assert!(!validate_submit_parachain_heads(5, vec![(ParaId(1), [1u8; 32].into())]));
+ });
+ }
+
+ #[test]
+ fn extension_rejects_header_from_the_same_relay_block() {
+ run_test(|| {
+ // when current best finalized is #10 and we're trying to import header#10 => tx is
+ // rejected
+ sync_to_relay_header_10();
+ assert!(!validate_submit_parachain_heads(10, vec![(ParaId(1), [1u8; 32].into())]));
  });
  }
 
  #[test]
- fn extension_rejects_same_header() {
+ fn extension_rejects_header_from_new_relay_block_with_same_hash() {
  run_test(|| {
  // when current best finalized is #10 and we're trying to import header#10 => tx is
  // rejected
  sync_to_relay_header_10();
- assert!(!validate_submit_parachain_heads(10, vec![ParaId(1)]));
+ assert!(!validate_submit_parachain_heads(20, vec![(ParaId(1), [1u8; 32].into())]));
  });
  }
 
@@ -179,7 +211,7 @@ mod tests {
  // when current best finalized is #10 and we're trying to import header#15 => tx is
  // accepted
  sync_to_relay_header_10();
- assert!(validate_submit_parachain_heads(15, vec![ParaId(1)]));
+ assert!(validate_submit_parachain_heads(15, vec![(ParaId(1), [2u8; 32].into())]));
  });
  }
 
@@ -189,7 +221,10 @@ mod tests {
  // when current best finalized is #10 and we're trying to import header#5, but another
  // parachain head is also supplied => tx is accepted
  sync_to_relay_header_10();
- assert!(validate_submit_parachain_heads(5, vec![ParaId(1), ParaId(2)]));
+ assert!(validate_submit_parachain_heads(
+ 5,
+ vec![(ParaId(1), [1u8; 32].into()), (ParaId(2), [1u8; 32].into())]
+ ));
  });
  }
 }