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.

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

WithMaxSize #4188

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 14 additions & 17 deletions polkadot/xcm/pallet-xcm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ use sp_runtime::{
use sp_std::{boxed::Box, marker::PhantomData, prelude::*, result::Result, vec};
use xcm::{latest::QueryResponseInfo, prelude::*};
use xcm_builder::{
ExecuteController, ExecuteControllerWeightInfo, MaxXcmEncodedSize, QueryController,
QueryControllerWeightInfo, SendController, SendControllerWeightInfo,
ExecuteController, ExecuteControllerWeightInfo, MaxXcmEncodedSize, MaxXcmEncodedSizeUsize,
QueryController, QueryControllerWeightInfo, SendController, SendControllerWeightInfo,
};
use xcm_executor::{
traits::{
Expand Down Expand Up @@ -188,7 +188,7 @@ pub mod pallet {
use super::*;
use frame_support::{
dispatch::{GetDispatchInfo, PostDispatchInfo},
parameter_types,
parameter_types, WithMaxSize,
};
use frame_system::Config as SysConfig;
use sp_core::H256;
Expand Down Expand Up @@ -945,17 +945,18 @@ pub mod pallet {

#[pallet::call(weight(<T as Config>::WeightInfo))]
impl<T: Config> Pallet<T> {
/// WARNING: DEPRECATED. `send` will be removed after June 2024. Use `send_blob` instead.
#[allow(deprecated)]
#[deprecated(note = "`send` will be removed after June 2024. Use `send_blob` instead.")]
/// Send an XCM from a local, signed, origin.
///
/// The destination, `dest`, will receive this message with a `DescendOrigin` instruction
/// that makes the origin of the message be the origin on this system.
#[pallet::call_index(0)]
pub fn send(
origin: OriginFor<T>,
dest: Box<VersionedLocation>,
message: Box<VersionedXcm<()>>,
message: Box<WithMaxSize<VersionedXcm<()>, MaxXcmEncodedSizeUsize>>,
) -> DispatchResult {
let origin_location = T::SendXcmOrigin::ensure_origin(origin)?;
Self::send_base(origin_location, dest, message)?;
Self::send_base(origin_location, dest, Box::new(message.value()))?;
Ok(())
}

Expand Down Expand Up @@ -1052,22 +1053,18 @@ pub mod pallet {
/// No more than `max_weight` will be used in its attempted execution. If this is less than
/// the maximum amount of weight that the message could take to be executed, then no
/// execution attempt will be made.
///
/// WARNING: DEPRECATED. `execute` will be removed after June 2024. Use `execute_blob`
/// instead.
#[allow(deprecated)]
#[deprecated(
note = "`execute` will be removed after June 2024. Use `execute_blob` instead."
)]
#[pallet::call_index(3)]
#[pallet::weight(max_weight.saturating_add(T::WeightInfo::execute()))]
pub fn execute(
origin: OriginFor<T>,
message: Box<VersionedXcm<<T as Config>::RuntimeCall>>,
message: Box<
WithMaxSize<VersionedXcm<<T as Config>::RuntimeCall>, MaxXcmEncodedSizeUsize>,
>,
max_weight: Weight,
) -> DispatchResultWithPostInfo {
let origin_location = T::ExecuteXcmOrigin::ensure_origin(origin)?;
let weight_used = Self::execute_base(origin_location, message, max_weight)?;
let weight_used =
Self::execute_base(origin_location, Box::new(message.value()), max_weight)?;
Ok(Some(weight_used.saturating_add(T::WeightInfo::execute())).into())
}

Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/xcm-builder/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub trait ExecuteControllerWeightInfo {

parameter_types! {
pub const MaxXcmEncodedSize: u32 = xcm::MAX_XCM_ENCODED_SIZE;
pub const MaxXcmEncodedSizeUsize: usize = xcm::MAX_XCM_ENCODED_SIZE as usize;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what's the best way to handle this

}

/// Execute an XCM locally, for a given origin.
Expand Down
5 changes: 3 additions & 2 deletions polkadot/xcm/xcm-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ pub use barriers::{

mod controller;
pub use controller::{
Controller, ExecuteController, ExecuteControllerWeightInfo, MaxXcmEncodedSize, QueryController,
QueryControllerWeightInfo, QueryHandler, SendController, SendControllerWeightInfo,
Controller, ExecuteController, ExecuteControllerWeightInfo, MaxXcmEncodedSize,
MaxXcmEncodedSizeUsize, QueryController, QueryControllerWeightInfo, QueryHandler,
SendController, SendControllerWeightInfo,
};

mod currency_adapter;
Expand Down
15 changes: 8 additions & 7 deletions polkadot/xcm/xcm-executor/integration-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#![cfg(test)]

use codec::Encode;
use frame_support::{dispatch::GetDispatchInfo, weights::Weight};
use frame_support::{dispatch::GetDispatchInfo, weights::Weight, WithMaxSize};
use polkadot_service::chain_spec::get_account_id_from_seed;
use polkadot_test_client::{
BlockBuilderExt, ClientBlockImportExt, DefaultTestClientBuilderExt, InitPolkadotBlockBuilder,
Expand Down Expand Up @@ -47,7 +47,7 @@ fn basic_buy_fees_message_executes() {
let execute = construct_extrinsic(
&client,
polkadot_test_runtime::RuntimeCall::Xcm(pallet_xcm::Call::execute {
message: Box::new(VersionedXcm::from(msg)),
message: Box::new(WithMaxSize::new(VersionedXcm::from(msg)).expect("XCM is valid")),
max_weight: Weight::from_parts(1_000_000_000, 1024 * 1024),
}),
sp_keyring::Sr25519Keyring::Alice,
Expand Down Expand Up @@ -122,7 +122,9 @@ fn transact_recursion_limit_works() {
}
let max_weight = <XcmConfig as xcm_executor::Config>::Weigher::weight(&mut msg).unwrap();
call = Some(polkadot_test_runtime::RuntimeCall::Xcm(pallet_xcm::Call::execute {
message: Box::new(VersionedXcm::from(msg.clone())),
message: Box::new(
WithMaxSize::new(VersionedXcm::from(msg.clone())).expect("XCM is valid"),
),
max_weight,
}));
}
Expand Down Expand Up @@ -212,12 +214,11 @@ fn query_response_fires() {
let max_weight = Weight::from_parts(1_000_000, 1024 * 1024);
let querier = Some(Here.into());
let msg = Xcm(vec![QueryResponse { query_id, response, max_weight, querier }]);
let msg = Box::new(VersionedXcm::from(msg));

let execute = construct_extrinsic(
&client,
polkadot_test_runtime::RuntimeCall::Xcm(pallet_xcm::Call::execute {
message: msg,
message: Box::new(WithMaxSize::new(VersionedXcm::from(msg)).expect("XCM is valid")),
max_weight: Weight::from_parts(1_000_000_000, 1024 * 1024),
}),
sp_keyring::Sr25519Keyring::Alice,
Expand Down Expand Up @@ -298,7 +299,7 @@ fn query_response_elicits_handler() {
let execute = construct_extrinsic(
&client,
polkadot_test_runtime::RuntimeCall::Xcm(pallet_xcm::Call::execute {
message: Box::new(VersionedXcm::from(msg)),
message: Box::new(WithMaxSize::new(VersionedXcm::from(msg)).expect("XCM is valid")),
max_weight: Weight::from_parts(1_000_000_000, 1024 * 1024),
}),
sp_keyring::Sr25519Keyring::Alice,
Expand Down Expand Up @@ -381,7 +382,7 @@ fn deposit_reserve_asset_works_for_any_xcm_sender() {
let execute = construct_extrinsic(
&client,
polkadot_test_runtime::RuntimeCall::Xcm(pallet_xcm::Call::execute {
message: Box::new(VersionedXcm::from(msg)),
message: Box::new(WithMaxSize::new(VersionedXcm::from(msg)).expect("XCM is valid")),
max_weight: Weight::from_parts(1_000_000_000, 1024 * 1024),
}),
sp_keyring::Sr25519Keyring::Alice,
Expand Down
2 changes: 2 additions & 0 deletions substrate/frame/support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ pub mod storage;
#[cfg(test)]
mod tests;
pub mod traits;
pub mod utils;
pub mod weights;
#[doc(hidden)]
pub mod unsigned {
Expand Down Expand Up @@ -103,6 +104,7 @@ pub use self::{
IterableStorageDoubleMap, IterableStorageMap, IterableStorageNMap, StorageDoubleMap,
StorageMap, StorageNMap, StoragePrefixedMap, StorageValue,
},
utils::WithMaxSize,
};
pub use sp_runtime::{
self, print, traits::Printable, ConsensusEngineId, MAX_MODULE_ERROR_ENCODED_SIZE,
Expand Down
99 changes: 99 additions & 0 deletions substrate/frame/support/src/utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// This file is part of Substrate.

// Copyright (C) Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: Apache-2.0

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use crate::Parameter;
use codec::{Decode, Encode, Input, MaxEncodedLen};
use frame_support_procedural::{CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound};
use scale_info::TypeInfo;
use sp_core::Get;

/// TODO: docs
pub struct InputWithMaxLength<'a, I: Input, S: Get<usize>> {
input: &'a mut I,
remaining_len: usize,
_phantom: core::marker::PhantomData<S>,
}

impl<'a, I: Input, S: Get<usize>> InputWithMaxLength<'a, I, S> {
pub fn new(input: &'a mut I) -> Self {
Self { input, remaining_len: S::get(), _phantom: Default::default() }
}
}

impl<'a, I: Input, S: Get<usize>> Input for InputWithMaxLength<'a, I, S> {
fn remaining_len(&mut self) -> Result<Option<usize>, codec::Error> {
self.input.remaining_len().map(|l| l.map(|l| l.min(self.remaining_len)))
}

fn read(&mut self, into: &mut [u8]) -> Result<(), codec::Error> {
if into.len() > self.remaining_len {
return Err("Not enough data to fill buffer".into());
}
self.input.read(into)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.input.read(into)
let res = self.input.read(into)?;
self.remaining_len -= into.len();
Ok(res)

We actually need to remove it from the remaining length :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really need some tests XD
and do we need to remove it before the read or it wouldn’t matter?

Copy link
Member

Choose a reason for hiding this comment

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

I thought about it. If this returns an error, I would assume that we bubble that up any way and the decoding failed.

}
}

/// TODO: docs
#[derive(Encode, EqNoBound, PartialEqNoBound, CloneNoBound, RuntimeDebugNoBound)]
pub struct WithMaxSize<T: Parameter + 'static, S: Get<usize>> {
value: T,
_phantom: core::marker::PhantomData<S>,
}

impl<T: Parameter + 'static, S: Get<usize>> WithMaxSize<T, S> {
/// TODO: docs
pub fn unchecked_new(value: T) -> Self {
Self { value, _phantom: Default::default() }
}

/// TODO: docs
pub fn new(value: T) -> Result<Self, ()> {
if value.encoded_size() <= S::get() {
Ok(Self { value, _phantom: Default::default() })
} else {
Err(())
}
}

pub fn value(self) -> T {
self.value
Copy link
Member

Choose a reason for hiding this comment

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

Could make sense to impl the Deref trait and mark with WrapperTypeEncode and WrapperTypeDecode.

}
}

impl<T: Parameter + 'static, S: Get<usize>> MaxEncodedLen for WithMaxSize<T, S> {
fn max_encoded_len() -> usize {
// not using T::max_encoded_len().min(S::get()) because while it is possible
// that T::max_encoded_len() is smaller, but in that case there will be no reason
// to use WithMaxSize
S::get()
}
}

impl<T: Parameter + 'static, S: Get<usize>> Decode for WithMaxSize<T, S> {
fn decode<I: Input>(input: &mut I) -> Result<Self, codec::Error> {
let mut input = InputWithMaxLength::<I, S>::new(input);
Ok(Self::unchecked_new(T::decode(&mut input)?))
}
}

impl<T: Parameter + 'static, S: Get<usize>> TypeInfo for WithMaxSize<T, S> {
type Identity = T;

fn type_info() -> scale_info::Type {
T::type_info()
}
}
Loading