From aa8a20c8e10190df2b064f9cadea8b650dc14106 Mon Sep 17 00:00:00 2001 From: Daniel Chmielewski Date: Fri, 5 Apr 2024 21:05:32 +0100 Subject: [PATCH 01/12] Improved comments in bridges/bin/runtime-common/src/integrity.rs. --- bridges/bin/runtime-common/src/integrity.rs | 60 ++++++++++----------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/bridges/bin/runtime-common/src/integrity.rs b/bridges/bin/runtime-common/src/integrity.rs index d3827a14dd6cc..fce5dbc5ce2d4 100644 --- a/bridges/bin/runtime-common/src/integrity.rs +++ b/bridges/bin/runtime-common/src/integrity.rs @@ -16,8 +16,8 @@ //! Integrity tests for chain constants and pallets configuration. //! -//! Most of the tests in this module assume that the bridge is using standard (see `crate::messages` -//! module for details) configuration. +//! Most of the tests in this module assume that the bridge is using standard configuration (see `crate::messages` +//! module for details). use crate::{messages, messages::MessageBridge}; @@ -34,9 +34,9 @@ use pallet_bridge_messages::WeightInfoExt as _; macro_rules! assert_chain_types( ( runtime: $r:path, this_chain: $this:path ) => { { - // if one of asserts fail, then either bridge isn't configured properly (or alternatively - non-standard - // configuration is used), or something has broke existing configuration (meaning that all bridged chains - // and relays will stop functioning) + // If one of the asserts fails, then either the bridge isn't configured properly (or alternatively, a non-standard + // configuration is used), or something has broken the existing configuration (meaning that all bridged chains + // and relays will stop functioning). use frame_system::{Config as SystemConfig, pallet_prelude::{BlockNumberFor, HeaderFor}}; use static_assertions::assert_type_eq_all; @@ -50,15 +50,15 @@ macro_rules! assert_chain_types( } ); -/// Macro that ensures that the bridge GRANDPA pallet is configured properly to bridge with given +/// Macro that ensures that the bridge GRANDPA pallet is configured properly to bridge with a given /// chain. #[macro_export] macro_rules! assert_bridge_grandpa_pallet_types( ( runtime: $r:path, with_bridged_chain_grandpa_instance: $i:path, bridged_chain: $bridged:path ) => { { - // if one of asserts fail, then either bridge isn't configured properly (or alternatively - non-standard - // configuration is used), or something has broke existing configuration (meaning that all bridged chains - // and relays will stop functioning) + // If one of the asserts fails, then either the bridge isn't configured properly (or alternatively, a non-standard + // configuration is used), or something has broken the existing configuration (meaning that all bridged chains + // and relays will stop functioning). use pallet_bridge_grandpa::Config as GrandpaConfig; use static_assertions::assert_type_eq_all; @@ -67,7 +67,7 @@ macro_rules! assert_bridge_grandpa_pallet_types( } ); -/// Macro that ensures that the bridge messages pallet is configured properly to bridge using given +/// Macro that ensures that the bridge messages pallet is configured properly to bridge using a given /// configuration. #[macro_export] macro_rules! assert_bridge_messages_pallet_types( @@ -77,9 +77,9 @@ macro_rules! assert_bridge_messages_pallet_types( bridge: $bridge:path ) => { { - // if one of asserts fail, then either bridge isn't configured properly (or alternatively - non-standard - // configuration is used), or something has broke existing configuration (meaning that all bridged chains - // and relays will stop functioning) + // If one of the asserts fails, then either the bridge isn't configured properly (or alternatively, a non-standard + // configuration is used), or something has broken the existing configuration (meaning that all bridged chains + // and relays will stop functioning). use $crate::messages::{ source::{FromThisChainMessagePayload, TargetHeaderChainAdapter}, target::{FromBridgedChainMessagePayload, SourceHeaderChainAdapter}, @@ -100,7 +100,7 @@ macro_rules! assert_bridge_messages_pallet_types( /// Macro that combines four other macro calls - `assert_chain_types`, `assert_bridge_types`, /// `assert_bridge_grandpa_pallet_types` and `assert_bridge_messages_pallet_types`. It may be used -/// at the chain that is implementing complete standard messages bridge (i.e. with bridge GRANDPA +/// at the chain that is implementing a complete standard messages bridge (i.e. with bridge GRANDPA /// and messages pallets deployed). #[macro_export] macro_rules! assert_complete_bridge_types( @@ -139,17 +139,17 @@ pub struct AssertChainConstants { /// /// In particular, this test ensures that: /// -/// 1) block weight limits are matching; -/// 2) block size limits are matching. +/// 1) Block weight limits are matching; +/// 2) Block size limits are matching. pub fn assert_chain_constants(params: AssertChainConstants) where R: frame_system::Config, { - // we don't check runtime version here, because in our case we'll be building relay from one - // repo and runtime will live in another repo, along with outdated relay version. To avoid + // We don't check runtime version here, because in our case we'll be building the relay from one + // repo and the runtime will live in another repo, along with an outdated relay version. To avoid // unneeded commits, let's not raise an error in case of version mismatch. - // if one of following assert fails, it means that we may need to upgrade bridged chain and + // If one of the following asserts fails, it means that we may need to upgrade the bridged chain and // relay to use updated constants. If constants are now smaller than before, it may lead to // undeliverable messages. @@ -161,7 +161,7 @@ where R::BlockLength::get(), params.block_length, ); - // `BlockWeights` struct is not implementing `PartialEq`, so we compare encoded values here + // `BlockWeights` struct is not implementing `PartialEq`, so we compare encoded values here. assert_eq!( R::BlockWeights::get().encode(), params.block_weights.encode(), @@ -171,7 +171,7 @@ where ); } -/// Test that the constants, used in GRANDPA pallet configuration are valid. +/// Test that the constants used in GRANDPA pallet configuration are valid. pub fn assert_bridge_grandpa_pallet_constants() where R: pallet_bridge_grandpa::Config, @@ -196,7 +196,7 @@ pub struct AssertBridgeMessagesPalletConstants { pub bridged_chain_id: ChainId, } -/// Test that the constants, used in messages pallet configuration are valid. +/// Test that the constants used in messages pallet configuration are valid. pub fn assert_bridge_messages_pallet_constants(params: AssertBridgeMessagesPalletConstants) where R: pallet_bridge_messages::Config, @@ -297,18 +297,18 @@ pub fn check_message_lane_weights< bridged_chain_extra_storage_proof_size: u32, this_chain_max_unrewarded_relayers: MessageNonce, this_chain_max_unconfirmed_messages: MessageNonce, - // whether `RefundBridgedParachainMessages` extension is deployed at runtime and is used for - // refunding this bridge transactions? + // Whether `RefundBridgedParachainMessages` extension is deployed at runtime and is used for + // refunding bridge transactions? // - // in other words: pass true for all known production chains + // In other words: pass true for all known production chains. runtime_includes_refund_extension: bool, ) { type Weights = >::WeightInfo; - // check basic weight assumptions + // Check basic weight assumptions. pallet_bridge_messages::ensure_weights_are_correct::>(); - // check that weights allow us to receive messages + // Check that weights allow us to receive messages. let max_incoming_message_proof_size = bridged_chain_extra_storage_proof_size .saturating_add(messages::target::maximal_incoming_message_size(C::max_extrinsic_size())); pallet_bridge_messages::ensure_able_to_receive_message::>( @@ -318,7 +318,7 @@ pub fn check_message_lane_weights< messages::target::maximal_incoming_message_dispatch_weight(C::max_extrinsic_weight()), ); - // check that weights allow us to receive delivery confirmations + // Check that weights allow us to receive delivery confirmations. let max_incoming_inbound_lane_data_proof_size = InboundLaneData::<()>::encoded_size_hint_u32(this_chain_max_unrewarded_relayers as _); pallet_bridge_messages::ensure_able_to_receive_confirmation::>( @@ -329,10 +329,10 @@ pub fn check_message_lane_weights< this_chain_max_unconfirmed_messages, ); - // check that extra weights of delivery/confirmation transactions include the weight + // Check that extra weights of delivery/confirmation transactions include the weight // of `RefundBridgedParachainMessages` operations. This signed extension assumes the worst case // (i.e. slashing if delivery transaction was invalid) and refunds some weight if - // assumption was wrong (i.e. if we did refund instead of slashing). This check + // the assumption was wrong (i.e. if we did refund instead of slashing). This check // ensures the extension will not refund weight when it doesn't need to (i.e. if pallet // weights do not account weights of refund extension). if runtime_includes_refund_extension { From 1cc6092aec232521867bcb2116424dd2f3860016 Mon Sep 17 00:00:00 2001 From: Daniel Chmielewski Date: Fri, 5 Apr 2024 21:07:27 +0100 Subject: [PATCH 02/12] Improved comments in bridges/bin/runtime-common/src/lib.rs. --- bridges/bin/runtime-common/src/lib.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bridges/bin/runtime-common/src/lib.rs b/bridges/bin/runtime-common/src/lib.rs index 2722f6f1c6d14..d2233e5afc463 100644 --- a/bridges/bin/runtime-common/src/lib.rs +++ b/bridges/bin/runtime-common/src/lib.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . -//! Common types/functions that may be used by runtimes of all bridged chains. +//! Common types and functions that may be used by runtimes of all bridged chains. #![warn(missing_docs)] #![cfg_attr(not(feature = "std"), no_std)] @@ -43,8 +43,8 @@ const LOG_TARGET_BRIDGE_DISPATCH: &str = "runtime::bridge-dispatch"; /// A duplication of the `FilterCall` trait. /// -/// We need this trait in order to be able to implement it for the messages pallet, -/// since the implementation is done outside of the pallet crate. +/// We need this trait to be able to implement it for the messages pallet, since the implementation +/// is done outside of the pallet crate. pub trait BridgeRuntimeFilterCall { /// Checks if a runtime call is valid. fn validate(call: &Call) -> TransactionValidity; @@ -76,10 +76,10 @@ impl, I: 'static> BridgeRuntimeFilterCall, { - /// Validate messages in order to avoid "mining" messages delivery and delivery confirmation - /// transactions, that are delivering outdated messages/confirmations. Without this validation, - /// even honest relayers may lose their funds if there are multiple relays running and - /// submitting the same messages/confirmations. + /// Validate messages to avoid "mining" message delivery and delivery confirmation transactions, + /// that deliver outdated messages/confirmations. Without this validation, even honest relayers + /// may lose their funds if there are multiple relays running and submitting the same + /// messages/confirmations. fn validate(call: &T::RuntimeCall) -> TransactionValidity { call.check_obsolete_call() } From 816d75a97b437cb37de57f61da950356c6374954 Mon Sep 17 00:00:00 2001 From: Daniel Chmielewski Date: Fri, 5 Apr 2024 21:18:33 +0100 Subject: [PATCH 03/12] Improved comments in bridges/bin/runtime-common/src/messages_api.rs. --- bridges/bin/runtime-common/src/messages_api.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bridges/bin/runtime-common/src/messages_api.rs b/bridges/bin/runtime-common/src/messages_api.rs index 7fbdeb3661247..79cecc5153fbf 100644 --- a/bridges/bin/runtime-common/src/messages_api.rs +++ b/bridges/bin/runtime-common/src/messages_api.rs @@ -37,8 +37,8 @@ where pallet_bridge_messages::Pallet::::outbound_message_data(lane, nonce)?; Some(OutboundMessageDetails { nonce, - // dispatch message weight is always zero at the source chain, since we're paying for - // dispatch at the target chain + // Dispatch message weight is always zero at the source chain, since we're paying for + // dispatch at the target chain. dispatch_weight: frame_support::weights::Weight::zero(), size: message_data.len() as _, }) From 8357bcf4d4cbe4811ec5692fddd5a48219f29a5b Mon Sep 17 00:00:00 2001 From: Daniel Chmielewski Date: Fri, 5 Apr 2024 21:24:31 +0100 Subject: [PATCH 04/12] Improved comments in bridges/bin/runtime-common/src/messages_benchmarking.rs. --- .../src/messages_benchmarking.rs | 72 +++++++++---------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/bridges/bin/runtime-common/src/messages_benchmarking.rs b/bridges/bin/runtime-common/src/messages_benchmarking.rs index 0c7a9ad1a83d6..2c3db7ad4c085 100644 --- a/bridges/bin/runtime-common/src/messages_benchmarking.rs +++ b/bridges/bin/runtime-common/src/messages_benchmarking.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . -//! Everything required to run benchmarks of messages module, based on +//! Everything required to run benchmarks of the messages module, based on //! `bridge_runtime_common::messages` implementation. #![cfg(feature = "runtime-benchmarks")] @@ -40,23 +40,23 @@ use sp_runtime::traits::{Header, Zero}; use sp_std::prelude::*; use xcm::latest::prelude::*; -/// Prepare inbound bridge message according to given message proof parameters. +/// Prepare an inbound bridge message according to given message proof parameters. fn prepare_inbound_message( params: &MessageProofParams, successful_dispatch_message_generator: impl Fn(usize) -> MessagePayload, ) -> MessagePayload { - // we only care about **this** message size when message proof needs to be `Minimal` + // We only care about **this** message size when the message proof needs to be `Minimal`. let expected_size = match params.size { StorageProofSize::Minimal(size) => size as usize, _ => 0, }; - // if we don't need a correct message, then we may just return some random blob + // If we don't need a correct message, then we may just return some random blob. if !params.is_successful_dispatch_expected { return vec![0u8; expected_size] } - // else let's prepare successful message. + // Else, let's prepare a successful message. let msg = successful_dispatch_message_generator(expected_size); assert!( msg.len() >= expected_size, @@ -67,12 +67,12 @@ fn prepare_inbound_message( msg } -/// Prepare proof of messages for the `receive_messages_proof` call. +/// Prepare a proof of messages for the `receive_messages_proof` call. /// -/// In addition to returning valid messages proof, environment is prepared to verify this message -/// proof. +/// In addition to returning a valid messages proof, the environment is prepared to verify this +/// message proof. /// -/// This method is intended to be used when benchmarking pallet, linked to the chain that +/// This method is intended to be used when benchmarking a pallet, linked to the chain that /// uses GRANDPA finality. For parachains, please use the `prepare_message_proof_from_parachain` /// function. pub fn prepare_message_proof_from_grandpa_chain( @@ -84,7 +84,7 @@ where FI: 'static, B: MessageBridge, { - // prepare storage proof + // Prepare the storage proof. let (state_root, storage_proof) = prepare_messages_storage_proof::( params.lane, params.message_nonces.clone(), @@ -95,7 +95,7 @@ where encode_lane_data, ); - // update runtime storage + // Update the runtime storage. let (_, bridged_header_hash) = insert_header_to_grandpa_pallet::(state_root); ( @@ -110,12 +110,12 @@ where ) } -/// Prepare proof of messages for the `receive_messages_proof` call. +/// Prepare a proof of messages for the `receive_messages_proof` call. /// -/// In addition to returning valid messages proof, environment is prepared to verify this message -/// proof. +/// In addition to returning a valid messages proof, the environment is prepared to verify this +/// message proof. /// -/// This method is intended to be used when benchmarking pallet, linked to the chain that +/// This method is intended to be used when benchmarking a pallet, linked to the chain that /// uses parachain finality. For GRANDPA chains, please use the /// `prepare_message_proof_from_grandpa_chain` function. pub fn prepare_message_proof_from_parachain( @@ -128,7 +128,7 @@ where B: MessageBridge, UnderlyingChainOf>: Chain + Parachain, { - // prepare storage proof + // Prepare the storage proof. let (state_root, storage_proof) = prepare_messages_storage_proof::( params.lane, params.message_nonces.clone(), @@ -139,7 +139,7 @@ where encode_lane_data, ); - // update runtime storage + // Update the runtime storage. let (_, bridged_header_hash) = insert_header_to_parachains_pallet::>>(state_root); @@ -155,9 +155,9 @@ where ) } -/// Prepare proof of messages delivery for the `receive_messages_delivery_proof` call. +/// Prepare a proof of messages delivery for the `receive_messages_delivery_proof` call. /// -/// This method is intended to be used when benchmarking pallet, linked to the chain that +/// This method is intended to be used when benchmarking a pallet, linked to the chain that /// uses GRANDPA finality. For parachains, please use the /// `prepare_message_delivery_proof_from_parachain` function. pub fn prepare_message_delivery_proof_from_grandpa_chain( @@ -168,7 +168,7 @@ where FI: 'static, B: MessageBridge, { - // prepare storage proof + // Prepare the storage proof. let lane = params.lane; let (state_root, storage_proof) = prepare_message_delivery_storage_proof::( params.lane, @@ -176,7 +176,7 @@ where params.size, ); - // update runtime storage + // Update the runtime storage. let (_, bridged_header_hash) = insert_header_to_grandpa_pallet::(state_root); FromBridgedChainMessagesDeliveryProof { @@ -186,9 +186,9 @@ where } } -/// Prepare proof of messages delivery for the `receive_messages_delivery_proof` call. +/// Prepare a proof of messages delivery for the `receive_messages_delivery_proof` call. /// -/// This method is intended to be used when benchmarking pallet, linked to the chain that +/// This method is intended to be used when benchmarking a pallet, linked to the chain that /// uses parachain finality. For GRANDPA chains, please use the /// `prepare_message_delivery_proof_from_grandpa_chain` function. pub fn prepare_message_delivery_proof_from_parachain( @@ -200,7 +200,7 @@ where B: MessageBridge, UnderlyingChainOf>: Chain + Parachain, { - // prepare storage proof + // Prepare the storage proof. let lane = params.lane; let (state_root, storage_proof) = prepare_message_delivery_storage_proof::( params.lane, @@ -208,7 +208,7 @@ where params.size, ); - // update runtime storage + // Update the runtime storage. let (_, bridged_header_hash) = insert_header_to_parachains_pallet::>>(state_root); @@ -219,7 +219,7 @@ where } } -/// Insert header to the bridge GRANDPA pallet. +/// Insert a header into the bridge GRANDPA pallet. pub(crate) fn insert_header_to_grandpa_pallet( state_root: bp_runtime::HashOf, ) -> (bp_runtime::BlockNumberOf, bp_runtime::HashOf) @@ -241,7 +241,7 @@ where (bridged_block_number, bridged_header_hash) } -/// Insert header to the bridge parachains pallet. +/// Insert a header into the bridge parachains pallet. pub(crate) fn insert_header_to_parachains_pallet( state_root: bp_runtime::HashOf, ) -> (bp_runtime::BlockNumberOf, bp_runtime::HashOf) @@ -263,21 +263,21 @@ where (bridged_block_number, bridged_header_hash) } -/// Returns callback which generates `BridgeMessage` from Polkadot XCM builder based on -/// `expected_message_size` for benchmark. +/// Returns a callback which generates a `BridgeMessage` from the Polkadot XCM builder based on +/// the `expected_message_size` for benchmarking. pub fn generate_xcm_builder_bridge_message_sample( destination: InteriorLocation, ) -> impl Fn(usize) -> MessagePayload { move |expected_message_size| -> MessagePayload { // For XCM bridge hubs, it is the message that - // will be pushed further to some XCM queue (XCMP/UMP) + // will be pushed further to some XCM queue (XCMP/UMP). let location = xcm::VersionedInteriorLocation::V4(destination.clone()); let location_encoded_size = location.encoded_size(); - // we don't need to be super-precise with `expected_size` here + // We don't need to be super-precise with `expected_size` here. let xcm_size = expected_message_size.saturating_sub(location_encoded_size); let xcm_data_size = xcm_size.saturating_sub( - // minus empty instruction size + // Minus empty instruction size. Instruction::<()>::ExpectPallet { index: 0, name: vec![], @@ -305,10 +305,10 @@ pub fn generate_xcm_builder_bridge_message_sample( .into(), ); - // this is the `BridgeMessage` from polkadot xcm builder, but it has no constructor - // or public fields, so just tuple - // (double encoding, because `.encode()` is called on original Xcm BLOB when it is pushed - // to the storage) + // This is the `BridgeMessage` from the Polkadot XCM builder, but it has no constructor + // Or public fields, so just a tuple. + // (Double encoding, because `.encode()` is called on the original XCM BLOB when it is pushed + // to the storage). (location, xcm).encode().encode() } } From 791fc9b30d4acb5c84c165b61c8b1be3cc76b68d Mon Sep 17 00:00:00 2001 From: Daniel Chmielewski Date: Fri, 5 Apr 2024 21:32:15 +0100 Subject: [PATCH 05/12] Improved comments in bridges/bin/runtime-common/src/messages_call_ext.rs. --- .../runtime-common/src/messages_call_ext.rs | 86 +++++++++---------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/bridges/bin/runtime-common/src/messages_call_ext.rs b/bridges/bin/runtime-common/src/messages_call_ext.rs index fb07f7b6dd691..0b91e74b4b415 100644 --- a/bridges/bin/runtime-common/src/messages_call_ext.rs +++ b/bridges/bin/runtime-common/src/messages_call_ext.rs @@ -67,7 +67,7 @@ pub struct UnrewardedRelayerOccupation { /// Info about a `ReceiveMessagesProof` call which tries to update a single lane. #[derive(PartialEq, RuntimeDebug)] pub struct ReceiveMessagesProofInfo { - /// Base messages proof info + /// Base messages proof info. pub base: BaseMessagesProofInfo, /// State of unrewarded relayers vector. pub unrewarded_relayers: UnrewardedRelayerOccupation, @@ -76,30 +76,30 @@ pub struct ReceiveMessagesProofInfo { impl ReceiveMessagesProofInfo { /// Returns true if: /// - /// - either inbound lane is ready to accept bundled messages; + /// - Either inbound lane is ready to accept bundled messages; /// - /// - or there are no bundled messages, but the inbound lane is blocked by too many unconfirmed + /// - Or there are no bundled messages, but the inbound lane is blocked by too many unconfirmed /// messages and/or unrewarded relayers. fn is_obsolete(&self, is_dispatcher_active: bool) -> bool { - // if dispatcher is inactive, we don't accept any delivery transactions + // If dispatcher is inactive, we don't accept any delivery transactions. if !is_dispatcher_active { return true } - // transactions with zero bundled nonces are not allowed, unless they're message + // Transactions with zero bundled nonces are not allowed, unless they're message // delivery transactions, which brings reward confirmations required to unblock - // the lane + // the lane. if self.base.bundled_range.is_empty() { let empty_transactions_allowed = - // we allow empty transactions when we can't accept delivery from new relayers + // We allow empty transactions when we can't accept delivery from new relayers. self.unrewarded_relayers.free_relayer_slots == 0 || - // or if we can't accept new messages at all + // Or if we can't accept new messages at all. self.unrewarded_relayers.free_message_slots == 0; return !empty_transactions_allowed } - // otherwise we require bundled messages to continue stored range + // Otherwise, we require bundled messages to continue stored range. !self.base.appends_to_stored_nonce() } } @@ -143,9 +143,9 @@ pub struct CallHelper, I: 'static> { impl, I: 'static> CallHelper { /// Returns true if: /// - /// - call is `receive_messages_proof` and all messages have been delivered; + /// - Call is `receive_messages_proof` and all messages have been delivered. /// - /// - call is `receive_messages_delivery_proof` and all messages confirmations have been + /// - Call is `receive_messages_delivery_proof` and all messages confirmations have been /// received. pub fn was_successful(info: &CallInfo) -> bool { match info { @@ -155,10 +155,10 @@ impl, I: 'static> CallHelper { if info.base.bundled_range.is_empty() { let post_occupation = unrewarded_relayers_occupation::(&inbound_lane_data); - // we don't care about `free_relayer_slots` here - it is checked in + // We don't care about `free_relayer_slots` here - it is checked in // `is_obsolete` and every relayer has delivered at least one message, // so if relayer slots are released, then message slots are also - // released + // released. return post_occupation.free_message_slots > info.unrewarded_relayers.free_message_slots } @@ -195,16 +195,16 @@ pub trait MessagesCallSubType, I: 'static>: /// Ensures that a `ReceiveMessagesProof` or a `ReceiveMessagesDeliveryProof` call: /// - /// - does not deliver already delivered messages. We require all messages in the - /// `ReceiveMessagesProof` call to be undelivered; + /// - Does not deliver already delivered messages. We require all messages in the + /// `ReceiveMessagesProof` call to be undelivered. /// - /// - does not submit empty `ReceiveMessagesProof` call with zero messages, unless the lane - /// needs to be unblocked by providing relayer rewards proof; + /// - Does not submit empty `ReceiveMessagesProof` call with zero messages, unless the lane + /// needs to be unblocked by providing relayer rewards proof. /// - /// - brings no new delivery confirmations in a `ReceiveMessagesDeliveryProof` call. We require - /// at least one new delivery confirmation in the unrewarded relayers set; + /// - Brings no new delivery confirmations in a `ReceiveMessagesDeliveryProof` call. We require + /// at least one new delivery confirmation in the unrewarded relayers set. /// - /// - does not violate some basic (easy verifiable) messages pallet rules obsolete (like + /// - Does not violate some basic (easy verifiable) messages pallet rules obsolete (like /// submitting a call when a pallet is halted or delivering messages when a dispatcher is /// inactive). /// @@ -239,7 +239,7 @@ impl< return Some(ReceiveMessagesProofInfo { base: BaseMessagesProofInfo { lane_id: proof.lane, - // we want all messages in this range to be new for us. Otherwise transaction + // We want all messages in this range to be new for us. Otherwise, transaction // will be considered obsolete. bundled_range: proof.nonces_start..=proof.nonces_end, best_stored_nonce: inbound_lane_data.last_delivered_nonce(), @@ -262,9 +262,9 @@ impl< return Some(ReceiveMessagesDeliveryProofInfo(BaseMessagesProofInfo { lane_id: proof.lane, - // there's a time frame between message delivery, message confirmation and reward + // There's a time frame between message delivery, message confirmation, and reward // confirmation. Because of that, we can't assume that our state has been confirmed - // to the bridged chain. So we are accepting any proof that brings new + // to the bridged chain. So, we are accepting any proof that brings new // confirmations. bundled_range: outbound_lane_data.latest_received_nonce + 1..= relayers_state.last_delivered_nonce, @@ -434,8 +434,8 @@ mod tests { #[test] fn extension_rejects_obsolete_messages() { sp_io::TestExternalities::new(Default::default()).execute_with(|| { - // when current best delivered is message#10 and we're trying to deliver messages 8..=9 - // => tx is rejected + // When current best delivered is message#10 and we're trying to deliver messages 8..=9, + // the transaction is rejected. deliver_message_10(); assert!(!validate_message_delivery(8, 9)); }); @@ -444,8 +444,8 @@ mod tests { #[test] fn extension_rejects_same_message() { sp_io::TestExternalities::new(Default::default()).execute_with(|| { - // when current best delivered is message#10 and we're trying to import messages 10..=10 - // => tx is rejected + // When current best delivered is message#10 and we're trying to import messages 10..=10, + // the transaction is rejected. deliver_message_10(); assert!(!validate_message_delivery(8, 10)); }); @@ -454,8 +454,8 @@ mod tests { #[test] fn extension_rejects_call_with_some_obsolete_messages() { sp_io::TestExternalities::new(Default::default()).execute_with(|| { - // when current best delivered is message#10 and we're trying to deliver messages - // 10..=15 => tx is rejected + // When current best delivered is message#10 and we're trying to deliver messages + // 10..=15, the transaction is rejected. deliver_message_10(); assert!(!validate_message_delivery(10, 15)); }); @@ -464,8 +464,8 @@ mod tests { #[test] fn extension_rejects_call_with_future_messages() { sp_io::TestExternalities::new(Default::default()).execute_with(|| { - // when current best delivered is message#10 and we're trying to deliver messages - // 13..=15 => tx is rejected + // When current best delivered is message#10 and we're trying to deliver messages + // 13..=15, the transaction is rejected. deliver_message_10(); assert!(!validate_message_delivery(13, 15)); }); @@ -474,8 +474,8 @@ mod tests { #[test] fn extension_reject_call_when_dispatcher_is_inactive() { sp_io::TestExternalities::new(Default::default()).execute_with(|| { - // when current best delivered is message#10 and we're trying to deliver message 11..=15 - // => tx is accepted, but we have inactive dispatcher, so... + // When current best delivered is message#10 and we're trying to deliver message 11..=15, + // the transaction is accepted, but we have inactive dispatcher, so... deliver_message_10(); DummyMessageDispatch::deactivate(); @@ -517,8 +517,8 @@ mod tests { #[test] fn extension_accepts_new_messages() { sp_io::TestExternalities::new(Default::default()).execute_with(|| { - // when current best delivered is message#10 and we're trying to deliver message 11..=15 - // => tx is accepted + // When current best delivered is message#10 and we're trying to deliver message 11..=15, + // the transaction is accepted. deliver_message_10(); assert!(validate_message_delivery(11, 15)); }); @@ -556,8 +556,8 @@ mod tests { #[test] fn extension_rejects_obsolete_confirmations() { sp_io::TestExternalities::new(Default::default()).execute_with(|| { - // when current best confirmed is message#10 and we're trying to confirm message#5 => tx - // is rejected + // When current best confirmed is message#10 and we're trying to confirm message#5, the transaction + // is rejected. confirm_message_10(); assert!(!validate_message_confirmation(5)); }); @@ -566,8 +566,8 @@ mod tests { #[test] fn extension_rejects_same_confirmation() { sp_io::TestExternalities::new(Default::default()).execute_with(|| { - // when current best confirmed is message#10 and we're trying to confirm message#10 => - // tx is rejected + // When current best confirmed is message#10 and we're trying to confirm message#10, + // the transaction is rejected. confirm_message_10(); assert!(!validate_message_confirmation(10)); }); @@ -585,8 +585,8 @@ mod tests { #[test] fn extension_accepts_new_confirmation() { sp_io::TestExternalities::new(Default::default()).execute_with(|| { - // when current best confirmed is message#10 and we're trying to confirm message#15 => - // tx is accepted + // When current best confirmed is message#10 and we're trying to confirm message#15, + // the transaction is accepted. confirm_message_10(); assert!(validate_message_confirmation(15)); }); @@ -601,10 +601,10 @@ mod tests { base: BaseMessagesProofInfo { lane_id: LaneId([0, 0, 0, 0]), bundled_range, - best_stored_nonce: 0, // doesn't matter for `was_successful` + best_stored_nonce: 0, // Doesn't matter for `was_successful`. }, unrewarded_relayers: UnrewardedRelayerOccupation { - free_relayer_slots: 0, // doesn't matter for `was_successful` + free_relayer_slots: 0, // Doesn't matter for `was_successful`. free_message_slots: if is_empty { 0 } else { From a8df8b58b31352fa7b49efce0dd2ef83cdccf689 Mon Sep 17 00:00:00 2001 From: Daniel Chmielewski Date: Fri, 5 Apr 2024 21:33:35 +0100 Subject: [PATCH 06/12] Improved comments in bridges/bin/runtime-common/src/messages_generation.rs. --- .../bin/runtime-common/src/messages_generation.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bridges/bin/runtime-common/src/messages_generation.rs b/bridges/bin/runtime-common/src/messages_generation.rs index c37aaa5d4d537..4f508ae22d2dc 100644 --- a/bridges/bin/runtime-common/src/messages_generation.rs +++ b/bridges/bin/runtime-common/src/messages_generation.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . -//! Helpers for generating message storage proofs, that are used by tests and by benchmarks. +//! Helpers for generating message storage proofs used by tests and benchmarks. use crate::messages::{AccountIdOf, BridgedChain, HashOf, HasherOf, MessageBridge, ThisChain}; @@ -53,7 +53,7 @@ where B: MessageBridge, HashOf>: Copy + Default, { - // prepare Bridged chain storage with messages and (optionally) outbound lane state + // Prepare bridged chain storage with messages and (optionally) outbound lane state. let message_count = message_nonces.end().saturating_sub(*message_nonces.start()) + 1; let mut storage_keys = Vec::with_capacity(message_count as usize + 1); let mut root = Default::default(); @@ -62,7 +62,7 @@ where let mut trie = TrieDBMutBuilderV1::>>::new(&mut mdb, &mut root).build(); - // insert messages + // Insert messages. for (i, nonce) in message_nonces.into_iter().enumerate() { let message_key = MessageKey { lane_id: lane, nonce }; let message_payload = match encode_message(nonce, &message_payload) { @@ -86,7 +86,7 @@ where storage_keys.push(storage_key); } - // insert outbound lane state + // Insert outbound lane state. if let Some(outbound_lane_data) = outbound_lane_data.as_ref().map(encode_outbound_lane_data) { let storage_key = @@ -98,7 +98,7 @@ where } } - // generate storage proof to be delivered to This chain + // Generate storage proof to be delivered to this chain. let storage_proof = record_all_trie_keys::>>, _>(&mdb, &root) .map_err(|_| "record_all_trie_keys has failed") .expect("record_all_trie_keys should not fail in benchmarks"); @@ -116,7 +116,7 @@ pub fn prepare_message_delivery_storage_proof( where B: MessageBridge, { - // prepare Bridged chain storage with inbound lane state + // Prepare bridged chain storage with inbound lane state. let storage_key = storage_keys::inbound_lane_data_key(B::BRIDGED_MESSAGES_PALLET_NAME, &lane).0; let mut root = Default::default(); let mut mdb = MemoryDB::default(); @@ -129,7 +129,7 @@ where .expect("TrieMut::insert should not fail in benchmarks"); } - // generate storage proof to be delivered to This chain + // Generate storage proof to be delivered to this chain. let storage_proof = record_all_trie_keys::>>, _>(&mdb, &root) .map_err(|_| "record_all_trie_keys has failed") .expect("record_all_trie_keys should not fail in benchmarks"); From 4296581dfcbbe4ea96fec18ad4de328a9e4086bd Mon Sep 17 00:00:00 2001 From: Daniel Chmielewski Date: Fri, 5 Apr 2024 21:40:27 +0100 Subject: [PATCH 07/12] Improved comments in bridges/bin/runtime-common/src/messages_xcm_extension.rs. --- .../src/messages_xcm_extension.rs | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/bridges/bin/runtime-common/src/messages_xcm_extension.rs b/bridges/bin/runtime-common/src/messages_xcm_extension.rs index 46ed4da0d8548..57288edaac808 100644 --- a/bridges/bin/runtime-common/src/messages_xcm_extension.rs +++ b/bridges/bin/runtime-common/src/messages_xcm_extension.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . -//! Module provides utilities for easier XCM handling, e.g: +//! Module provides utilities for easier XCM handling, e.g.: //! `XcmExecutor` -> `MessageSender` -> `OutboundMessageQueue` //! | //! `Relayer` @@ -40,18 +40,18 @@ use sp_std::{fmt::Debug, marker::PhantomData}; use xcm::prelude::*; use xcm_builder::{DispatchBlob, DispatchBlobError}; -/// Message dispatch result type for single message. +/// Message dispatch result type for a single message. #[derive(CloneNoBound, EqNoBound, PartialEqNoBound, Encode, Decode, Debug, TypeInfo)] pub enum XcmBlobMessageDispatchResult { - /// We've been unable to decode message payload. + /// We've been unable to decode the message payload. InvalidPayload, /// Message has been dispatched. Dispatched, - /// Message has **NOT** been dispatched because of given error. + /// Message has **NOT** been dispatched because of a given error. NotDispatched(#[codec(skip)] Option), } -/// [`XcmBlobMessageDispatch`] is responsible for dispatching received messages +/// [`XcmBlobMessageDispatch`] is responsible for dispatching received messages. /// /// It needs to be used at the target bridge hub. pub struct XcmBlobMessageDispatch { @@ -132,7 +132,7 @@ pub struct SenderAndLane { } impl SenderAndLane { - /// Create new object using provided location and lane. + /// Create a new object using the provided location and lane. pub fn new(location: Location, lane: LaneId) -> Self { SenderAndLane { location, lane } } @@ -178,7 +178,7 @@ impl< if let Some(sender_and_lane) = Lanes::get().iter().find(|link| link.0.lane == lane).map(|link| &link.0) { - // notify XCM queue manager about updated lane state + // Notify XCM queue manager about updated lane state. LocalXcmQueueManager::::on_bridge_messages_delivered( sender_and_lane, enqueued_messages, @@ -197,8 +197,8 @@ pub struct LocalXcmQueueManager(PhantomData); /// send a "congestion" XCM message to the sending chain. const OUTBOUND_LANE_CONGESTED_THRESHOLD: MessageNonce = 8_192; -/// After we have sent "congestion" XCM message to the sending chain, we wait until number -/// of messages in the outbound bridge queue drops to this count, before sending `uncongestion` +/// After we have sent "congestion" XCM message to the sending chain, we wait until the number +/// of messages in the outbound bridge queue drops to this count before sending `uncongestion` /// XCM message. const OUTBOUND_LANE_UNCONGESTED_THRESHOLD: MessageNonce = 1_024; @@ -208,17 +208,17 @@ impl LocalXcmQueueManager { sender_and_lane: &SenderAndLane, enqueued_messages: MessageNonce, ) { - // skip if we dont want to handle congestion + // Skip if we don't want to handle congestion. if !H::supports_congestion_detection() { return } - // if we have already sent the congestion signal, we don't want to do anything + // If we have already sent the congestion signal, we don't want to do anything. if Self::is_congested_signal_sent(sender_and_lane.lane) { return } - // if the bridge queue is not congested, we don't want to do anything + // If the bridge queue is not congested, we don't want to do anything. let is_congested = enqueued_messages > OUTBOUND_LANE_CONGESTED_THRESHOLD; if !is_congested { return @@ -248,17 +248,17 @@ impl LocalXcmQueueManager { sender_and_lane: &SenderAndLane, enqueued_messages: MessageNonce, ) { - // skip if we don't want to handle congestion + // Skip if we don't want to handle congestion. if !H::supports_congestion_detection() { return } - // if we have not sent the congestion signal before, we don't want to do anything + // If we have not sent the congestion signal before, we don't want to do anything. if !Self::is_congested_signal_sent(sender_and_lane.lane) { return } - // if the bridge queue is still congested, we don't want to do anything + // If the bridge queue is still congested, we don't want to do anything. let is_congested = enqueued_messages > OUTBOUND_LANE_UNCONGESTED_THRESHOLD; if is_congested { return @@ -410,14 +410,14 @@ mod tests { run_test(|| { let enqueued = fill_up_lane_to_congestion(); - // next sent message leads to congested signal + // The next sent message leads to a congested signal. LocalXcmQueueManager::::on_bridge_message_enqueued( &TestSenderAndLane::get(), enqueued + 1, ); assert_eq!(DummySendXcm::messages_sent(), 1); - // next sent message => we don't sent another congested signal + // The next sent message means we don't send another congested signal. LocalXcmQueueManager::::on_bridge_message_enqueued( &TestSenderAndLane::get(), enqueued, @@ -442,7 +442,7 @@ mod tests { run_test(|| { let enqueued = fill_up_lane_to_congestion(); - // next sent message leads to congested signal + // The next sent message leads to a congested signal. LocalXcmQueueManager::::on_bridge_message_enqueued( &TestSenderAndLane::get(), enqueued + 1, @@ -458,7 +458,7 @@ mod tests { LocalXcmQueueManager::::send_congested_signal(&TestSenderAndLane::get()).unwrap(); assert_eq!(DummySendXcm::messages_sent(), 1); - // when we receive a delivery report for other lane, we don't send an uncongested signal + // When we receive a delivery report for other lane, we don't send an uncongested signal. TestBlobHaulerAdapter::on_messages_delivered(LaneId([42, 42, 42, 42]), 0); assert_eq!(DummySendXcm::messages_sent(), 1); }); From 3e4b5224c07b0fa2524a1d2216eaae72618695fc Mon Sep 17 00:00:00 2001 From: Daniel Chmielewski Date: Fri, 5 Apr 2024 21:46:23 +0100 Subject: [PATCH 08/12] Improved comments in bridges/bin/runtime-common/src/messages.rs. --- bridges/bin/runtime-common/src/messages.rs | 114 ++++++++++----------- 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/bridges/bin/runtime-common/src/messages.rs b/bridges/bin/runtime-common/src/messages.rs index 4aca53f3b9836..3488e87eee4c0 100644 --- a/bridges/bin/runtime-common/src/messages.rs +++ b/bridges/bin/runtime-common/src/messages.rs @@ -14,11 +14,11 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . -//! Types that allow runtime to act as a source/target endpoint of message lanes. +//! Types that enable the runtime to act as a source/target endpoint of message lanes. //! -//! Messages are assumed to be encoded `Call`s of the target chain. Call-dispatch -//! pallet is used to dispatch incoming messages. Message identified by a tuple -//! of to elements - message lane id and message nonce. +//! Messages are assumed to be encoded `Call`s of the target chain. The call-dispatch +//! pallet is used to dispatch incoming messages. Messages are identified by a tuple +//! of two elements - message lane ID and message nonce. pub use bp_runtime::{RangeInclusiveExt, UnderlyingChainOf, UnderlyingChainProvider}; @@ -39,49 +39,49 @@ use sp_std::{convert::TryFrom, marker::PhantomData, vec::Vec}; /// Bidirectional message bridge. pub trait MessageBridge { - /// Name of the paired messages pallet instance at the Bridged chain. + /// Name of the paired messages pallet instance at the bridged chain. /// - /// Should be the name that is used in the `construct_runtime!()` macro. + /// Should be the name used in the `construct_runtime!()` macro. const BRIDGED_MESSAGES_PALLET_NAME: &'static str; - /// This chain in context of message bridge. + /// This chain in the context of the message bridge. type ThisChain: ThisChainWithMessages; - /// Bridged chain in context of message bridge. + /// Bridged chain in the context of the message bridge. type BridgedChain: BridgedChainWithMessages; /// Bridged header chain. type BridgedHeaderChain: HeaderChain>; } -/// This chain that has `pallet-bridge-messages` module. +/// This chain that has the `pallet-bridge-messages` module. pub trait ThisChainWithMessages: UnderlyingChainProvider { /// Call origin on the chain. type RuntimeOrigin; } -/// Bridged chain that has `pallet-bridge-messages` module. +/// Bridged chain that has the `pallet-bridge-messages` module. pub trait BridgedChainWithMessages: UnderlyingChainProvider {} -/// This chain in context of message bridge. +/// This chain in the context of the message bridge. pub type ThisChain = ::ThisChain; -/// Bridged chain in context of message bridge. +/// Bridged chain in the context of the message bridge. pub type BridgedChain = ::BridgedChain; /// Hash used on the chain. pub type HashOf = bp_runtime::HashOf<::Chain>; /// Hasher used on the chain. pub type HasherOf = bp_runtime::HasherOf>; -/// Account id used on the chain. +/// Account ID used on the chain. pub type AccountIdOf = bp_runtime::AccountIdOf>; -/// Type of balances that is used on the chain. +/// Type of balances used on the chain. pub type BalanceOf = bp_runtime::BalanceOf>; -/// Sub-module that is declaring types required for processing This -> Bridged chain messages. +/// Sub-module declaring types required for processing This -> Bridged chain messages. pub mod source { use super::*; /// Message payload for This -> Bridged chain messages. pub type FromThisChainMessagePayload = crate::messages_xcm_extension::XcmAsPlainPayload; - /// Maximal size of outbound message payload. + /// Maximum size of outbound message payload. pub struct FromThisChainMaximalOutboundPayloadSize(PhantomData); impl Get for FromThisChainMaximalOutboundPayloadSize { @@ -92,16 +92,16 @@ pub mod source { /// Messages delivery proof from bridged chain: /// - /// - hash of finalized header; - /// - storage proof of inbound lane state; - /// - lane id. + /// - Hash of finalized header. + /// - Storage proof of inbound lane state. + /// - Lane ID. #[derive(Clone, Decode, Encode, Eq, PartialEq, RuntimeDebug, TypeInfo)] pub struct FromBridgedChainMessagesDeliveryProof { /// Hash of the bridge header the proof is for. pub bridged_header_hash: BridgedHeaderHash, /// Storage trie proof generated for [`Self::bridged_header_hash`]. pub storage_proof: RawStorageProof, - /// Lane id of which messages were delivered and the proof is for. + /// Lane ID of which messages were delivered and the proof is for. pub lane: LaneId, } @@ -116,18 +116,18 @@ pub mod source { } } - /// 'Parsed' message delivery proof - inbound lane id and its state. + /// 'Parsed' message delivery proof - inbound lane ID and its state. pub type ParsedMessagesDeliveryProofFromBridgedChain = (LaneId, InboundLaneData>>); - /// Return maximal message size of This -> Bridged chain message. + /// Return maximum message size of This -> Bridged chain message. pub fn maximal_message_size() -> u32 { super::target::maximal_incoming_message_size( UnderlyingChainOf::>::max_extrinsic_size(), ) } - /// `TargetHeaderChain` implementation that is using default types and perform default checks. + /// `TargetHeaderChain` implementation using default types and performs default checks. pub struct TargetHeaderChainAdapter(PhantomData); impl TargetHeaderChain>> @@ -146,33 +146,33 @@ pub mod source { } } - /// Do basic Bridged-chain specific verification of This -> Bridged chain message. + /// Perform basic Bridged-chain specific verification of This -> Bridged chain message. /// - /// Ok result from this function means that the delivery transaction with this message + /// An Ok result from this function means that the delivery transaction with this message /// may be 'mined' by the target chain. pub fn verify_chain_message( payload: &FromThisChainMessagePayload, ) -> Result<(), VerificationError> { - // IMPORTANT: any error that is returned here is fatal for the bridge, because - // this code is executed at the bridge hub and message sender actually lives - // at some sibling parachain. So we are failing **after** the message has been - // sent and we can't report it back to sender (unless error report mechanism is + // IMPORTANT: Any error returned here is fatal for the bridge, because + // this code is executed at the bridge hub, and the message sender actually lives + // at some sibling parachain. So, we are failing **after** the message has been + // sent, and we can't report it back to the sender (unless error report mechanism is // embedded into message and its dispatcher). - // apart from maximal message size check (see below), we should also check the message - // dispatch weight here. But we assume that the bridged chain will just push the message + // Apart from maximum message size check (see below), we should also check the message + // dispatch weight here. However, we assume that the bridged chain will just push the message // to some queue (XCMP, UMP, DMP), so the weight is constant and fits the block. - // The maximal size of extrinsic at Substrate-based chain depends on the + // The maximum size of an extrinsic at Substrate-based chains depends on the // `frame_system::Config::MaximumBlockLength` and - // `frame_system::Config::AvailableBlockRatio` constants. This check is here to be sure that - // the lane won't stuck because message is too large to fit into delivery transaction. + // `frame_system::Config::AvailableBlockRatio` constants. This check ensures that + // the lane won't get stuck because the message is too large to fit into the delivery transaction. // - // **IMPORTANT NOTE**: the delivery transaction contains storage proof of the message, not - // the message itself. The proof is always larger than the message. But unless chain state + // **IMPORTANT NOTE**: The delivery transaction contains the storage proof of the message, not + // the message itself. The proof is always larger than the message. But unless the chain state // is enormously large, it should be several dozens/hundreds of bytes. The delivery // transaction also contains signatures and signed extensions. Because of this, we reserve - // 1/3 of the the maximal extrinsic size for this data. + // 1/3 of the maximum extrinsic size for this data. if payload.len() > maximal_message_size::() as usize { return Err(VerificationError::MessageTooLarge) } @@ -180,9 +180,9 @@ pub mod source { Ok(()) } - /// Verify proof of This -> Bridged chain messages delivery. + /// Verify proof of This -> Bridged chain message delivery. /// - /// This function is used when Bridged chain is directly using GRANDPA finality. For Bridged + /// This function is used when the Bridged chain is directly using GRANDPA finality. For Bridged /// parachains, please use the `verify_messages_delivery_proof_from_parachain`. pub fn verify_messages_delivery_proof( proof: FromBridgedChainMessagesDeliveryProof>>, @@ -192,7 +192,7 @@ pub mod source { let mut storage = B::BridgedHeaderChain::storage_proof_checker(bridged_header_hash, storage_proof) .map_err(VerificationError::HeaderChain)?; - // Messages delivery proof is just proof of single storage key read => any error + // Message delivery proof is just proof of a single storage key read => any error // is fatal. let storage_inbound_lane_data_key = bp_messages::storage_keys::inbound_lane_data_key( B::BRIDGED_MESSAGES_PALLET_NAME, @@ -202,14 +202,14 @@ pub mod source { .read_and_decode_mandatory_value(storage_inbound_lane_data_key.0.as_ref()) .map_err(VerificationError::InboundLaneStorage)?; - // check that the storage proof doesn't have any untouched trie nodes + // Check that the storage proof doesn't have any untouched trie nodes. storage.ensure_no_unused_nodes().map_err(VerificationError::StorageProof)?; Ok((lane, inbound_lane_data)) } } -/// Sub-module that is declaring types required for processing Bridged -> This chain messages. +/// Sub-module declaring types required for processing Bridged -> This chain messages. pub mod target { use super::*; @@ -218,10 +218,10 @@ pub mod target { /// Messages proof from bridged chain: /// - /// - hash of finalized header; - /// - storage proof of messages and (optionally) outbound lane state; - /// - lane id; - /// - nonces (inclusive range) of messages which are included in this proof. + /// - Hash of finalized header. + /// - Storage proof of messages and (optionally) outbound lane state. + /// - Lane ID. + /// - Nonces (inclusive range) of messages included in this proof. #[derive(Clone, Decode, Encode, Eq, PartialEq, RuntimeDebug, TypeInfo)] pub struct FromBridgedChainMessagesProof { /// Hash of the finalized bridged header the proof is for. @@ -247,17 +247,17 @@ pub mod target { } } - /// Return maximal dispatch weight of the message we're able to receive. + /// Return maximum dispatch weight of the message we're able to receive. pub fn maximal_incoming_message_dispatch_weight(maximal_extrinsic_weight: Weight) -> Weight { maximal_extrinsic_weight / 2 } - /// Return maximal message size given maximal extrinsic size. + /// Return maximum message size given maximum extrinsic size. pub fn maximal_incoming_message_size(maximal_extrinsic_size: u32) -> u32 { maximal_extrinsic_size / 3 * 2 } - /// `SourceHeaderChain` implementation that is using default types and perform default checks. + /// `SourceHeaderChain` implementation using default types and performs default checks. pub struct SourceHeaderChainAdapter(PhantomData); impl SourceHeaderChain for SourceHeaderChainAdapter { @@ -296,16 +296,16 @@ pub mod target { let mut parser = StorageProofCheckerAdapter::<_, B> { storage, _dummy: Default::default() }; let nonces_range = nonces_start..=nonces_end; - // receiving proofs where end < begin is ok (if proof includes outbound lane state) + // Receiving proofs where end < begin is OK (if proof includes outbound lane state). let messages_in_the_proof = nonces_range.checked_len().unwrap_or(0); if messages_in_the_proof != MessageNonce::from(messages_count) { return Err(VerificationError::MessagesCountMismatch) } - // Read messages first. All messages that are claimed to be in the proof must - // be in the proof. So any error in `read_value`, or even missing value is fatal. + // Read messages first. All messages claimed to be in the proof must + // be present. So, any error in `read_value`, or even a missing value is fatal. // - // Mind that we allow proofs with no messages if outbound lane state is proved. + // Note that we allow proofs with no messages if outbound lane state is proved. let mut messages = Vec::with_capacity(messages_in_the_proof as _); for nonce in nonces_range { let message_key = MessageKey { lane_id: lane, nonce }; @@ -313,25 +313,25 @@ pub mod target { messages.push(Message { key: message_key, payload: message_payload }); } - // Now let's check if proof contains outbound lane state proof. It is optional, so + // Now, let's check if the proof contains the outbound lane state proof. It is optional, so // we simply ignore `read_value` errors and missing value. let proved_lane_messages = ProvedLaneMessages { lane_state: parser.read_and_decode_outbound_lane_data(&lane)?, messages, }; - // Now we may actually check if the proof is empty or not. + // Now, we may actually check if the proof is empty or not. if proved_lane_messages.lane_state.is_none() && proved_lane_messages.messages.is_empty() { return Err(VerificationError::EmptyMessageProof) } - // check that the storage proof doesn't have any untouched trie nodes + // Check that the storage proof doesn't have any untouched trie nodes. parser .storage .ensure_no_unused_nodes() .map_err(VerificationError::StorageProof)?; - // We only support single lane messages in this generated_schema + // We only support single-lane messages in this generated schema. let mut proved_messages = ProvedMessages::new(); proved_messages.insert(lane, proved_lane_messages); From 867cb922493095fd52ffeae5c73332b227f3ebae Mon Sep 17 00:00:00 2001 From: Daniel Chmielewski Date: Fri, 5 Apr 2024 21:54:51 +0100 Subject: [PATCH 09/12] Improved comments in bridges/bin/runtime-common/src/mock.rs. --- bridges/bin/runtime-common/src/mock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridges/bin/runtime-common/src/mock.rs b/bridges/bin/runtime-common/src/mock.rs index ad71cd0d456d8..2a275ede6e8f3 100644 --- a/bridges/bin/runtime-common/src/mock.rs +++ b/bridges/bin/runtime-common/src/mock.rs @@ -98,7 +98,7 @@ pub type TestStakeAndSlash = pallet_bridge_relayers::StakeAndSlashNamed< /// Message lane used in tests. pub const TEST_LANE_ID: LaneId = LaneId([0, 0, 0, 0]); -/// Bridged chain id used in tests. +/// Bridged chain ID used in tests. pub const TEST_BRIDGED_CHAIN_ID: ChainId = *b"brdg"; /// Maximal extrinsic weight at the `BridgedChain`. pub const BRIDGED_CHAIN_MAX_EXTRINSIC_WEIGHT: usize = 2048; From 24c16b310253fc000316bb0d1d7bfdfacfe3ad7d Mon Sep 17 00:00:00 2001 From: Daniel Chmielewski Date: Fri, 5 Apr 2024 21:56:18 +0100 Subject: [PATCH 10/12] Improved comments in bridges/bin/runtime-common/src/parachains_benchmarking.rs. --- .../runtime-common/src/parachains_benchmarking.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bridges/bin/runtime-common/src/parachains_benchmarking.rs b/bridges/bin/runtime-common/src/parachains_benchmarking.rs index b3050b9ac0f3c..05bdea2292326 100644 --- a/bridges/bin/runtime-common/src/parachains_benchmarking.rs +++ b/bridges/bin/runtime-common/src/parachains_benchmarking.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . -//! Everything required to run benchmarks of parachains finality module. +//! Everything required to run benchmarks of the parachains finality module. #![cfg(feature = "runtime-benchmarks")] @@ -34,8 +34,8 @@ use sp_trie::{trie_types::TrieDBMutBuilderV1, LayoutV1, MemoryDB, TrieMut}; /// Prepare proof of messages for the `receive_messages_proof` call. /// -/// In addition to returning valid messages proof, environment is prepared to verify this message -/// proof. +/// In addition to returning a valid messages proof, the environment is prepared to verify this +/// message proof. pub fn prepare_parachain_heads_proof( parachains: &[ParaId], parachain_head_size: u32, @@ -50,7 +50,7 @@ where { let parachain_head = ParaHead(vec![0u8; parachain_head_size as usize]); - // insert all heads to the trie + // Insert all heads into 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(); @@ -59,7 +59,7 @@ where let mut trie = TrieDBMutBuilderV1::::new(&mut mdb, &mut state_root).build(); - // insert parachain heads + // Insert parachain heads. for (i, parachain) in parachains.into_iter().enumerate() { let storage_key = parachain_head_storage_key_at_source(R::ParasPalletName::get(), *parachain); @@ -76,7 +76,7 @@ where } } - // generate heads storage proof + // Generate heads storage proof. let proof = record_all_trie_keys::, _>(&mdb, &state_root) .map_err(|_| "record_all_trie_keys has failed") .expect("record_all_trie_keys should not fail in benchmarks"); From 59349c027064a6a98a98470036725719370769a3 Mon Sep 17 00:00:00 2001 From: Daniel Chmielewski Date: Fri, 5 Apr 2024 21:59:31 +0100 Subject: [PATCH 11/12] Improved comments in bridges/bin/runtime-common/src/priority_calculator.rs. --- .../runtime-common/src/priority_calculator.rs | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/bridges/bin/runtime-common/src/priority_calculator.rs b/bridges/bin/runtime-common/src/priority_calculator.rs index 5035553f508df..786d281d7fca3 100644 --- a/bridges/bin/runtime-common/src/priority_calculator.rs +++ b/bridges/bin/runtime-common/src/priority_calculator.rs @@ -16,29 +16,29 @@ //! Bridge transaction priority calculator. //! -//! We want to prioritize message delivery transactions with more messages over -//! transactions with less messages. That's because we reject delivery transactions -//! if it contains already delivered message. And if some transaction delivers -//! single message with nonce `N`, then the transaction with nonces `N..=N+100` will -//! be rejected. This can lower bridge throughput down to one message per block. +//! We aim to prioritize message delivery transactions with more messages over +//! those with fewer messages. That's because we reject delivery transactions +//! containing already delivered messages. If a transaction delivers +//! a single message with nonce `N`, then the transaction with nonces `N..=N+100` will +//! be rejected. This can reduce bridge throughput to one message per block. use bp_messages::MessageNonce; use frame_support::traits::Get; use sp_runtime::transaction_validity::TransactionPriority; -// reexport everything from `integrity_tests` module +// Reexport everything from the `integrity_tests` module. #[allow(unused_imports)] pub use integrity_tests::*; -/// Compute priority boost for message delivery transaction that delivers -/// given number of messages. +/// Compute the priority boost for a message delivery transaction that delivers +/// a given number of messages. pub fn compute_priority_boost( messages: MessageNonce, ) -> TransactionPriority where PriorityBoostPerMessage: Get, { - // we don't want any boost for transaction with single message => minus one + // We don't want any boost for a transaction with a single message => minus one. PriorityBoostPerMessage::get().saturating_mul(messages.saturating_sub(1)) } @@ -71,8 +71,8 @@ mod integrity_tests { /// Ensures that the value of `PriorityBoostPerMessage` matches the value of /// `tip_boost_per_message`. /// - /// We want two transactions, `TX1` with `N` messages and `TX2` with `N+1` messages, have almost - /// the same priority if we'll add `tip_boost_per_message` tip to the `TX1`. We want to be sure + /// We want two transactions, `TX1` with `N` messages and `TX2` with `N+1` messages, to have almost + /// the same priority if we add `tip_boost_per_message` tip to `TX1`. We aim to ensure /// that if we add plain `PriorityBoostPerMessage` priority to `TX1`, the priority will be close /// to `TX2` as well. pub fn ensure_priority_boost_is_sane( @@ -100,7 +100,7 @@ mod integrity_tests { let priority_with_tip = estimate_message_delivery_transaction_priority::(1, tip); - const ERROR_MARGIN: TransactionPriority = 5; // 5% + const ERROR_MARGIN: TransactionPriority = 5; // 5%. if priority_with_boost.abs_diff(priority_with_tip).saturating_mul(100) / priority_with_tip > ERROR_MARGIN @@ -116,7 +116,7 @@ mod integrity_tests { } } - /// Compute priority boost that we give to message delivery transaction for additional message. + /// Compute the priority boost that we give to a message delivery transaction for an additional message. #[cfg(feature = "integrity-test")] fn compute_priority_boost_per_message( tip_boost_per_message: BalanceOf, @@ -128,7 +128,7 @@ mod integrity_tests { Runtime::RuntimeCall: Dispatchable, BalanceOf: Send + Sync + FixedPointOperand, { - // estimate priority of transaction that delivers one message and has large tip + // Estimate the priority of a transaction that delivers one message and has a large tip. let maximal_messages_in_delivery_transaction = Runtime::MaxUnconfirmedMessagesAtInboundLane::get(); let small_with_tip_priority = @@ -137,7 +137,7 @@ mod integrity_tests { tip_boost_per_message .saturating_mul(maximal_messages_in_delivery_transaction.saturated_into()), ); - // estimate priority of transaction that delivers maximal number of messages, but has no tip + // Estimate the priority of a transaction that delivers the maximum number of messages, but has no tip. let large_without_tip_priority = estimate_message_delivery_transaction_priority::< Runtime, MessagesInstance, @@ -148,7 +148,7 @@ mod integrity_tests { .saturating_div(maximal_messages_in_delivery_transaction - 1) } - /// Estimate message delivery transaction priority. + /// Estimate the priority of a message delivery transaction. #[cfg(feature = "integrity-test")] fn estimate_message_delivery_transaction_priority( messages: MessageNonce, @@ -161,26 +161,26 @@ mod integrity_tests { Runtime::RuntimeCall: Dispatchable, BalanceOf: Send + Sync + FixedPointOperand, { - // just an estimation of extra transaction bytes that are added to every transaction - // (including signature, signed extensions extra and etc + in our case it includes - // all call arguments except the proof itself) + // An estimation of extra transaction bytes added to every transaction + // (including signature, signed extensions, and extras + in our case, it includes + // all call arguments except the proof itself). let base_tx_size = 512; - // let's say we are relaying similar small messages and for every message we add more trie - // nodes to the proof (x0.5 because we expect some nodes to be reused) + // Let's say we are relaying similar small messages and for every message, we add more trie + // nodes to the proof (x0.5 because we expect some nodes to be reused). let estimated_message_size = 512; - // let's say all our messages have the same dispatch weight + // Let's say all our messages have the same dispatch weight. let estimated_message_dispatch_weight = Runtime::WeightInfo::message_dispatch_weight(estimated_message_size); - // messages proof argument size is (for every message) messages size + some additional - // trie nodes. Some of them are reused by different messages, so let's take 2/3 of default - // "overhead" constant + // Message proof argument size is, for every message, message size + some additional + // trie nodes. Some of them are reused by different messages, so let's take 2/3 of the default + // "overhead" constant. let messages_proof_size = Runtime::WeightInfo::expected_extra_storage_proof_size() .saturating_mul(2) .saturating_div(3) .saturating_add(estimated_message_size) .saturating_mul(messages as _); - // finally we are able to estimate transaction size and weight + // Finally, we can estimate transaction size and weight. let transaction_size = base_tx_size.saturating_add(messages_proof_size); let transaction_weight = Runtime::WeightInfo::receive_messages_proof_weight( &PreComputedSize(transaction_size as _), From 1f4a2ebcafdc3a6d3ceb04921eb6e79fa71cb7aa Mon Sep 17 00:00:00 2001 From: Daniel Chmielewski Date: Fri, 5 Apr 2024 22:08:58 +0100 Subject: [PATCH 12/12] Improved comments in bridges/bin/runtime-common/src/refund_relayer_extension.rs. --- .../src/refund_relayer_extension.rs | 230 +++++++++--------- 1 file changed, 115 insertions(+), 115 deletions(-) diff --git a/bridges/bin/runtime-common/src/refund_relayer_extension.rs b/bridges/bin/runtime-common/src/refund_relayer_extension.rs index 455392a0a277f..de79ca69e5be6 100644 --- a/bridges/bin/runtime-common/src/refund_relayer_extension.rs +++ b/bridges/bin/runtime-common/src/refund_relayer_extension.rs @@ -14,10 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . -//! Signed extension that refunds relayer if he has delivered some new messages. -//! It also refunds transaction cost if the transaction is an `utility.batchAll()` -//! with calls that are: delivering new message and all necessary underlying headers -//! (parachain or relay chain). +//! Signed extension that refunds the relayer if they have delivered new messages. +//! It also refunds the transaction cost if the transaction is an `utility.batchAll()`. +//! with calls that are delivering new messages and all necessary underlying headers +//! (either parachain or relay chain). use crate::messages_call_ext::{ CallHelper as MessagesCallHelper, CallInfo as MessagesCallInfo, MessagesCallSubType, @@ -57,7 +57,7 @@ use sp_runtime::{ use sp_std::{marker::PhantomData, vec, vec::Vec}; type AccountIdOf = ::AccountId; -// without this typedef rustfmt fails with internal err +// Without this typedef, rustfmt fails with an internal error. type BalanceOf = <::OnChargeTransaction as OnChargeTransaction>::Balance; type CallOf = ::RuntimeCall; @@ -67,7 +67,7 @@ type CallOf = ::RuntimeCall; pub trait RefundableParachainId { /// The instance of the bridge parachains pallet. type Instance; - /// The parachain Id. + /// The parachain ID. type Id: Get; } @@ -98,7 +98,7 @@ where pub trait RefundableMessagesLaneId { /// The instance of the bridge messages pallet. type Instance: 'static; - /// The messages lane id. + /// The messages lane ID. type Id: Get; } @@ -119,7 +119,7 @@ pub trait RefundCalculator { /// The underlying integer type in which the refund is calculated. type Balance; - /// Compute refund for given transaction. + /// Compute refund for a given transaction. fn compute_refund( info: &DispatchInfo, post_info: &PostDispatchInfo, @@ -128,7 +128,7 @@ pub trait RefundCalculator { ) -> Self::Balance; } -/// `RefundCalculator` implementation which refunds the actual transaction fee. +/// `RefundCalculator` implementation that refunds the actual transaction fee. pub struct ActualFeeRefund(PhantomData); impl RefundCalculator for ActualFeeRefund @@ -149,7 +149,7 @@ where } } -/// Data that is crafted in `pre_dispatch` method and used at `post_dispatch`. +/// Data crafted in the `pre_dispatch` method and used at `post_dispatch`. #[cfg_attr(test, derive(Debug, PartialEq))] pub struct PreDispatchData { /// Transaction submitter (relayer) account. @@ -171,14 +171,14 @@ pub enum CallInfo { RelayFinalityAndMsgs(SubmitFinalityProofInfo, MessagesCallInfo), /// Parachain finality + message delivery/confirmation calls. /// - /// This variant is used only when bridging with parachain. + /// This variant is used only when bridging with a parachain. ParachainFinalityAndMsgs(SubmitParachainHeadsInfo, MessagesCallInfo), /// Standalone message delivery/confirmation call. Msgs(MessagesCallInfo), } impl CallInfo { - /// Returns true if call is a message delivery call (with optional finality calls). + /// Returns true if the call is a message delivery call (with optional finality calls). fn is_receive_messages_proof_call(&self) -> bool { match self.messages_call_info() { MessagesCallInfo::ReceiveMessagesProof(_) => true, @@ -195,7 +195,7 @@ impl CallInfo { } } - /// Returns mutable reference to pre-dispatch `finality_target` sent to the + /// Returns a mutable reference to the pre-dispatch `finality_target` sent to the /// `SubmitFinalityProof` call. #[cfg(test)] fn submit_finality_proof_info_mut( @@ -228,10 +228,10 @@ impl CallInfo { } } -/// The actions on relayer account that need to be performed because of his actions. +/// The actions on the relayer account that need to be performed because of their actions. #[derive(RuntimeDebug, PartialEq)] pub enum RelayerAccountAction { - /// Do nothing with relayer account. + /// Do nothing with the relayer account. None, /// Reward the relayer. Reward(AccountId, RewardsAccountParams, Reward), @@ -265,26 +265,26 @@ where /// Unpack batch runtime call. fn expand_call(call: &CallOf) -> Vec<&CallOf>; - /// Given runtime call, check if it has supported format. Additionally, check if any of - /// (optionally batched) calls are obsolete and we shall reject the transaction. + /// Given a runtime call, check if it has a supported format. Additionally, check if any of + /// the (optionally batched) calls are obsolete and we shall reject the transaction. fn parse_and_check_for_obsolete_call( call: &CallOf, ) -> Result, TransactionValidityError>; - /// Check if parsed call is already obsolete. + /// Check if the parsed call is already obsolete. fn check_obsolete_parsed_call( call: &CallOf, ) -> Result<&CallOf, TransactionValidityError>; /// Called from post-dispatch and shall perform additional checks (apart from relay - /// chain finality and messages transaction finality) of given call result. + /// chain finality and message transaction finality) of the given call result. fn additional_call_result_check( relayer: &AccountIdOf, call_info: &CallInfo, ) -> bool; - /// Given post-dispatch information, analyze the outcome of relayer call and return - /// actions that need to be performed on relayer account. + /// Given post-dispatch information, analyze the outcome of the relayer call and return + /// actions that need to be performed on the relayer account. fn analyze_call_result( pre: Option>>>, info: &DispatchInfo, @@ -302,8 +302,8 @@ where _ => return RelayerAccountAction::None, }; - // now we know that the relayer either needs to be rewarded, or slashed - // => let's prepare the correspondent account that pays reward/receives slashed amount + // Now we know that the relayer either needs to be rewarded or slashed. + // => Let's prepare the correspondent account that pays reward/receives slashed amount. let reward_account_params = RewardsAccountParams::new( ::Id::get(), @@ -317,17 +317,17 @@ where }, ); - // prepare return value for the case if the call has failed or it has not caused - // expected side effects (e.g. not all messages have been accepted) + // Prepare return value for the case if the call has failed or it has not caused + // expected side effects (e.g. not all messages have been accepted). // - // we are not checking if relayer is registered here - it happens during the slash attempt + // We are not checking if the relayer is registered here - it happens during the slash attempt. // - // there are couple of edge cases here: + // There are a couple of edge cases here: // - // - when the relayer becomes registered during message dispatch: this is unlikely + relayer - // should be ready for slashing after registration; + // - When the relayer becomes registered during message dispatch: this is unlikely + relayer + // should be ready for slashing after registration. // - // - when relayer is registered after `validate` is called and priority is not boosted: + // - When relayer is registered after `validate` is called and priority is not boosted: // relayer should be ready for slashing after registration. let may_slash_relayer = Self::bundled_messages_for_priority_boost(Some(&call_info)).is_some(); @@ -348,12 +348,12 @@ where return slash_relayer_if_delivery_result } - // check if relay chain state has been updated + // Check if the relay chain state has been updated. if let Some(finality_proof_info) = call_info.submit_finality_proof_info() { if !SubmitFinalityProofHelper::::was_successful( finality_proof_info.block_number, ) { - // we only refund relayer if all calls have updated chain state + // We only refund the relayer if all calls have updated chain state. log::trace!( target: "runtime::bridge", "{} via {:?}: relayer {:?} has submitted invalid relay chain finality proof", @@ -364,15 +364,15 @@ where return slash_relayer_if_delivery_result } - // there's a conflict between how bridge GRANDPA pallet works and a `utility.batchAll` + // There's a conflict between how the bridge GRANDPA pallet works and a `utility.batchAll` // transaction. If relay chain header is mandatory, the GRANDPA pallet returns // `Pays::No`, because such transaction is mandatory for operating the bridge. But - // `utility.batchAll` transaction always requires payment. But in both cases we'll - // refund relayer - either explicitly here, or using `Pays::No` if he's choosing - // to submit dedicated transaction. + // `utility.batchAll` transaction always requires payment. In both cases, we'll + // refund the relayer - either explicitly here, or using `Pays::No` if they're choosing + // to submit a dedicated transaction. - // submitter has means to include extra weight/bytes in the `submit_finality_proof` - // call, so let's subtract extra weight/size to avoid refunding for this extra stuff + // Submitter has means to include extra weight/bytes in the `submit_finality_proof` + // call, so let's subtract extra weight/size to avoid refunding for this extra stuff. extra_weight = finality_proof_info.extra_weight; extra_size = finality_proof_info.extra_size; } @@ -391,59 +391,59 @@ where return slash_relayer_if_delivery_result } - // do additional check + // Do additional check. if !Self::additional_call_result_check(&relayer, &call_info) { return slash_relayer_if_delivery_result } - // regarding the tip - refund that happens here (at this side of the bridge) isn't the whole - // relayer compensation. He'll receive some amount at the other side of the bridge. It shall - // (in theory) cover the tip there. Otherwise, if we'll be compensating tip here, some - // malicious relayer may use huge tips, effectively depleting account that pay rewards. The - // cost of this attack is nothing. Hence we use zero as tip here. + // Regarding the tip - the refund that happens here (on this side of the bridge) isn't the whole + // relayer compensation. They'll receive some amount on the other side of the bridge, which shall + // (in theory) cover the tip there. Otherwise, if we were compensating the tip here, some + // malicious relayer may use huge tips, effectively depleting the account that pays rewards. The + // cost of this attack is nothing. Hence, we use zero as the tip here. let tip = Zero::zero(); - // decrease post-dispatch weight/size using extra weight/size that we know now + // Decrease post-dispatch weight/size using extra weight/size that we know now. let post_info_len = len.saturating_sub(extra_size as usize); let mut post_info_weight = post_info.actual_weight.unwrap_or(info.weight).saturating_sub(extra_weight); - // let's also replace the weight of slashing relayer with the weight of rewarding relayer + // Let's also replace the weight of slashing the relayer with the weight of rewarding the relayer. if call_info.is_receive_messages_proof_call() { post_info_weight = post_info_weight.saturating_sub( ::WeightInfo::extra_weight_of_successful_receive_messages_proof_call(), ); } - // compute the relayer refund + // Compute the relayer refund. let mut post_info = *post_info; post_info.actual_weight = Some(post_info_weight); let refund = Self::Refund::compute_refund(info, &post_info, post_info_len, tip); - // we can finally reward relayer + // We can finally reward the relayer. RelayerAccountAction::Reward(relayer, reward_account_params, refund) } - /// Returns number of bundled messages `Some(_)`, if the given call info is a: + /// Returns the number of bundled messages `Some(_)`, if the given call info is a: /// - /// - message delivery transaction; + /// - Message delivery transaction; /// - /// - with reasonable bundled messages that may be accepted by the messages pallet. + /// - With reasonable bundled messages that may be accepted by the messages pallet. /// /// This function is used to check whether the transaction priority should be /// virtually boosted. The relayer registration (we only boost priority for registered /// relayer transactions) must be checked outside. fn bundled_messages_for_priority_boost(call_info: Option<&CallInfo>) -> Option { - // we only boost priority of message delivery transactions + // We only boost priority of message delivery transactions. let parsed_call = match call_info { Some(parsed_call) if parsed_call.is_receive_messages_proof_call() => parsed_call, _ => return None, }; - // compute total number of messages in transaction + // Compute the total number of messages in the transaction. let bundled_messages = parsed_call.messages_call_info().bundled_messages().saturating_len(); - // a quick check to avoid invalid high-priority transactions + // A quick check to avoid invalid high-priority transactions. let max_unconfirmed_messages_in_confirmation_tx = ::Instance, >>::MaxUnconfirmedMessagesAtInboundLane::get( @@ -456,7 +456,7 @@ where } } -/// Adapter that allow implementing `sp_runtime::traits::SignedExtension` for any +/// Adapter that allows implementing `sp_runtime::traits::SignedExtension` for any /// `RefundSignedExtension`. #[derive( DefaultNoBound, @@ -499,27 +499,27 @@ where _info: &DispatchInfoOf, _len: usize, ) -> TransactionValidity { - // this is the only relevant line of code for the `pre_dispatch` + // This is the only relevant line of code for the `pre_dispatch`. // - // we're not calling `validate` from `pre_dispatch` directly because of performance + // We're not calling `validate` from `pre_dispatch` directly because of performance // reasons, so if you're adding some code that may fail here, please check if it needs - // to be added to the `pre_dispatch` as well + // to be added to `pre_dispatch` as well. let parsed_call = T::parse_and_check_for_obsolete_call(call)?; - // the following code just plays with transaction priority and never returns an error + // The following code just plays with transaction priority and never returns an error. - // we only boost priority of presumably correct message delivery transactions + // We only boost priority of presumably correct message delivery transactions. let bundled_messages = match T::bundled_messages_for_priority_boost(parsed_call.as_ref()) { Some(bundled_messages) => bundled_messages, None => return Ok(Default::default()), }; - // we only boost priority if relayer has staked required balance + // We only boost priority if the relayer has staked the required balance. if !RelayersPallet::::is_registration_active(who) { return Ok(Default::default()) } - // compute priority boost + // Compute priority boost. let priority_boost = crate::priority_calculator::compute_priority_boost::(bundled_messages); let valid_transaction = ValidTransactionBuilder::default().priority(priority_boost); @@ -545,7 +545,7 @@ where _info: &DispatchInfoOf, _len: usize, ) -> Result { - // this is a relevant piece of `validate` that we need here (in `pre_dispatch`) + // This is a relevant piece of `validate` that we need here (in `pre_dispatch`). let parsed_call = T::parse_and_check_for_obsolete_call(call)?; Ok(parsed_call.map(|call_info| { @@ -597,12 +597,12 @@ where /// Signed extension that refunds a relayer for new messages coming from a parachain. /// -/// Also refunds relayer for successful finality delivery if it comes in batch (`utility.batchAll`) -/// with message delivery transaction. Batch may deliver either both relay chain header and +/// Also refunds the relayer for a successful finality delivery if it comes in a batch (`utility.batchAll`) +/// with a message delivery transaction. The batch may deliver either both relay chain header and /// parachain head, or just parachain head. Corresponding headers must be used in messages /// proof verification. /// -/// Extension does not refund transaction tip due to security reasons. +/// The extension does not refund the transaction tip due to security reasons. #[derive( DefaultNoBound, CloneNoBound, @@ -616,22 +616,22 @@ where #[scale_info(skip_type_params(Runtime, Para, Msgs, Refund, Priority, Id))] pub struct RefundBridgedParachainMessages( PhantomData<( - // runtime with `frame-utility`, `pallet-bridge-grandpa`, `pallet-bridge-parachains`, - // `pallet-bridge-messages` and `pallet-bridge-relayers` pallets deployed + // Runtime with `frame-utility`, `pallet-bridge-grandpa`, `pallet-bridge-parachains`, + // `pallet-bridge-messages` and `pallet-bridge-relayers` pallets deployed. Runtime, - // implementation of `RefundableParachainId` trait, which specifies the instance of - // the used `pallet-bridge-parachains` pallet and the bridged parachain id + // Implementation of `RefundableParachainId` trait, which specifies the instance of + // the used `pallet-bridge-parachains` pallet and the bridged parachain ID. Para, - // implementation of `RefundableMessagesLaneId` trait, which specifies the instance of - // the used `pallet-bridge-messages` pallet and the lane within this pallet + // Implementation of `RefundableMessagesLaneId` trait, which specifies the instance of + // the used `pallet-bridge-messages` pallet and the lane within this pallet. Msgs, - // implementation of the `RefundCalculator` trait, that is used to compute refund that - // we give to relayer for his transaction + // Implementation of the `RefundCalculator` trait, that is used to compute the refund that + // we give to the relayer for their transaction. Refund, - // getter for per-message `TransactionPriority` boost that we give to message - // delivery transactions + // Getter for per-message `TransactionPriority` boost that we give to message + // delivery transactions. Priority, - // the runtime-unique identifier of this signed extension + // The runtime-unique identifier of this signed extension. Id, )>, ); @@ -708,12 +708,12 @@ where } fn additional_call_result_check(relayer: &Runtime::AccountId, call_info: &CallInfo) -> bool { - // check if parachain state has been updated + // Check if the parachain state has been updated. if let Some(para_proof_info) = call_info.submit_parachain_heads_info() { if !SubmitParachainHeadsHelper::::was_successful( para_proof_info, ) { - // we only refund relayer if all calls have updated chain state + // We only refund the relayer if all calls have updated chain state. log::trace!( target: "runtime::bridge", "{} from parachain {} via {:?}: relayer {:?} has submitted invalid parachain finality proof", @@ -733,12 +733,12 @@ where /// Signed extension that refunds a relayer for new messages coming from a standalone (GRANDPA) /// chain. /// -/// Also refunds relayer for successful finality delivery if it comes in batch (`utility.batchAll`) -/// with message delivery transaction. Batch may deliver either both relay chain header and +/// Also refunds the relayer for successful finality delivery if it comes in a batch (`utility.batchAll`) +/// with a message delivery transaction. The batch may deliver either both relay chain header and /// parachain head, or just parachain head. Corresponding headers must be used in messages /// proof verification. /// -/// Extension does not refund transaction tip due to security reasons. +/// The extension does not refund the transaction tip due to security reasons. #[derive( DefaultNoBound, CloneNoBound, @@ -752,21 +752,21 @@ where #[scale_info(skip_type_params(Runtime, GrandpaInstance, Msgs, Refund, Priority, Id))] pub struct RefundBridgedGrandpaMessages( PhantomData<( - // runtime with `frame-utility`, `pallet-bridge-grandpa`, - // `pallet-bridge-messages` and `pallet-bridge-relayers` pallets deployed + // Runtime with `frame-utility`, `pallet-bridge-grandpa`, + // `pallet-bridge-messages` and `pallet-bridge-relayers` pallets deployed. Runtime, - // bridge GRANDPA pallet instance, used to track bridged chain state + // Bridge GRANDPA pallet instance, used to track the bridged chain state. GrandpaInstance, - // implementation of `RefundableMessagesLaneId` trait, which specifies the instance of - // the used `pallet-bridge-messages` pallet and the lane within this pallet + // Implementation of `RefundableMessagesLaneId` trait, which specifies the instance of + // the used `pallet-bridge-messages` pallet and the lane within this pallet. Msgs, - // implementation of the `RefundCalculator` trait, that is used to compute refund that - // we give to relayer for his transaction + // Implementation of the `RefundCalculator` trait, that is used to compute the refund that + // we give to the relayer for their transaction. Refund, - // getter for per-message `TransactionPriority` boost that we give to message - // delivery transactions + // Getter for per-message `TransactionPriority` boost that we give to message + // delivery transactions. Priority, - // the runtime-unique identifier of this signed extension + // The runtime-unique identifier of this signed extension. Id, )>, ); @@ -915,8 +915,8 @@ mod tests { ExistentialDeposit::get().saturating_add(test_stake * 100) } - // in tests, the following accounts are equal (because of how `into_sub_account_truncating` - // works) + // In tests, the following accounts are equal (because of how `into_sub_account_truncating` + // works). fn delivery_rewards_account() -> ThisChainAccountId { TestPaymentProcedure::rewards_account(MsgProofsRewardsAccount::get()) @@ -1497,7 +1497,7 @@ mod tests { initialize_environment(100, 100, 100); Balances::set_balance(&relayer_account_at_this_chain(), ExistentialDeposit::get()); - // message delivery is failing + // Message delivery is failing. assert_eq!(run_validate(message_delivery_call(200)), Ok(Default::default()),); assert_eq!( run_validate(parachain_finality_and_delivery_batch_call(200, 200)), @@ -1511,7 +1511,7 @@ mod tests { run_validate(all_finality_and_delivery_batch_call_ex(200, 200, 200)), Ok(Default::default()), ); - // message confirmation validation is passing + // Message confirmation validation is passing. assert_eq!( run_validate_ignore_priority(message_confirmation_call(200)), Ok(Default::default()), @@ -2056,7 +2056,7 @@ mod tests { 0, ); - // without any size/weight refund: we expect regular reward + // Without any size/weight refund: we expect a regular reward. let pre_dispatch_data = all_finality_pre_dispatch_data(); let regular_reward = expected_delivery_reward(); run_post_dispatch(Some(pre_dispatch_data), Ok(())); @@ -2068,7 +2068,7 @@ mod tests { Some(regular_reward), ); - // now repeat the same with size+weight refund: we expect smaller reward + // Now repeat the same with size + weight refund: we expect a smaller reward. let mut pre_dispatch_data = all_finality_pre_dispatch_data(); match pre_dispatch_data.call_info { CallInfo::AllFinalityAndMsgs(ref mut info, ..) => { @@ -2183,7 +2183,7 @@ mod tests { ExistentialDeposit::get() + test_stake * 10, ); - // slashing works for message delivery calls + // Slashing works for message delivery calls. BridgeRelayers::register(RuntimeOrigin::signed(relayer_account_at_this_chain()), 1000) .unwrap(); assert_eq!(Balances::reserved_balance(relayer_account_at_this_chain()), test_stake); @@ -2214,7 +2214,7 @@ mod tests { Balances::free_balance(delivery_rewards_account()) ); - // reserve doesn't work for message confirmation calls + // Reserve doesn't work for message confirmation calls. let confirmation_rewards_account_balance = Balances::free_balance(confirmation_rewards_account()); @@ -2234,7 +2234,7 @@ mod tests { run_post_dispatch(Some(all_finality_confirmation_pre_dispatch_data()), Ok(())); assert_eq!(Balances::reserved_balance(relayer_account_at_this_chain()), test_stake); - // check that unreserve has happened, not slashing + // Check that unreserve has happened, not slashing. assert_eq!( delivery_rewards_account_balance + test_stake * 3, Balances::free_balance(delivery_rewards_account()) @@ -2264,8 +2264,8 @@ mod tests { run_test(|| { initialize_environment(100, 100, 100); - // the `analyze_call_result` should return slash if number of bundled messages is - // within reasonable limits + // The `analyze_call_result` should return slash if the number of bundled messages is + // within reasonable limits. assert_eq!( run_analyze_call_result(all_finality_pre_dispatch_data(), Ok(())), RelayerAccountAction::Slash( @@ -2288,7 +2288,7 @@ mod tests { ), ); - // the `analyze_call_result` should not return slash if number of bundled messages is + // The `analyze_call_result` should not return slash if the number of bundled messages is // larger than the assert_eq!( run_analyze_call_result( @@ -2319,7 +2319,7 @@ mod tests { run_test(|| { initialize_environment(100, 100, 100); - // relay + parachain + message delivery calls batch is ignored + // Relay + parachain + message delivery calls batch is ignored. assert_eq!( TestGrandpaExtensionProvider::parse_and_check_for_obsolete_call( &all_finality_and_delivery_batch_call(200, 200, 200) @@ -2333,7 +2333,7 @@ mod tests { Ok(None), ); - // relay + parachain + message confirmation calls batch is ignored + // Relay + parachain + message confirmation calls batch is ignored. assert_eq!( TestGrandpaExtensionProvider::parse_and_check_for_obsolete_call( &all_finality_and_confirmation_batch_call(200, 200, 200) @@ -2347,7 +2347,7 @@ mod tests { Ok(None), ); - // parachain + message delivery call batch is ignored + // Parachain + message delivery call batch is ignored. assert_eq!( TestGrandpaExtensionProvider::parse_and_check_for_obsolete_call( ¶chain_finality_and_delivery_batch_call(200, 200) @@ -2355,7 +2355,7 @@ mod tests { Ok(None), ); - // parachain + message confirmation call batch is ignored + // Parachain + message confirmation call batch is ignored. assert_eq!( TestGrandpaExtensionProvider::parse_and_check_for_obsolete_call( ¶chain_finality_and_confirmation_batch_call(200, 200) @@ -2363,7 +2363,7 @@ mod tests { Ok(None), ); - // relay + message delivery call batch is accepted + // Relay + message delivery call batch is accepted. assert_eq!( TestGrandpaExtensionProvider::parse_and_check_for_obsolete_call( &relay_finality_and_delivery_batch_call(200, 200) @@ -2377,7 +2377,7 @@ mod tests { Ok(Some(relay_finality_pre_dispatch_data_ex().call_info)), ); - // relay + message confirmation call batch is accepted + // Relay + message confirmation call batch is accepted. assert_eq!( TestGrandpaExtensionProvider::parse_and_check_for_obsolete_call( &relay_finality_and_confirmation_batch_call(200, 200) @@ -2391,7 +2391,7 @@ mod tests { Ok(Some(relay_finality_confirmation_pre_dispatch_data_ex().call_info)), ); - // message delivery call batch is accepted + // Message delivery call batch is accepted. assert_eq!( TestGrandpaExtensionProvider::parse_and_check_for_obsolete_call( &message_delivery_call(200) @@ -2399,7 +2399,7 @@ mod tests { Ok(Some(delivery_pre_dispatch_data().call_info)), ); - // message confirmation call batch is accepted + // Message confirmation call batch is accepted. assert_eq!( TestGrandpaExtensionProvider::parse_and_check_for_obsolete_call( &message_confirmation_call(200) @@ -2555,11 +2555,11 @@ mod tests { let best_delivered_message = MaxUnconfirmedMessagesAtInboundLane::get(); initialize_environment(100, 100, best_delivered_message); - // register relayer so it gets priority boost + // Register relayer so it gets priority boost. BridgeRelayers::register(RuntimeOrigin::signed(relayer_account_at_this_chain()), 1000) .unwrap(); - // allow empty message delivery transactions + // Allow empty message delivery transactions. let lane_id = TestLaneId::get(); let in_lane_data = InboundLaneData { last_confirmed_nonce: 0, @@ -2571,7 +2571,7 @@ mod tests { }; pallet_bridge_messages::InboundLanes::::insert(lane_id, in_lane_data); - // now check that the priority of empty tx is the same as priority of 1-message tx + // Now check that the priority of an empty tx is the same as the priority of a 1-message tx. let priority_of_zero_messages_delivery = run_validate(message_delivery_call(best_delivered_message)).unwrap().priority; let priority_of_one_messages_delivery =