Remove BoundedVec half-impls in xcm (#6636)

* Replace sp-core dependency with more primitive crates

* Remove BoundedVec half-impls in xcm

* Fixes

* Bump bounded-collections

* Address review comments

* Bump bounded-collections

* Fix benchmarks

* Fixes

* Fixes

* cargo fmt

* Fix tests

* Update url

* Bump url to 2.3.1

* Bump anyhow

* Use bounded-collections in pallet-xcm

* Update substrate

Author: KiChjang

Cargo.lock

@@ -10,7 +10,7 @@ edition.workspace = true
 
 [dependencies]
 tokio = { version = "1.24.2", default-features = false, features = ["macros", "net", "rt-multi-thread", "sync"] }
-url = "2.0.0"
+url = "2.3.1"
 tokio-tungstenite = "0.17"
 futures-util = "0.3.23"
 lazy_static = "1.4.0"

node/zombienet-backchannel/Cargo.toml

@@ -6,12 +6,12 @@ authors.workspace = true
 edition.workspace = true
 
 [dependencies]
+bounded-collections = { version = "0.1.4", default-features = false }
 derivative = { version = "2.2.0", default-features = false, features = [ "use_core" ] }
 impl-trait-for-tuples = "0.2.2"
 log = { version = "0.4.17", default-features = false }
 parity-scale-codec = { version = "3.3.0", default-features = false, features = [ "derive", "max-encoded-len" ] }
 scale-info = { version = "2.1.2", default-features = false, features = ["derive"] }
-sp-core = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
 sp-weights = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
 serde = { version = "1.0.136", optional = true, features = ["derive"] }
 xcm-procedural = { path = "procedural" }
@@ -23,9 +23,9 @@ sp-io = { git = "https://github.com/paritytech/substrate", branch = "master" }
 default = ["std"]
 wasm-api = []
 std = [
+	"bounded-collections/std",
 	"parity-scale-codec/std",
 	"scale-info/std",
 	"serde",
-	"sp-core/std",
 	"sp-weights/std",
 ]

xcm/Cargo.toml

@@ -21,7 +21,7 @@ use frame_benchmarking::{benchmarks, BenchmarkError};
 use frame_support::dispatch::GetDispatchInfo;
 use sp_std::vec;
 use xcm::{
-	latest::{prelude::*, MaybeErrorCode, Weight},
+	latest::{prelude::*, MaxDispatchErrorLen, MaybeErrorCode, Weight},
 	DoubleEncoded,
 };
 use xcm_executor::{ExecutorError, FeesMode};
@@ -360,9 +360,9 @@ benchmarks! {
 
 	expect_transact_status {
 		let mut executor = new_executor::<T>(Default::default());
-		// 1024 is an overestimate but should be good enough until we have `max_encoded_len`.
-		// Eventually it should be replaced by `DispatchError::max_encoded_len()`.
-		let worst_error = || MaybeErrorCode::Error(vec![0; 1024]);
+		let worst_error = || -> MaybeErrorCode {
+			vec![0; MaxDispatchErrorLen::get() as usize].into()
+		};
 		executor.set_transact_status(worst_error());
 
 		let instruction = Instruction::ExpectTransactStatus(worst_error());
@@ -430,7 +430,7 @@ benchmarks! {
 
 	clear_transact_status {
 		let mut executor = new_executor::<T>(Default::default());
-		executor.set_transact_status(MaybeErrorCode::Error(b"MyError".to_vec()));
+		executor.set_transact_status(b"MyError".to_vec().into());
 
 		let instruction = Instruction::ClearTransactStatus;
 		let xcm = Xcm(vec![instruction]);

xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs

@@ -6,6 +6,7 @@ version.workspace = true
 
 
 [dependencies]
+bounded-collections = { version = "0.1.4", default-features = false }
 codec = { package = "parity-scale-codec", version = "3.3.0", default-features = false, features = ["derive"] }
 scale-info = { version = "2.1.2", default-features = false, features = ["derive"] }
 serde = { version = "1.0.137", optional = true, features = ["derive"] }
@@ -31,6 +32,7 @@ xcm-builder = { path = "../xcm-builder" }
 [features]
 default = ["std"]
 std = [
+	"bounded-collections/std",
 	"codec/std",
 	"scale-info/std",
 	"serde",

xcm/pallet-xcm/Cargo.toml

@@ -15,10 +15,10 @@
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 use super::*;
+use bounded_collections::{ConstU32, WeakBoundedVec};
 use frame_benchmarking::{benchmarks, BenchmarkError, BenchmarkResult};
 use frame_support::weights::Weight;
 use frame_system::RawOrigin;
-use sp_core::{bounded::WeakBoundedVec, ConstU32};
 use sp_std::prelude::*;
 use xcm::{latest::prelude::*, v2};
 

xcm/pallet-xcm/src/benchmarking.rs

@@ -18,9 +18,9 @@
 
 use super::{BodyId, BodyPart, Junctions, MultiLocation, NetworkId};
 use crate::v3::Junction as NewJunction;
+use bounded_collections::{ConstU32, WeakBoundedVec};
 use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
 use scale_info::TypeInfo;
-use sp_core::{bounded::WeakBoundedVec, ConstU32};
 
 /// A single item in a path to describe the relative location of a consensus system.
 ///

xcm/src/v2/junction.rs

@@ -59,11 +59,11 @@ use super::{
 	DoubleEncoded, GetWeight,
 };
 use alloc::{vec, vec::Vec};
+use bounded_collections::{ConstU32, WeakBoundedVec};
 use core::{fmt::Debug, result};
 use derivative::Derivative;
 use parity_scale_codec::{self, Decode, Encode, MaxEncodedLen};
 use scale_info::TypeInfo;
-use sp_core::{bounded::WeakBoundedVec, ConstU32};
 
 mod junction;
 mod multiasset;

xcm/src/v2/mod.rs

@@ -22,6 +22,7 @@ use super::v2::{
 };
 use crate::{DoubleEncoded, GetWeight};
 use alloc::{vec, vec::Vec};
+use bounded_collections::{parameter_types, BoundedVec};
 use core::{
 	convert::{TryFrom, TryInto},
 	fmt::Debug,
@@ -200,14 +201,20 @@ pub mod prelude {
 	}
 }
 
-#[derive(Clone, Eq, PartialEq, Encode, Decode, Debug, TypeInfo)]
+parameter_types! {
+	pub MaxPalletNameLen: u32 = 48;
+	/// Maximum size of the encoded error code coming from a `Dispatch` result, used for
+	/// `MaybeErrorCode`. This is not (yet) enforced, so it's just an indication of expectation.
+	pub MaxDispatchErrorLen: u32 = 128;
+	pub MaxPalletsInfo: u32 = 64;
+}
+
+#[derive(Clone, Eq, PartialEq, Encode, Decode, Debug, TypeInfo, MaxEncodedLen)]
 pub struct PalletInfo {
 	#[codec(compact)]
 	index: u32,
-	// TODO: Change to `BoundedVec` so `MaxEncodedLen` derive will work.
-	name: Vec<u8>,
-	// TODO: Change to `BoundedVec` so `MaxEncodedLen` derive will work.
-	module_name: Vec<u8>,
+	name: BoundedVec<u8, MaxPalletNameLen>,
+	module_name: BoundedVec<u8, MaxPalletNameLen>,
 	#[codec(compact)]
 	major: u32,
 	#[codec(compact)]
@@ -216,8 +223,6 @@ pub struct PalletInfo {
 	patch: u32,
 }
 
-const MAX_NAME_LEN: usize = 48;
-
 impl PalletInfo {
 	pub fn new(
 		index: u32,
@@ -227,44 +232,25 @@ impl PalletInfo {
 		minor: u32,
 		patch: u32,
 	) -> result::Result<Self, Error> {
-		if name.len() > MAX_NAME_LEN || module_name.len() > MAX_NAME_LEN {
-			return Err(Error::Overflow)
-		}
-		Ok(Self { index, name, module_name, major, minor, patch })
-	}
-}
+		let name = BoundedVec::try_from(name).map_err(|_| Error::Overflow)?;
+		let module_name = BoundedVec::try_from(module_name).map_err(|_| Error::Overflow)?;
 
-impl MaxEncodedLen for PalletInfo {
-	fn max_encoded_len() -> usize {
-		parity_scale_codec::Compact::<u32>::max_encoded_len() * 4 + (MAX_NAME_LEN + 1) * 2
+		Ok(Self { index, name, module_name, major, minor, patch })
 	}
 }
 
-#[derive(Clone, Eq, PartialEq, Encode, Decode, Debug, TypeInfo)]
+#[derive(Clone, Eq, PartialEq, Encode, Decode, Debug, TypeInfo, MaxEncodedLen)]
 pub enum MaybeErrorCode {
 	Success,
-	// TODO: Change to a `BoundedVec` so that deriving `MaxEncodedLen` works.
-	Error(Vec<u8>),
-	TruncatedError(Vec<u8>),
-}
-
-/// Maximum size of the encoded error code coming from a `Dispatch` result, used for
-/// `MaybeErrorCode`. This is not (yet) enforced, so it's just an indication of expectation.
-const MAX_DISPATCH_ERROR_LEN: usize = 128;
-
-impl MaxEncodedLen for MaybeErrorCode {
-	fn max_encoded_len() -> usize {
-		MAX_DISPATCH_ERROR_LEN + 3
-	}
+	Error(BoundedVec<u8, MaxDispatchErrorLen>),
+	TruncatedError(BoundedVec<u8, MaxDispatchErrorLen>),
 }
 
 impl From<Vec<u8>> for MaybeErrorCode {
-	fn from(mut v: Vec<u8>) -> Self {
-		if v.len() <= MAX_DISPATCH_ERROR_LEN {
-			MaybeErrorCode::Error(v)
-		} else {
-			v.truncate(MAX_DISPATCH_ERROR_LEN);
-			MaybeErrorCode::TruncatedError(v)
+	fn from(v: Vec<u8>) -> Self {
+		match BoundedVec::try_from(v) {
+			Ok(error) => MaybeErrorCode::Error(error),
+			Err(error) => MaybeErrorCode::TruncatedError(BoundedVec::truncate_from(error)),
 		}
 	}
 }
@@ -275,26 +261,6 @@ impl Default for MaybeErrorCode {
 	}
 }
 
-/// Maximum number of pallets which we expect to be returned in `PalletsInfo`.
-const MAX_PALLETS_INFO_LEN: usize = 64;
-
-#[derive(Clone, Eq, PartialEq, Encode, Decode, Debug, TypeInfo)]
-pub struct VecPalletInfo(Vec<PalletInfo>);
-impl TryFrom<Vec<PalletInfo>> for VecPalletInfo {
-	type Error = Error;
-	fn try_from(v: Vec<PalletInfo>) -> result::Result<Self, Error> {
-		if v.len() > MAX_PALLETS_INFO_LEN {
-			return Err(Error::Overflow)
-		}
-		Ok(VecPalletInfo(v))
-	}
-}
-impl MaxEncodedLen for VecPalletInfo {
-	fn max_encoded_len() -> usize {
-		PalletInfo::max_encoded_len() * MAX_PALLETS_INFO_LEN
-	}
-}
-
 /// Response data to a query.
 #[derive(Clone, Eq, PartialEq, Encode, Decode, Debug, TypeInfo, MaxEncodedLen)]
 pub enum Response {
@@ -307,8 +273,7 @@ pub enum Response {
 	/// An XCM version.
 	Version(super::Version),
 	/// The index, instance name, pallet name and version of some pallets.
-	// TODO: Change to a `BoundedVec` so that deriving `MaxEncodedLen` works.
-	PalletsInfo(VecPalletInfo),
+	PalletsInfo(BoundedVec<PalletInfo, MaxPalletsInfo>),
 	/// The status of a dispatch attempt using `Transact`.
 	DispatchResult(MaybeErrorCode),
 }

xcm/src/v3/mod.rs

@@ -24,7 +24,7 @@ log = { version = "0.4.17", default-features = false }
 polkadot-parachain = { path = "../../parachain", default-features = false }
 
 [dev-dependencies]
-sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
+primitive-types = "0.12.1"
 pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "master" }
 pallet-xcm = { path = "../pallet-xcm" }
 polkadot-runtime-parachains = { path = "../../runtime/parachains" }

xcm/xcm-builder/Cargo.toml

@@ -147,7 +147,7 @@ fn report_failed_transact_status_should_work() {
 	let r = XcmExecutor::<TestConfig>::execute_xcm(Parent, message, hash, weight_limit);
 	assert_eq!(r, Outcome::Complete(Weight::from_parts(70, 70)));
 	let expected_msg = Xcm(vec![QueryResponse {
-		response: Response::DispatchResult(MaybeErrorCode::Error(vec![2])),
+		response: Response::DispatchResult(vec![2].into()),
 		query_id: 42,
 		max_weight: Weight::from_parts(5000, 5000),
 		querier: Some(Here.into()),
@@ -197,7 +197,7 @@ fn expect_failed_transact_status_should_work() {
 			require_weight_at_most: Weight::from_parts(50, 50),
 			call: TestCall::OnlyRoot(Weight::from_parts(50, 50), None).encode().into(),
 		},
-		ExpectTransactStatus(MaybeErrorCode::Error(vec![2])),
+		ExpectTransactStatus(vec![2].into()),
 	]);
 	let hash = fake_message_hash(&message);
 	let weight_limit = Weight::from_parts(70, 70);
@@ -210,7 +210,7 @@ fn expect_failed_transact_status_should_work() {
 			require_weight_at_most: Weight::from_parts(50, 50),
 			call: TestCall::Any(Weight::from_parts(50, 50), None).encode().into(),
 		},
-		ExpectTransactStatus(MaybeErrorCode::Error(vec![2])),
+		ExpectTransactStatus(vec![2].into()),
 	]);
 	let hash = fake_message_hash(&message);
 	let weight_limit = Weight::from_parts(70, 70);

xcm/xcm-builder/src/tests/transacting.rs

@@ -20,7 +20,7 @@ use frame_support::{
 	weights::Weight,
 };
 use parity_scale_codec::Encode;
-use sp_core::H256;
+use primitive_types::H256;
 use sp_runtime::{testing::Header, traits::IdentityLookup, AccountId32};
 use sp_std::cell::RefCell;
 

xcm/xcm-builder/tests/mock/mod.rs

@@ -773,7 +773,8 @@ impl<Config: config::Config> XcmExecutor<Config> {
 					})
 					.collect::<Result<Vec<_>, XcmError>>()?;
 				let QueryResponseInfo { destination, query_id, max_weight } = response_info;
-				let response = Response::PalletsInfo(pallets.try_into()?);
+				let response =
+					Response::PalletsInfo(pallets.try_into().map_err(|_| XcmError::Overflow)?);
 				let querier = Self::to_querier(self.cloned_origin(), &destination)?;
 				let instruction = QueryResponse { query_id, response, max_weight, querier };
 				let message = Xcm(vec![instruction]);