Skip to content

Remove without_storage_info for messages pallet #1487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
svyatonik merged 2 commits into from
Jul 4, 2022
Merged

Remove without_storage_info for messages pallet #1487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5df8917
draft: remove without_storage_info for messages pallet
svyatonik Jul 1, 2022
bdbd087
some cleanup
svyatonik Jul 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions bin/millau/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ impl pallet_bridge_messages::Config<WithRialtoMessagesInstance> for Runtime {
type MaxUnrewardedRelayerEntriesAtInboundLane = MaxUnrewardedRelayerEntriesAtInboundLane;
type MaxUnconfirmedMessagesAtInboundLane = MaxUnconfirmedMessagesAtInboundLane;

type MaximalOutboundPayloadSize = crate::rialto_messages::ToRialtoMaximalOutboundPayloadSize;
type OutboundPayload = crate::rialto_messages::ToRialtoMessagePayload;
type OutboundMessageFee = Balance;

Expand Down Expand Up @@ -484,6 +485,8 @@ impl pallet_bridge_messages::Config<WithRialtoParachainMessagesInstance> for Run
type MaxUnrewardedRelayerEntriesAtInboundLane = MaxUnrewardedRelayerEntriesAtInboundLane;
type MaxUnconfirmedMessagesAtInboundLane = MaxUnconfirmedMessagesAtInboundLane;

type MaximalOutboundPayloadSize =
crate::rialto_parachain_messages::ToRialtoParachainMaximalOutboundPayloadSize;
type OutboundPayload = crate::rialto_parachain_messages::ToRialtoParachainMessagePayload;
type OutboundMessageFee = Balance;

Expand Down
15 changes: 8 additions & 7 deletions bin/millau/runtime/src/rialto_messages.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ pub type FromRialtoMessageDispatch = messages::target::FromBridgedChainMessageDi
frame_support::traits::ConstU64<BASE_XCM_WEIGHT_TWICE>,
>;

/// Maximal outbound payload size of Millau -> Rialto messages.
pub type ToRialtoMaximalOutboundPayloadSize =
messages::source::FromThisChainMaximalOutboundPayloadSize<WithRialtoMessageBridge>;

/// Millau <-> Rialto message bridge.
#[derive(RuntimeDebug, Clone, Copy)]
pub struct WithRialtoMessageBridge;
Expand Down Expand Up @@ -145,12 +149,9 @@ impl messages::ThisChainWithMessages for Millau {
}

fn estimate_delivery_confirmation_transaction() -> MessageTransaction<Weight> {
let inbound_data_size = InboundLaneData::<bp_millau::AccountId>::encoded_size_hint(
bp_millau::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE,
1,
1,
)
.unwrap_or(u32::MAX);
let inbound_data_size = InboundLaneData::<bp_millau::AccountId>::encoded_size_hint(1, 1)
.and_then(|x| u32::try_from(x).ok())
.unwrap_or(u32::MAX);

MessageTransaction {
dispatch_weight: bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT,
Expand Down Expand Up @@ -346,10 +347,10 @@ mod tests {

let max_incoming_inbound_lane_data_proof_size =
bp_messages::InboundLaneData::<()>::encoded_size_hint(
bp_millau::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE,
bp_millau::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX as _,
bp_millau::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX as _,
)
.and_then(|x| u32::try_from(x).ok())
.unwrap_or(u32::MAX);
pallet_bridge_messages::ensure_able_to_receive_confirmation::<Weights>(
bp_millau::Millau::max_extrinsic_size(),
Expand Down
13 changes: 7 additions & 6 deletions bin/millau/runtime/src/rialto_parachain_messages.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ pub type FromRialtoParachainMessageDispatch = messages::target::FromBridgedChain
frame_support::traits::ConstU64<BASE_XCM_WEIGHT_TWICE>,
>;

/// Maximal outbound payload size of Millau -> RialtoParachain messages.
pub type ToRialtoParachainMaximalOutboundPayloadSize =
messages::source::FromThisChainMaximalOutboundPayloadSize<WithRialtoParachainMessageBridge>;

/// Millau <-> RialtoParachain message bridge.
#[derive(RuntimeDebug, Clone, Copy)]
pub struct WithRialtoParachainMessageBridge;
Expand Down Expand Up @@ -134,12 +138,9 @@ impl messages::ThisChainWithMessages for Millau {
}

fn estimate_delivery_confirmation_transaction() -> MessageTransaction<Weight> {
Copy link
Copy Markdown
Collaborator

@serban300 serban300 Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method seems to have the same implementation everywhere. Would it make sense to define in on the trait ? Of course we would also need to add MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT, EXTRA_STORAGE_PROOF_SIZE and TX_EXTRA_BYTES to the trait definition.

Copy link
Copy Markdown
Contributor Author

@svyatonik svyatonik Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it worth to have single implementation, thanks! But probably let's not move all chain weight specific constants to the generic trait (ThisChainWithMessages). Some chains still may use different techniques to "estimate" this tx => may even not have such constants. WDYT about:

trait ThisChainWithMessages {
  ...
  type DeliveryTransactionEstimation: DeliveryTransactionEstimation;
  ...
}

trait DeliveryTransactionEstimation {
  fn estimate_delivery_confirmation_transaction() -> MessageTransaction<Weight>;
}

struct BasicDeliveryTransactionEstimation<const MaxDeliverTxWeight:Weight, const ExtraStorageProofSize: u32, const TxExtraBytes: u32>(PhantomData);

impl<const MaxDeliverTxWeight:Weight, const ExtraStorageProofSize: u32, const TxExtraBytes: u32> DeliveryTransactionEstimation for BasicDeliveryTransactionEstimation<...> {
fn estimate_delivery_confirmation_transaction() -> MessageTransaction<Weight> {
 // use consts here
}
}

Copy link
Copy Markdown
Collaborator

@serban300 serban300 Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

let inbound_data_size = InboundLaneData::<bp_millau::AccountId>::encoded_size_hint(
bp_millau::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE,
1,
1,
)
.unwrap_or(u32::MAX);
let inbound_data_size = InboundLaneData::<bp_millau::AccountId>::encoded_size_hint(1, 1)
.and_then(|x| u32::try_from(x).ok())
.unwrap_or(u32::MAX);

MessageTransaction {
dispatch_weight: bp_millau::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT,
Expand Down
1 change: 1 addition & 0 deletions bin/rialto-parachain/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ impl pallet_bridge_messages::Config<WithMillauMessagesInstance> for Runtime {
type MaxUnrewardedRelayerEntriesAtInboundLane = MaxUnrewardedRelayerEntriesAtInboundLane;
type MaxUnconfirmedMessagesAtInboundLane = MaxUnconfirmedMessagesAtInboundLane;

type MaximalOutboundPayloadSize = crate::millau_messages::ToMillauMaximalOutboundPayloadSize;
type OutboundPayload = crate::millau_messages::ToMillauMessagePayload;
type OutboundMessageFee = Balance;

Expand Down
13 changes: 7 additions & 6 deletions bin/rialto-parachain/runtime/src/millau_messages.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ pub type FromMillauMessagesProof = messages::target::FromBridgedChainMessagesPro
pub type ToMillauMessagesDeliveryProof =
messages::source::FromBridgedChainMessagesDeliveryProof<bp_millau::Hash>;

/// Maximal outbound payload size of Rialto -> Millau messages.
pub type ToMillauMaximalOutboundPayloadSize =
messages::source::FromThisChainMaximalOutboundPayloadSize<WithMillauMessageBridge>;

/// Millau <-> RialtoParachain message bridge.
#[derive(RuntimeDebug, Clone, Copy)]
pub struct WithMillauMessageBridge;
Expand Down Expand Up @@ -134,12 +138,9 @@ impl messages::ThisChainWithMessages for RialtoParachain {

fn estimate_delivery_confirmation_transaction() -> MessageTransaction<Weight> {
let inbound_data_size =
InboundLaneData::<bp_rialto_parachain::AccountId>::encoded_size_hint(
bp_rialto_parachain::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE,
1,
1,
)
.unwrap_or(u32::MAX);
InboundLaneData::<bp_rialto_parachain::AccountId>::encoded_size_hint(1, 1)
.and_then(|x| u32::try_from(x).ok())
.unwrap_or(u32::MAX);

MessageTransaction {
dispatch_weight:
Expand Down
1 change: 1 addition & 0 deletions bin/rialto/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ impl pallet_bridge_messages::Config<WithMillauMessagesInstance> for Runtime {
type MaxUnrewardedRelayerEntriesAtInboundLane = MaxUnrewardedRelayerEntriesAtInboundLane;
type MaxUnconfirmedMessagesAtInboundLane = MaxUnconfirmedMessagesAtInboundLane;

type MaximalOutboundPayloadSize = crate::millau_messages::ToMillauMaximalOutboundPayloadSize;
type OutboundPayload = crate::millau_messages::ToMillauMessagePayload;
type OutboundMessageFee = Balance;

Expand Down
15 changes: 8 additions & 7 deletions bin/rialto/runtime/src/millau_messages.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ pub type FromMillauMessagesProof = messages::target::FromBridgedChainMessagesPro
pub type ToMillauMessagesDeliveryProof =
messages::source::FromBridgedChainMessagesDeliveryProof<bp_millau::Hash>;

/// Maximal outbound payload size of Rialto -> Millau messages.
pub type ToMillauMaximalOutboundPayloadSize =
messages::source::FromThisChainMaximalOutboundPayloadSize<WithMillauMessageBridge>;

/// Millau <-> Rialto message bridge.
#[derive(RuntimeDebug, Clone, Copy)]
pub struct WithMillauMessageBridge;
Expand Down Expand Up @@ -144,12 +148,9 @@ impl messages::ThisChainWithMessages for Rialto {
}

fn estimate_delivery_confirmation_transaction() -> MessageTransaction<Weight> {
let inbound_data_size = InboundLaneData::<bp_rialto::AccountId>::encoded_size_hint(
bp_rialto::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE,
1,
1,
)
.unwrap_or(u32::MAX);
let inbound_data_size = InboundLaneData::<bp_rialto::AccountId>::encoded_size_hint(1, 1)
.and_then(|x| u32::try_from(x).ok())
.unwrap_or(u32::MAX);

MessageTransaction {
dispatch_weight: bp_rialto::MAX_SINGLE_MESSAGE_DELIVERY_CONFIRMATION_TX_WEIGHT,
Expand Down Expand Up @@ -343,10 +344,10 @@ mod tests {

let max_incoming_inbound_lane_data_proof_size =
bp_messages::InboundLaneData::<()>::encoded_size_hint(
bp_rialto::MAXIMAL_ENCODED_ACCOUNT_ID_SIZE,
bp_rialto::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX as _,
bp_rialto::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX as _,
)
.and_then(|x| u32::try_from(x).ok())
.unwrap_or(u32::MAX);
pallet_bridge_messages::ensure_able_to_receive_confirmation::<Weights>(
bp_rialto::Rialto::max_extrinsic_size(),
Expand Down
13 changes: 11 additions & 2 deletions bin/runtime-common/src/messages.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,15 @@ pub mod source {
/// Message payload for This -> Bridged chain messages.
pub type FromThisChainMessagePayload = Vec<u8>;

/// Maximal size of outbound message payload.
pub struct FromThisChainMaximalOutboundPayloadSize<B>(PhantomData<B>);

impl<B: MessageBridge> Get<u32> for FromThisChainMaximalOutboundPayloadSize<B> {
fn get() -> u32 {
maximal_message_size::<B>()
}
}

/// Messages delivery proof from bridged chain:
///
/// - hash of finalized header;
Expand All @@ -216,7 +225,7 @@ pub mod source {
}

impl<BridgedHeaderHash> Size for FromBridgedChainMessagesDeliveryProof<BridgedHeaderHash> {
fn size_hint(&self) -> u32 {
fn size(&self) -> u32 {
u32::try_from(
self.storage_proof
.iter()
Expand Down Expand Up @@ -529,7 +538,7 @@ pub mod target {
}

impl<BridgedHeaderHash> Size for FromBridgedChainMessagesProof<BridgedHeaderHash> {
fn size_hint(&self) -> u32 {
fn size(&self) -> u32 {
u32::try_from(
self.storage_proof
.iter()
Expand Down
81 changes: 77 additions & 4 deletions modules/messages/src/inbound_lane.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@

//! Everything about incoming messages receival.

use crate::Config;

use bp_messages::{
target_chain::{DispatchMessage, DispatchMessageData, MessageDispatch},
DeliveredMessages, InboundLaneData, LaneId, MessageKey, MessageNonce, OutboundLaneData,
UnrewardedRelayer,
};
use bp_runtime::messages::MessageDispatchResult;
use frame_support::RuntimeDebug;
use codec::{Decode, Encode, EncodeLike, MaxEncodedLen};
use frame_support::{traits::Get, RuntimeDebug};
use scale_info::{Type, TypeInfo};
use sp_std::prelude::PartialEq;

/// Inbound lane storage.
Expand All @@ -44,6 +48,76 @@ pub trait InboundLaneStorage {
fn set_data(&mut self, data: InboundLaneData<Self::Relayer>);
}

/// Inbound lane data wrapper that implements `MaxEncodedLen`.
///
/// We have already had `MaxEncodedLen`-like functionality before, but its usage has
/// been localized and we haven't been passing bounds (maximal count of unrewarded relayer entries,
/// maximal count of unconfirmed messages) everywhere. This wrapper allows us to avoid passing
/// these generic bounds all over the code.
///
/// The encoding of this type matches encoding of the corresponding `MessageData`.
#[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, Eq)]
pub struct StoredInboundLaneData<T: Config<I>, I: 'static>(pub InboundLaneData<T::InboundRelayer>);

impl<T: Config<I>, I: 'static> sp_std::ops::Deref for StoredInboundLaneData<T, I> {
type Target = InboundLaneData<T::InboundRelayer>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<T: Config<I>, I: 'static> sp_std::ops::DerefMut for StoredInboundLaneData<T, I> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

impl<T: Config<I>, I: 'static> Default for StoredInboundLaneData<T, I> {
fn default() -> Self {
StoredInboundLaneData(Default::default())
}
}

impl<T: Config<I>, I: 'static> From<InboundLaneData<T::InboundRelayer>>
for StoredInboundLaneData<T, I>
{
fn from(data: InboundLaneData<T::InboundRelayer>) -> Self {
StoredInboundLaneData(data)
}
}

impl<T: Config<I>, I: 'static> From<StoredInboundLaneData<T, I>>
for InboundLaneData<T::InboundRelayer>
{
fn from(data: StoredInboundLaneData<T, I>) -> Self {
data.0
}
}

impl<T: Config<I>, I: 'static> EncodeLike<StoredInboundLaneData<T, I>>
for InboundLaneData<T::InboundRelayer>
{
}

impl<T: Config<I>, I: 'static> TypeInfo for StoredInboundLaneData<T, I> {
type Identity = Self;

fn type_info() -> Type {
InboundLaneData::<T::InboundRelayer>::type_info()
}
}

impl<T: Config<I>, I: 'static> MaxEncodedLen for StoredInboundLaneData<T, I> {
fn max_encoded_len() -> usize {
InboundLaneData::<T::InboundRelayer>::encoded_size_hint(
T::MaxUnrewardedRelayerEntriesAtInboundLane::get() as usize,
T::MaxUnconfirmedMessagesAtInboundLane::get() as usize,
)
.unwrap_or(usize::MAX)
}
}

/// Result of single message receival.
#[derive(RuntimeDebug, PartialEq, Eq)]
pub enum ReceivalResult {
Expand Down Expand Up @@ -333,7 +407,7 @@ mod tests {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(TEST_LANE_ID);
let max_nonce =
<TestRuntime as crate::Config>::MaxUnrewardedRelayerEntriesAtInboundLane::get();
<TestRuntime as Config>::MaxUnrewardedRelayerEntriesAtInboundLane::get();
for current_nonce in 1..max_nonce + 1 {
assert_eq!(
lane.receive_message::<TestMessageDispatch, _>(
Expand Down Expand Up @@ -372,8 +446,7 @@ mod tests {
fn fails_to_receive_messages_above_unconfirmed_messages_limit_per_lane() {
run_test(|| {
let mut lane = inbound_lane::<TestRuntime, _>(TEST_LANE_ID);
let max_nonce =
<TestRuntime as crate::Config>::MaxUnconfirmedMessagesAtInboundLane::get();
let max_nonce = <TestRuntime as Config>::MaxUnconfirmedMessagesAtInboundLane::get();
for current_nonce in 1..=max_nonce {
assert_eq!(
lane.receive_message::<TestMessageDispatch, _>(
Expand Down
Loading