Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add HrmpChannelManagementHooks #2661

Closed
wants to merge 14 commits into from
18 changes: 18 additions & 0 deletions runtime/parachains/src/hrmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use sp_std::{
collections::{btree_map::BTreeMap, btree_set::BTreeSet},
prelude::*,
};
use xcm::v0::{Result as XcmResult, ExecuteHrmp};

/// A description of a request to open an HRMP channel.
#[derive(Encode, Decode)]
Expand Down Expand Up @@ -412,6 +413,23 @@ decl_module! {
}
}

impl<T: Config> ExecuteHrmp for Module<T> {
fn hrmp_init_open_channel(sender: u32, recipient: u32, max_message_size: u32, max_capacity: u32) -> XcmResult {
Self::init_open_channel(sender.into(), recipient.into(), max_capacity, max_message_size).map_err(|_| ().into())
Copy link
Contributor Author

@4meta5 4meta5 Mar 22, 2021

Choose a reason for hiding this comment

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

Right now, this is returning the XcmError::Undefined if a runtime error is returned. What error should it be?

}
fn hrmp_accept_open_channel(recipient: u32, sender: u32) -> XcmResult {
Self::accept_open_channel(recipient.into(), sender.into()).map_err(|_| ().into())
}
fn hrmp_close_channel(initiator: u32, sender: u32, recipient: u32) -> XcmResult {
Self::close_channel(initiator.into(),
HrmpChannelId {
sender: sender.into(),
recipient: recipient.into(),
}
).map_err(|_| ().into())
}
}

/// Routines and getters related to HRMP.
impl<T: Config> Module<T> {
/// Block initialization logic, called by initializer.
Expand Down
1 change: 1 addition & 0 deletions runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,7 @@ impl xcm_executor::Config for XcmConfig {
type Call = Call;
type XcmSender = xcm_sender::RelayChainXcmSender<Runtime>;
type AssetTransactor = LocalAssetTransactor;
type HrmpExecutor = Hrmp;
4meta5 marked this conversation as resolved.
Show resolved Hide resolved
type OriginConverter = LocalOriginConverter;
type IsReserve = ();
type IsTeleporter = ();
Expand Down
49 changes: 48 additions & 1 deletion xcm/src/v0/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub use junction::{Junction, NetworkId};
pub use multi_asset::{MultiAsset, AssetInstance};
pub use multi_location::MultiLocation;
pub use order::Order;
pub use traits::{Error, Result, SendXcm, ExecuteXcm};
pub use traits::{Error, Result, SendXcm, ExecuteXcm, ExecuteHrmp};

// TODO: Efficient encodings for Vec<MultiAsset>, Vec<Order>, using initial byte values 128+ to encode the number of
// items in the vector.
Expand Down Expand Up @@ -203,6 +203,53 @@ pub enum Xcm {
#[codec(compact)] sender: u32,
#[codec(compact)] recipient: u32,
},

/// A message to propose opening a channel on the relay-chain between the
/// sender para to the recipient para.
///
/// - `recipient`: The recipient in the to-be opened channel.
/// - `max_message_size`: The maximum size of a message proposed by the sender.
/// - `max_capacity`: The maximum number of messages that can be queued in the channel.
///
/// Safety: The message should originate directly from the sender para.
///
/// Kind: *Instruction*.
///
/// Errors:
HrmpInitOpenChannel {
#[codec(compact)] recipient: u32,
#[codec(compact)] max_message_size: u32,
#[codec(compact)] max_capacity: u32,
},

/// A message to accept opening a channel on the relay-chain.
///
/// - `sender`: The sender in the to-be opened channel.
///
/// Safety: The message should originate directly from the recipient para.
///
/// Kind: *Instruction*.
///
/// Errors:
HrmpAcceptOpenChannel {
#[codec(compact)] sender: u32,
},

/// A message to close a channel on the relay-chain.
///
/// - `sender`: The sender in the to-be closed channel.
/// - `recipient`: The recipient in the to-be closed channel.
///
/// Safety: The message should originate directly from the sender or
/// recipient para (either member of the channel).
///
/// Kind: *Instruction*.
///
/// Errors:
HrmpCloseChannel {
#[codec(compact)] sender: u32,
#[codec(compact)] recipient: u32,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Recalling an open channel request is upcoming in #3543

I think it's worth to reconsider these messages in the light of that PR.

}

impl From<Xcm> for VersionedXcm {
Expand Down
18 changes: 18 additions & 0 deletions xcm/src/v0/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,21 @@ impl SendXcm for () {
Err(Error::Unimplemented)
}
}

pub trait ExecuteHrmp {
4meta5 marked this conversation as resolved.
Show resolved Hide resolved
fn hrmp_init_open_channel(sender: u32, recipient: u32, max_message_size: u32, max_capacity: u32) -> Result;
fn hrmp_accept_open_channel(recipient: u32, sender: u32) -> Result;
fn hrmp_close_channel(initiator: u32, sender: u32, recipient: u32) -> Result;
Copy link
Contributor

Choose a reason for hiding this comment

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

DQ: why are all parameters u32 all across the place?

Copy link
Contributor

Choose a reason for hiding this comment

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

I presumed it's the same reason why we use u32 within XCM definitions. The alternative you are thinking of is ParaId. That comes from the primitives crate. Unlike one may think, it can be pretty heavy, bringing all kinds of cruft. At the same time, we want this crate to be used pervasively thus we want to minimize the number of dependencies.

}

impl ExecuteHrmp for () {
fn hrmp_init_open_channel(_sender: u32, _recipient: u32, _max_message_size: u32, _max_capacity: u32) -> Result {
Err(().into())
}
fn hrmp_accept_open_channel(_recipient: u32, _sender: u32) -> Result {
Err(().into())
}
fn hrmp_close_channel(_initiator: u32, _sender: u32, _recipient: u32) -> Result {
Err(().into())
}
}
5 changes: 4 additions & 1 deletion xcm/xcm-executor/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

use xcm::v0::SendXcm;
use xcm::v0::{SendXcm, ExecuteHrmp};
use frame_support::dispatch::{Dispatchable, Parameter};
use crate::traits::{TransactAsset, ConvertOrigin, FilterAssetLocation, InvertLocation};

Expand All @@ -29,6 +29,9 @@ pub trait Config {
/// How to withdraw and deposit an asset.
type AssetTransactor: TransactAsset;

/// How to execute HRMP-related actions
type HrmpExecutor: ExecuteHrmp;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could be improved by specifying what exactly actions are (i.e. the channel management). It is also better to take into account how the developers will approach it. I think this is mostly relevant for relay-chains which is a way less common case. Thus it would be good to guide a typical implementer of this to use the () dummy


/// How to get a call origin from a `OriginKind` value.
type OriginConverter: ConvertOrigin<<Self::Call as Dispatchable>::Origin>;

Expand Down
23 changes: 22 additions & 1 deletion xcm/xcm-executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use sp_std::{prelude::*, marker::PhantomData, convert::TryInto};
use frame_support::{ensure, dispatch::Dispatchable};
use parity_scale_codec::Decode;
use xcm::v0::{
Xcm, Order, ExecuteXcm, SendXcm, Error as XcmError, Result as XcmResult,
Xcm, Order, ExecuteXcm, ExecuteHrmp, SendXcm, Error as XcmError, Result as XcmResult,
MultiLocation, MultiAsset, Junction,
};

Expand Down Expand Up @@ -95,6 +95,27 @@ impl<Config: config::Config> ExecuteXcm for XcmExecutor<Config> {
// message makes sense.
return Ok(());
}
(origin, Xcm::HrmpInitOpenChannel { recipient, max_message_size, max_capacity } ) => {
let sender = match origin {
MultiLocation::X1(Junction::Parachain { id }) => id,
_ => Err(XcmError::BadOrigin)?,
};
return Config::HrmpExecutor::hrmp_init_open_channel(sender, recipient, max_message_size, max_capacity)
}
(origin, Xcm::HrmpAcceptOpenChannel { sender } ) => {
let recipient = match origin {
MultiLocation::X1(Junction::Parachain { id }) => id,
_ => Err(XcmError::BadOrigin)?,
};
return Config::HrmpExecutor::hrmp_accept_open_channel(recipient, sender)
}
(origin, Xcm::HrmpCloseChannel { sender, recipient } ) => {
let initiator = match origin {
MultiLocation::X1(Junction::Parachain { id }) => id,
_ => Err(XcmError::BadOrigin)?,
};
return Config::HrmpExecutor::hrmp_close_channel(initiator, sender, recipient)
}
(origin, Xcm::RelayTo { dest: MultiLocation::X1(Junction::Parachain { id }), inner }) => {
let msg = Xcm::RelayedFrom { superorigin: origin, inner }.into();
return Config::XcmSender::send_xcm(Junction::Parachain { id }.into(), msg)
Expand Down