Skip to content

Commit

Permalink
Add another condition to the reject-obsolete-parachain-heads extension (
Browse files Browse the repository at this point in the history
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
  • Loading branch information
svyatonik authored and serban300 committed Apr 10, 2024
1 parent 7990efc commit 04c0296
Show file tree
Hide file tree
Showing 13 changed files with 225 additions and 92 deletions.
1 change: 1 addition & 0 deletions bridges/bin/millau/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 16 additions & 0 deletions bridges/bin/runtime-common/src/messages_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand All @@ -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();
}

Expand Down
8 changes: 5 additions & 3 deletions bridges/bin/runtime-common/src/parachains_benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>,
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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)
}
7 changes: 7 additions & 0 deletions bridges/modules/grandpa/src/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
},
Expand Down
2 changes: 1 addition & 1 deletion bridges/modules/grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
2 changes: 1 addition & 1 deletion bridges/modules/messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
16 changes: 8 additions & 8 deletions bridges/modules/parachains/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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! {
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand Down
59 changes: 47 additions & 12 deletions bridges/modules/parachains/src/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
}
},
Expand Down Expand Up @@ -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;

Expand All @@ -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,
Expand All @@ -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())]));
});
}

Expand All @@ -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())]));
});
}

Expand All @@ -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())]
));
});
}
}
Loading

0 comments on commit 04c0296

Please sign in to comment.