Validate memo in transfer extrinsic (#417)

* Validate memo in `transfer` extrinsic

Added `ValidateMemo` trait

contracts/pallet-ibc/src/ics20/mod.rs

@@ -919,3 +919,7 @@ where
 		);
 	}
 }
+
+pub trait ValidateMemo {
+	fn validate(&self) -> Result<(), String>;
+}

contracts/pallet-ibc/src/lib.rs

@@ -30,6 +30,7 @@
 #[macro_use]
 extern crate alloc;
 
+use crate::ics20::ValidateMemo;
 use codec::{Decode, Encode};
 use core::fmt::Debug;
 use cumulus_primitives_core::ParaId;
@@ -273,7 +274,8 @@ pub mod pallet {
 			+ Clone
 			+ Eq
 			+ TryFrom<crate::ics20::MemoData>
-			+ TryInto<crate::ics20::MemoData>;
+			+ TryInto<crate::ics20::MemoData>
+			+ ValidateMemo;
 
 		type SubstrateMultihopXcmHandler: SubstrateMultihopXcmHandler<AccountId = Self::AccountId>;
 
@@ -716,10 +718,15 @@ pub mod pallet {
 		/// Access denied
 		AccessDenied,
 		RateLimiter,
-		//Fee errors
+		/// Fee errors
 		FailedSendFeeToAccount,
-		//Failed to derive origin sender address.
+		/// Failed to derive origin sender address.
 		OriginAddress,
+		/// The memo hasn't passed the validation. Potential reasons:
+		/// - The memo is too long.
+		/// - The memo is in invalid format
+		/// - The memo contains unsupported middlewares
+		InvalidMemo,
 	}
 
 	#[pallet::hooks]
@@ -961,6 +968,15 @@ pub mod pallet {
 				});
 			};
 
+			memo.as_ref()
+				.map(|memo| {
+					memo.validate().map_err(|e| {
+						log::debug!(target: "pallet_ibc", "[transfer]: memo validation error: {}", e);
+						Error::<T>::InvalidMemo
+					})
+				})
+				.transpose()?;
+
 			let msg = MsgTransfer {
 				source_port,
 				source_channel,

contracts/pallet-ibc/src/mock.rs

@@ -3,6 +3,7 @@ use crate::{
 	routing::ModuleRouter,
 };
 use cumulus_primitives_core::ParaId;
+use derive_more::Display;
 use frame_support::{
 	pallet_prelude::ConstU32,
 	parameter_types,
@@ -29,6 +30,7 @@ use sp_runtime::{
 	MultiSignature, Perbill,
 };
 use std::{
+	convert::Infallible,
 	sync::Arc,
 	time::{SystemTime, UNIX_EPOCH},
 };
@@ -45,7 +47,7 @@ pub type Balance = u128;
 pub type AccountId = <<MultiSignature as Verify>::Signer as IdentifyAccount>::AccountId;
 use super::*;
 use crate::{
-	ics20::IbcMemoHandler,
+	ics20::{IbcMemoHandler, MemoData},
 	light_clients::{AnyClientMessage, AnyConsensusState},
 };
 use ibc::mock::{client_state::MockConsensusState, header::MockClientMessage, host::MockHostBlock};
@@ -213,6 +215,43 @@ fn create_alice_key() -> <Test as Config>::AccountIdConversion {
 	IbcAccount(account_id_32)
 }
 
+#[derive(Clone, Debug, Eq, PartialEq, Default, Display, Encode, Decode, TypeInfo)]
+pub struct RawMemo(pub String);
+
+impl FromStr for RawMemo {
+	type Err = Infallible;
+
+	fn from_str(s: &str) -> Result<Self, Self::Err> {
+		Ok(Self(s.to_string()))
+	}
+}
+
+impl TryFrom<MemoData> for RawMemo {
+	type Error = <String as TryFrom<MemoData>>::Error;
+
+	fn try_from(value: MemoData) -> Result<Self, Self::Error> {
+		Ok(Self(value.try_into()?))
+	}
+}
+
+impl TryFrom<RawMemo> for MemoData {
+	type Error = <MemoData as TryFrom<String>>::Error;
+
+	fn try_from(value: RawMemo) -> Result<Self, Self::Error> {
+		Ok(value.0.try_into()?)
+	}
+}
+
+impl ValidateMemo for RawMemo {
+	fn validate(&self) -> Result<(), String> {
+		if self.0 == "invalid memo" {
+			return Err(self.0.clone())
+		} else {
+			Ok(())
+		}
+	}
+}
+
 impl Config for Test {
 	type TimeProvider = Timestamp;
 	type RuntimeEvent = RuntimeEvent;
@@ -238,7 +277,7 @@ impl Config for Test {
 	type TransferOrigin = EnsureSigned<Self::IbcAccountId>;
 	type RelayerOrigin = EnsureSigned<Self::AccountId>;
 	type HandleMemo = IbcMemoHandler<(), Test>;
-	type MemoMessage = alloc::string::String;
+	type MemoMessage = RawMemo;
 	type IsReceiveEnabled = sp_core::ConstBool<true>;
 	type IsSendEnabled = sp_core::ConstBool<true>;
 	type FeeAccount = FeeAccount;

contracts/pallet-ibc/src/tests.rs

@@ -8,7 +8,7 @@ use crate::{
 };
 use core::time::Duration;
 use frame_support::{
-	assert_ok,
+	assert_noop, assert_ok,
 	traits::{
 		fungibles::{Inspect, Mutate},
 		Currency, Hooks, Len,
@@ -265,6 +265,66 @@ fn send_transfer() {
 	})
 }
 
+#[test]
+fn send_transfer_with_invalid_memo() {
+	let mut ext = new_test_ext();
+	let balance = 100000 * MILLIS;
+	ext.execute_with(|| {
+		let pair = sp_core::sr25519::Pair::from_seed(b"12345678901234567890123456789012");
+		let ss58_address =
+			ibc_primitives::runtime_interface::account_id_to_ss58(pair.public().0, 49);
+		setup_client_and_consensus_state(PortId::transfer());
+		let asset_id =
+			<<Test as Config>::IbcDenomToAssetIdConversion as DenomToAssetId<Test>>::from_denom_to_asset_id(
+				"PICA",
+			)
+			.unwrap();
+		let _ = <<Test as Config>::NativeCurrency as Currency<
+			<Test as frame_system::Config>::AccountId,
+		>>::deposit_creating(&AccountId32::new([0; 32]), balance);
+
+		let timeout = Timeout::Offset { timestamp: Some(1000), height: Some(5) };
+
+		let ctx = Context::<Test>::default();
+		let channel_end = ctx
+			.channel_end(&(PortId::transfer(), ChannelId::new(0)))
+			.expect("expect source_channel unwrap");
+		let destination_channel = channel_end.counterparty().channel_id.unwrap();
+		Ibc::add_channels_to_feeless_channel_list(
+			RuntimeOrigin::root(),
+			0,
+			destination_channel.sequence(),
+		)
+		.expect("expect add channels to feeless list");
+
+		let result = Ibc::transfer(
+			RuntimeOrigin::signed(AccountId32::new([0; 32])),
+			TransferParams {
+				to: MultiAddress::Raw(ss58_address.as_bytes().to_vec()),
+				source_channel: 0,
+				timeout: timeout.clone(),
+			},
+			asset_id,
+			balance,
+			Some(RawMemo("{}".to_string())),
+		);
+		assert_ok!(result);
+
+		let result = Ibc::transfer(
+			RuntimeOrigin::signed(AccountId32::new([0; 32])),
+			TransferParams {
+				to: MultiAddress::Raw(ss58_address.as_bytes().to_vec()),
+				source_channel: 0,
+				timeout: timeout.clone(),
+			},
+			asset_id,
+			balance,
+			Some(RawMemo("invalid memo".to_string())),
+		);
+		assert_noop!(result, crate::Error::<Test>::InvalidMemo);
+	});
+}
+
 #[test]
 fn send_transfer_no_fee_feeless_channels() {
 	let mut ext = new_test_ext();

hyperspace/core/src/substrate/default.rs

@@ -5,6 +5,7 @@ use self::parachain_subxt::api::{
 		frame_system::{extensions::check_nonce::CheckNonce, EventRecord},
 		pallet_ibc::{events::IbcEvent as MetadataIbcEvent, TransferParams as RawTransferParams},
 		pallet_ibc_ping::SendPingParams as RawSendPingParams,
+		parachain_runtime::RawMemo,
 	},
 	sudo::calls::Sudo,
 };
@@ -118,7 +119,7 @@ define_runtime_transactions!(
 	TransferParamsWrapper,
 	SendPingParamsWrapper,
 	parachain_subxt::api::runtime_types::pallet_ibc::Any,
-	String,
+	RawMemo,
 	|x| parachain_subxt::api::tx().ibc().deliver(x),
 	|x, y, z, w| parachain_subxt::api::tx().ibc().transfer(x, y, z, w),
 	|x| parachain_subxt::api::tx().sudo().sudo(x),

utils/parachain-node/runtime/Cargo.toml

@@ -22,7 +22,8 @@ codec = { package = "parity-scale-codec", version = "3.0.0", default-features =
 hex-literal = { version = "0.3.4", optional = true }
 log = { version = "0.4.17", default-features = false }
 scale-info = { version = "2.1.1", default-features = false, features = ["derive"] }
-serde = { version = "1.0.140", optional = true, features = ["derive"] }
+serde = { version = "1.0.140", features = ["derive"], default-features = false }
+serde_json = { version = "1", default-features = false }
 smallvec = "1.9.0"
 
 # Local
@@ -97,7 +98,6 @@ std = [
 	"codec/std",
 	"log/std",
 	"scale-info/std",
-	"serde",
 	"cumulus-pallet-aura-ext/std",
 	"cumulus-pallet-dmp-queue/std",
 	"cumulus-pallet-parachain-system/std",
@@ -147,6 +147,7 @@ std = [
 	"asset-registry/std",
 	"orml-traits/std",
 	"simnode-apis/std",
+	"serde_json/std",
 	"frame-benchmarking/std",
 ]
 testing = [

utils/parachain-node/runtime/src/lib.rs

@@ -24,11 +24,13 @@ extern crate alloc;
 
 use alloc::string::String;
 use asset_registry::{AssetMetadata, DefaultAssetMetadata};
+use core::fmt::{Display, Formatter};
 
 mod weights;
 pub mod xcm_config;
 
-use codec::Encode;
+use alloc::string::ToString;
+use codec::{Decode, Encode};
 use core::str::FromStr;
 use cumulus_pallet_parachain_system::RelayNumberStrictlyIncreases;
 use ibc::core::{
@@ -82,12 +84,18 @@ pub use sp_runtime::BuildStorage;
 
 // Polkadot imports
 use polkadot_runtime_common::{BlockHashCount, SlowAdjustingFeeUpdate};
+use scale_info::TypeInfo;
+use serde::Deserialize;
+use sp_core::crypto::Infallible;
 use sp_runtime::traits::AccountIdConversion;
 
 use weights::{BlockExecutionWeight, ExtrinsicBaseWeight, RocksDbWeight};
 
 // XCM Imports
-use pallet_ibc::routing::ModuleRouter;
+use pallet_ibc::{
+	ics20::{MemoData, ValidateMemo},
+	routing::ModuleRouter,
+};
 use xcm::latest::prelude::BodyId;
 use xcm_executor::XcmExecutor;
 
@@ -717,6 +725,55 @@ fn create_alice_key() -> <Runtime as pallet_ibc::Config>::AccountIdConversion {
 	IbcAccount(account_id_32)
 }
 
+#[derive(Deserialize, Debug)]
+#[serde(rename_all = "snake_case")]
+pub enum MemoMiddlewareNamespaceChain {
+	Forward { next: Option<Box<Self>> },
+	Wasm { next: Option<Box<Self>> },
+}
+
+#[derive(Clone, Debug, Eq, PartialEq, Default, Encode, Decode, TypeInfo)]
+pub struct RawMemo(pub String);
+
+impl FromStr for RawMemo {
+	type Err = Infallible;
+
+	fn from_str(s: &str) -> Result<Self, Self::Err> {
+		Ok(Self(s.to_string()))
+	}
+}
+
+impl TryFrom<MemoData> for RawMemo {
+	type Error = <String as TryFrom<MemoData>>::Error;
+
+	fn try_from(value: MemoData) -> Result<Self, Self::Error> {
+		Ok(Self(value.try_into()?))
+	}
+}
+
+impl TryFrom<RawMemo> for MemoData {
+	type Error = <MemoData as TryFrom<String>>::Error;
+
+	fn try_from(value: RawMemo) -> Result<Self, Self::Error> {
+		Ok(value.0.try_into()?)
+	}
+}
+
+impl Display for RawMemo {
+	fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
+		write!(f, "{}", self.0)
+	}
+}
+
+impl ValidateMemo for RawMemo {
+	fn validate(&self) -> Result<(), String> {
+		// the MiddlewareNamespaceChain type contains all the supported middlewares
+		serde_json::from_str::<MemoMiddlewareNamespaceChain>(&self.0)
+			.map(|_| ())
+			.map_err(|e| e.to_string())
+	}
+}
+
 impl pallet_ibc::Config for Runtime {
 	type TimeProvider = Timestamp;
 	type RuntimeEvent = RuntimeEvent;
@@ -738,7 +795,7 @@ impl pallet_ibc::Config for Runtime {
 	type SpamProtectionDeposit = SpamProtectionDeposit;
 	type TransferOrigin = EnsureSigned<Self::IbcAccountId>;
 	type RelayerOrigin = EnsureSigned<Self::AccountId>;
-	type MemoMessage = alloc::string::String;
+	type MemoMessage = RawMemo;
 	type IsReceiveEnabled = sp_core::ConstBool<true>;
 	type IsSendEnabled = sp_core::ConstBool<true>;
 	type HandleMemo = ();