Skip to content
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

Jump to bottom

Fix max-size messages at test chains #2064

Merged
merged 2 commits into from
Apr 26, 2023
Merged

Fix max-size messages at test chains #2064

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94fb90b
fix max-size messages submit for test chains
svyatonik Apr 24, 2023
d7c8f77
impl review suggestions
svyatonik Apr 26, 2023
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
2 changes: 2 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions bin/runtime-common/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,17 @@ pub enum CustomNetworkId {
}

impl CustomNetworkId {
/// Tries to create self from bridges chain id.
pub const fn try_from_chain_id(chain: bp_runtime::ChainId) -> Option<Self> {
bkontur marked this conversation as resolved.
Show resolved Hide resolved
match chain {
bp_runtime::MILLAU_CHAIN_ID => Some(Self::Millau),
bp_runtime::RIALTO_CHAIN_ID => Some(Self::Rialto),
bp_runtime::RIALTO_PARACHAIN_CHAIN_ID => Some(Self::RialtoParachain),
_ => None,
}
}

/// Converts self to XCM' network id.
pub const fn as_network_id(&self) -> NetworkId {
match *self {
CustomNetworkId::Millau => NetworkId::Kusama,
Expand Down
7 changes: 5 additions & 2 deletions relays/bin-substrate/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ relay-utils = { path = "../utils" }
relay-westend-client = { path = "../client-westend" }
relay-wococo-client = { path = "../client-wococo" }
rialto-runtime = { path = "../../bin/rialto/runtime" }
# we are not using this runtime to craft callsour transactions, but we still need it
# to prepare large XCM messages
rialto-parachain-runtime = { path = "../../bin/rialto-parachain/runtime" }
substrate-relay-helper = { path = "../lib-substrate-relay" }

# Substrate Dependencies
Expand All @@ -62,8 +65,8 @@ polkadot-parachain = { git = "https://github.com/paritytech/polkadot", branch =
polkadot-primitives = { git = "https://github.com/paritytech/polkadot", branch = "master" }
polkadot-runtime-common = { git = "https://github.com/paritytech/polkadot", branch = "master" }
polkadot-runtime-parachains = { git = "https://github.com/paritytech/polkadot", branch = "master" }
xcm = { git = "https://github.com/paritytech/polkadot", branch = "master", default-features = false }

xcm = { git = "https://github.com/paritytech/polkadot", branch = "master" }
xcm-executor = { git = "https://github.com/paritytech/polkadot", branch = "master" }

[dev-dependencies]
bp-test-utils = { path = "../../primitives/test-utils" }
Expand Down
27 changes: 26 additions & 1 deletion relays/bin-substrate/src/chains/millau.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,36 @@
//! Millau chain specification for CLI.

use crate::cli::{encode_message::CliEncodeMessage, CliChain};
use bp_runtime::EncodedOrDecodedCall;
use bp_runtime::{ChainId, EncodedOrDecodedCall, RIALTO_CHAIN_ID, RIALTO_PARACHAIN_CHAIN_ID};
use bridge_runtime_common::CustomNetworkId;
use relay_millau_client::Millau;
use relay_substrate_client::SimpleRuntimeVersion;
use xcm_executor::traits::ExportXcm;

impl CliEncodeMessage for Millau {
fn encode_wire_message(
target: ChainId,
at_target_xcm: xcm::v3::Xcm<()>,
) -> anyhow::Result<Vec<u8>> {
let target = match target {
RIALTO_CHAIN_ID => CustomNetworkId::Rialto.as_network_id(),
RIALTO_PARACHAIN_CHAIN_ID => CustomNetworkId::RialtoParachain.as_network_id(),
_ => return Err(anyhow::format_err!("Unsupported target chain: {:?}", target)),
};

Ok(millau_runtime::xcm_config::ToRialtoOrRialtoParachainSwitchExporter::validate(
target,
0,
&mut Some(Self::dummy_universal_source()?),
&mut Some(target.into()),
&mut Some(at_target_xcm),
)
.map_err(|e| anyhow::format_err!("Failed to prepare outbound message: {:?}", e))?
.0
.1
.0)
}

fn encode_execute_xcm(
message: xcm::VersionedXcm<Self::Call>,
) -> anyhow::Result<EncodedOrDecodedCall<Self::Call>> {
Expand Down
25 changes: 24 additions & 1 deletion relays/bin-substrate/src/chains/rialto.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,34 @@
//! Rialto chain specification for CLI.

use crate::cli::{encode_message::CliEncodeMessage, CliChain};
use bp_runtime::EncodedOrDecodedCall;
use bp_runtime::{ChainId, EncodedOrDecodedCall, MILLAU_CHAIN_ID};
use bridge_runtime_common::CustomNetworkId;
use relay_rialto_client::Rialto;
use relay_substrate_client::SimpleRuntimeVersion;
use xcm_executor::traits::ExportXcm;

impl CliEncodeMessage for Rialto {
fn encode_wire_message(
target: ChainId,
at_target_xcm: xcm::v3::Xcm<()>,
) -> anyhow::Result<Vec<u8>> {
let target = match target {
MILLAU_CHAIN_ID => CustomNetworkId::Millau.as_network_id(),
_ => return Err(anyhow::format_err!("Unsupported target chian: {:?}", target)),
};

Ok(rialto_runtime::millau_messages::ToMillauBlobExporter::validate(
target,
0,
&mut Some(Self::dummy_universal_source()?),
&mut Some(target.into()),
&mut Some(at_target_xcm),
)
.map_err(|e| anyhow::format_err!("Failed to prepare outbound message: {:?}", e))?
.0
.0)
}

fn encode_execute_xcm(
message: xcm::VersionedXcm<Self::Call>,
) -> anyhow::Result<EncodedOrDecodedCall<Self::Call>> {
Expand Down
25 changes: 24 additions & 1 deletion relays/bin-substrate/src/chains/rialto_parachain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,34 @@
//! Rialto parachain specification for CLI.

use crate::cli::{encode_message::CliEncodeMessage, CliChain};
use bp_runtime::EncodedOrDecodedCall;
use bp_runtime::{ChainId, EncodedOrDecodedCall, MILLAU_CHAIN_ID};
use bridge_runtime_common::CustomNetworkId;
use relay_rialto_parachain_client::RialtoParachain;
use relay_substrate_client::SimpleRuntimeVersion;
use xcm_executor::traits::ExportXcm;

impl CliEncodeMessage for RialtoParachain {
fn encode_wire_message(
target: ChainId,
at_target_xcm: xcm::v3::Xcm<()>,
) -> anyhow::Result<Vec<u8>> {
let target = match target {
MILLAU_CHAIN_ID => CustomNetworkId::Millau.as_network_id(),
_ => return Err(anyhow::format_err!("Unsupported target chain: {:?}", target)),
};

Ok(rialto_parachain_runtime::millau_messages::ToMillauBlobExporter::validate(
target,
0,
&mut Some(Self::dummy_universal_source()?),
&mut Some(target.into()),
&mut Some(at_target_xcm),
)
.map_err(|e| anyhow::format_err!("Failed to prepare outbound message: {:?}", e))?
.0
.0)
}

fn encode_execute_xcm(
message: xcm::VersionedXcm<Self::Call>,
) -> anyhow::Result<EncodedOrDecodedCall<Self::Call>> {
Expand Down
102 changes: 74 additions & 28 deletions relays/bin-substrate/src/cli/encode_message.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,15 @@
// along with Parity Bridges Common. If not, see <http://www.gnu.org/licenses/>.

use crate::cli::{ExplicitOrMaximal, HexBytes};
use bp_runtime::EncodedOrDecodedCall;
use bp_runtime::{
ChainId, EncodedOrDecodedCall, MILLAU_CHAIN_ID, RIALTO_CHAIN_ID, RIALTO_PARACHAIN_CHAIN_ID,
};
use bridge_runtime_common::CustomNetworkId;
use codec::Encode;
use frame_support::weights::Weight;
use relay_substrate_client::Chain;
use structopt::StructOpt;
use xcm::latest::prelude::*;

/// All possible messages that may be delivered to generic Substrate chain.
///
Expand All @@ -43,6 +47,31 @@ pub enum Message {
pub type RawMessage = Vec<u8>;

pub trait CliEncodeMessage: Chain {
/// Returns dummy `AccountId32` universal source given this network id.
fn dummy_universal_source() -> anyhow::Result<xcm::v3::Junctions> {
use xcm::v3::prelude::*;

let this_network = CustomNetworkId::try_from_chain_id(Self::ID)
.map(|n| n.as_network_id())
.ok_or_else(|| anyhow::format_err!("Unsupported chain: {:?}", Self::ID))?;
let this_location: InteriorMultiLocation = this_network.into();

let origin = MultiLocation {
parents: 0,
interior: X1(AccountId32 { network: Some(this_network), id: [0u8; 32] }),
};
let universal_source = this_location
.within_global(origin)
.map_err(|e| anyhow::format_err!("Invalid location: {:?}", e))?;

Ok(universal_source)
}
/// Returns XCM blob that is passed to the `send_message` function of the messages pallet
/// and then is sent over the wire.
fn encode_wire_message(
target: ChainId,
at_target_xcm: xcm::v3::Xcm<()>,
) -> anyhow::Result<Vec<u8>>;
/// Encode an `execute` XCM call of the XCM pallet.
fn encode_execute_xcm(
message: xcm::VersionedXcm<Self::Call>,
Expand All @@ -56,41 +85,45 @@ pub trait CliEncodeMessage: Chain {
}

/// Encode message payload passed through CLI flags.
pub(crate) fn encode_message<Source: Chain, Target: Chain>(
pub(crate) fn encode_message<Source: CliEncodeMessage, Target: Chain>(
message: &Message,
) -> anyhow::Result<RawMessage> {
Ok(match message {
Message::Raw { ref data } => data.0.clone(),
Message::Sized { ref size } => {
let expected_xcm_size = match *size {
let destination = match Target::ID {
Copy link
Contributor

@bkontur bkontur Apr 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

can we use try_from_chain_id here? Something like:

match CustomNetworkId::try_from_chain_id(Target::ID) {
    Some(network_id) => network_id,
    _ => return Err(anyhow::format_err!("Unsupported target chain: {:?}", Target::ID)),
}

Copy link
Contributor

Choose a reason for hiding this comment

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

and/or maybe implement TryFrom for CustomNetworkId

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can we use try_from_chain_id here? Something like:

Indeed, I've missed that. Thanks

and/or maybe implement TryFrom for CustomNetworkId

The idea was to make the pairing fn for as_network_id, which is const fn, but right now we don't need it. So changed to the idiomatic way

MILLAU_CHAIN_ID => CustomNetworkId::Millau.as_network_id(),
RIALTO_CHAIN_ID => CustomNetworkId::RialtoParachain.as_network_id(),
RIALTO_PARACHAIN_CHAIN_ID => CustomNetworkId::RialtoParachain.as_network_id(),
_ => return Err(anyhow::format_err!("Unsupported target chain: {:?}", Target::ID)),
};
let expected_size = match *size {
ExplicitOrMaximal::Explicit(size) => size,
ExplicitOrMaximal::Maximal => compute_maximal_message_size(
Source::max_extrinsic_size(),
Target::max_extrinsic_size(),
),
};

// there's no way to craft XCM of the given size - we'll be using `ExpectPallet`
// instruction, which has byte vector inside
let mut current_vec_size = expected_xcm_size;
let xcm = loop {
let xcm = xcm::VersionedXcm::<()>::V3(
vec![xcm::v3::Instruction::ExpectPallet {
index: 0,
name: vec![42; current_vec_size as usize],
module_name: vec![],
crate_major: 0,
min_crate_minor: 0,
}]
.into(),
);
if xcm.encode().len() <= expected_xcm_size as usize {
break xcm
}

current_vec_size -= 1;
};
xcm.encode()
} as usize;

let at_target_xcm = vec![xcm::v3::Instruction::ClearOrigin; expected_size].into();
let at_target_xcm_size =
Source::encode_wire_message(Target::ID, at_target_xcm)?.encoded_size();
let at_target_xcm_overhead = at_target_xcm_size.saturating_sub(expected_size);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we get the overhead just by calling encode_wire_message(Target::ID, vec![]) ? Or by sending a None as the xcm message to validate() (inside encode_wire_message()) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It'll probably make our messages 1 or 2 bytes above the maximal allowed size because Compact<u32> (length of XCM instructions vector) is larger for larger vectors => this message will be rejected. Current approach may made messages 1-byte smaller in edge cases, but it is still fine. And yeah - it seems a bit hacky, but I haven't found any better way

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, agree that this version is the safest.

let at_target_xcm = vec![
xcm::v3::Instruction::ClearOrigin;
expected_size.saturating_sub(at_target_xcm_overhead)
]
.into();

xcm::VersionedXcm::<()>::V3(
vec![ExportMessage {
network: destination,
destination: destination.into(),
xcm: at_target_xcm,
}]
.into(),
)
.encode()
},
})
}
Expand Down Expand Up @@ -123,13 +156,21 @@ mod tests {
use relay_millau_client::Millau;
use relay_rialto_client::Rialto;

fn approximate_message_size<Source: CliEncodeMessage>(xcm_msg_len: usize) -> usize {
xcm_msg_len + Source::dummy_universal_source().unwrap().encoded_size()
}

#[test]
fn encode_explicit_size_message_works() {
let msg = encode_message::<Rialto, Millau>(&Message::Sized {
size: ExplicitOrMaximal::Explicit(100),
})
.unwrap();
assert_eq!(msg.len(), 100);
// since it isn't the returned XCM what is sent over the wire, we can only check if
// it is close to what we need
assert!(
(1f64 - (approximate_message_size::<Rialto>(msg.len()) as f64) / 100_f64).abs() < 0.1
);
// check that it decodes to valid xcm
let _ = decode_xcm::<()>(msg).unwrap();
}
Expand All @@ -144,7 +185,12 @@ mod tests {
let msg =
encode_message::<Rialto, Millau>(&Message::Sized { size: ExplicitOrMaximal::Maximal })
.unwrap();
assert_eq!(msg.len(), maximal_size as usize);
// since it isn't the returned XCM what is sent over the wire, we can only check if
// it is close to what we need
assert!(
(1f64 - approximate_message_size::<Rialto>(msg.len()) as f64 / maximal_size as f64)
.abs() < 0.1
);
// check that it decodes to valid xcm
let _ = decode_xcm::<()>(msg).unwrap();
}
Expand Down