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

Implement AliasOrigin processing in XCVM #7245

Merged
merged 32 commits into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
43044fc
Implement AliasOrigin processing in XCVM
KiChjang May 17, 2023
59b6056
add builder types and first test
vstam1 May 22, 2023
cb0ff78
switch to more general builder types
vstam1 May 22, 2023
7efcc39
clone target for RemovePrefixAccountId32
vstam1 May 22, 2023
c1c41f6
change builder types
vstam1 May 23, 2023
97f1f31
change AliasForeignAccountId32 and add test for AliasCase
vstam1 May 24, 2023
d984c95
add Aliasers type to xcm configs
vstam1 May 24, 2023
19e3123
add benchmark
vstam1 May 24, 2023
312461d
merge master
vstam1 May 24, 2023
f115cca
benchmark fix
vstam1 May 24, 2023
c7c3473
add benchmark function for runtimes
vstam1 May 24, 2023
39743b6
fix alias_origin result types
vstam1 May 24, 2023
de583b6
fix benchmark test
vstam1 May 24, 2023
7530772
add runtime-benchmarks feature in pallet-xcm-benchmarks
vstam1 May 24, 2023
f29f111
fmt
vstam1 May 24, 2023
56c7e03
remove AliasCase, add test and fmt
vstam1 May 25, 2023
e3f71ed
Merge branch 'master' into kckyeung/xcm-alias-origin
vstam1 May 25, 2023
a325ee9
address feedback
vstam1 May 30, 2023
31c12fd
merge master
vstam1 May 30, 2023
9b74bfb
Merge branch 'master' into kckyeung/xcm-alias-origin
vstam1 May 31, 2023
e2c2da2
".git/.scripts/commands/bench/bench.sh" xcm kusama pallet_xcm_benchma…
May 31, 2023
0f9bba5
".git/.scripts/commands/bench/bench.sh" xcm westend pallet_xcm_benchm…
May 31, 2023
cb82d9b
".git/.scripts/commands/bench/bench.sh" xcm rococo pallet_xcm_benchma…
May 31, 2023
26f2062
address feedback
vstam1 Jun 1, 2023
e5c910b
lock
vstam1 Jun 2, 2023
f3e4342
merge master
vstam1 Jun 2, 2023
da0c6d4
".git/.scripts/commands/bench/bench.sh" xcm kusama pallet_xcm_benchma…
Jun 2, 2023
c34c201
".git/.scripts/commands/bench/bench.sh" xcm westend pallet_xcm_benchm…
Jun 2, 2023
046c7d1
".git/.scripts/commands/bench/bench.sh" xcm rococo pallet_xcm_benchma…
Jun 2, 2023
e7d8749
Merge branch 'master' into kckyeung/xcm-alias-origin
vstam1 Jun 5, 2023
a6350e1
change doc
vstam1 Jun 5, 2023
2f2c80c
fmt
vstam1 Jun 5, 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
3 changes: 3 additions & 0 deletions xcm/xcm-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,6 @@ pub use universal_exports::{
ExporterFor, HaulBlob, HaulBlobError, HaulBlobExporter, NetworkExportTable,
SovereignPaidRemoteExporter, UnpaidLocalExporter, UnpaidRemoteExporter,
};

mod origin_aliases;
pub use origin_aliases::{AliasCase, RemovePrefixAccountId32};
46 changes: 46 additions & 0 deletions xcm/xcm-builder/src/origin_aliases.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (C) Parity Technologies (UK) Ltd.
// This file is part of Polkadot.

// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.
vstam1 marked this conversation as resolved.
Show resolved Hide resolved

use frame_support::traits::{ContainsPair, Get};
use sp_std::marker::PhantomData;
use xcm::latest::prelude::*;

pub struct RemovePrefixAccountId32<Prefix>(PhantomData<Prefix>);
vstam1 marked this conversation as resolved.
Show resolved Hide resolved
impl<Prefix: Get<MultiLocation>> ContainsPair<MultiLocation, MultiLocation>
for RemovePrefixAccountId32<Prefix>
{
fn contains(origin: &MultiLocation, target: &MultiLocation) -> bool {
if let Ok(appended) = (*target).clone().prepended_with(Prefix::get()) {
if appended == *origin {
match appended.last() {
Some(AccountId32 { .. }) => return true,
_ => return false,
}
}
}
false
}
}

pub struct AliasCase<T>(PhantomData<T>);
impl<T: Get<(MultiLocation, MultiLocation)>> ContainsPair<MultiLocation, MultiLocation>
for AliasCase<T>
{
fn contains(origin: &MultiLocation, target: &MultiLocation) -> bool {
let (o, t) = T::get();
&o == origin && &t == target
}
}
71 changes: 71 additions & 0 deletions xcm/xcm-builder/src/tests/aliases.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Copyright (C) Parity Technologies (UK) Ltd.
// This file is part of Polkadot.

// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

use super::*;

#[test]
fn alias_sibling_account_should_work() {
// Wrong parachain
assert!(!RemovePrefixAccountId32::<AliasSiblingPrefix>::contains(
&(Parent, Parachain(2), AccountId32 { network: None, id: [0; 32] }).into(),
&(AccountId32 { network: None, id: [0; 32] }).into()
));

// Accounts Differ
assert!(!RemovePrefixAccountId32::<AliasSiblingPrefix>::contains(
&(Parent, Parachain(1), AccountId32 { network: None, id: [0; 32] }).into(),
&(AccountId32 { network: None, id: [1; 32] }).into()
));

assert!(RemovePrefixAccountId32::<AliasSiblingPrefix>::contains(
&(Parent, Parachain(1), AccountId32 { network: None, id: [0; 32] }).into(),
&(AccountId32 { network: None, id: [0; 32] }).into()
));
}

#[test]
fn alias_child_account_should_work() {
// Wrong parachain
assert!(!RemovePrefixAccountId32::<AliasChildPrefix>::contains(
&(Parachain(2), AccountId32 { network: None, id: [0; 32] }).into(),
&(AccountId32 { network: None, id: [0; 32] }).into()
));

// Accounts Differ
assert!(!RemovePrefixAccountId32::<AliasChildPrefix>::contains(
&(Parachain(1), AccountId32 { network: None, id: [0; 32] }).into(),
&(AccountId32 { network: None, id: [1; 32] }).into()
));

assert!(RemovePrefixAccountId32::<AliasChildPrefix>::contains(
&(Parachain(1), AccountId32 { network: None, id: [0; 32] }).into(),
&(AccountId32 { network: None, id: [0; 32] }).into()
));
}

#[test]
fn alias_parent_account_should_work() {
// Accounts Differ
assert!(!RemovePrefixAccountId32::<AliasParentPrefix>::contains(
&(Parent, AccountId32 { network: None, id: [0; 32] }).into(),
&(AccountId32 { network: None, id: [1; 32] }).into()
));

assert!(RemovePrefixAccountId32::<AliasParentPrefix>::contains(
&(Parent, AccountId32 { network: None, id: [0; 32] }).into(),
&(AccountId32 { network: None, id: [0; 32] }).into()
));
}
13 changes: 12 additions & 1 deletion xcm/xcm-builder/src/tests/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@

use crate::{
barriers::{AllowSubscriptionsFrom, RespectSuspension},
origin_aliases::{AliasChildAccountId32, AliasParentAccountId32},
test_utils::*,
AliasSiblingAccountId32,
};
pub use crate::{
AllowExplicitUnpaidExecutionFrom, AllowKnownQueryResponses, AllowTopLevelPaidExecutionFrom,
Expand All @@ -30,7 +32,7 @@ pub use frame_support::{
},
ensure, parameter_types,
sp_runtime::DispatchErrorWithPostInfo,
traits::{Contains, Get, IsInVec},
traits::{ConstU32, Contains, Get, IsInVec},
};
pub use parity_scale_codec::{Decode, Encode};
pub use sp_io::hashing::blake2_256;
Expand Down Expand Up @@ -642,6 +644,14 @@ impl AssetExchange for TestAssetExchange {
}
}

parameter_types! {
pub static AliasSiblingPrefix: MultiLocation = (Parent, Parachain(1)).into();
pub static AliasChildPrefix: MultiLocation = (Parachain(1)).into();
pub static AliasParentPrefix: MultiLocation = (Parent).into();
}

pub type TestAliases = ();

pub struct TestConfig;
impl Config for TestConfig {
type RuntimeCall = TestCall;
Expand All @@ -667,6 +677,7 @@ impl Config for TestConfig {
type MessageExporter = TestMessageExporter;
type CallDispatcher = TestCall;
type SafeCallFilter = Everything;
type Aliasers = TestAliases;
}

pub fn fungible_multi_asset(location: MultiLocation, amount: u128) -> MultiAsset {
Expand Down
1 change: 1 addition & 0 deletions xcm/xcm-builder/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use xcm_executor::{traits::prelude::*, Config, XcmExecutor};
mod mock;
use mock::*;

mod aliases;
mod assets;
mod barriers;
mod basic;
Expand Down
1 change: 1 addition & 0 deletions xcm/xcm-builder/tests/mock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ impl xcm_executor::Config for XcmConfig {
type UniversalAliases = Nothing;
type CallDispatcher = RuntimeCall;
type SafeCallFilter = Everything;
type Aliasers = ();
}

pub type LocalOriginToLocation = SignedToAccountId32<RuntimeOrigin, AccountId, KusamaNetwork>;
Expand Down
4 changes: 4 additions & 0 deletions xcm/xcm-executor/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ pub trait Config {
/// Combinations of (Asset, Location) pairs which we trust as teleporters.
type IsTeleporter: ContainsPair<MultiAsset, MultiLocation>;

/// A list of (Origin, Target) pairs allowing a given Origin to be substituted with its
vstam1 marked this conversation as resolved.
Show resolved Hide resolved
/// corresponding Target pair.
type Aliasers: ContainsPair<MultiLocation, MultiLocation>;

/// This chain's Universal Location.
type UniversalLocation: Get<InteriorMultiLocation>;

Expand Down
10 changes: 9 additions & 1 deletion xcm/xcm-executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,15 @@ impl<Config: config::Config> XcmExecutor<Config> {
self.context.topic = None;
Ok(())
},
AliasOrigin(_) => Err(XcmError::NoPermission),
AliasOrigin(target) => {
let origin = self.origin_ref().ok_or(XcmError::BadOrigin)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

@KiChjang @vstam1

I dont see any usage of AliasOrigin now, and cannot find any restrictions or recommendation for this instruction,
but if we have ClearOrigin before AliasOrigin in XCM message, then it wont work, I dont know maybe it is intended,

just thinking, what if we change:

let origin = self.origin_ref().ok_or(XcmError::BadOrigin)?;
if Config::Aliasers::contains(origin, &target) {

to:

let effective_origin = self.context.origin.as_ref().unwrap_or(&self.original_origin);
if Config::Aliasers::contains(effective_origin, &target) {

would that be a problem or issue?

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 was thinking about that as well, but instead of using the original origin (could very easily lead to an origin escalation attack), we could also just have Aliasers::contains accept an Option<MultiLocation> as the origin ML. This will then allow custom logic to be used when the origin register is set to None.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The question with this approach now is to really ask ourselves whether AliasOrigin is a privileged operation or not. To be safe, I've erred on the side of caution and made it so, but it does not necessarily have to be the case if we document it properly, letting other chains know that they absolutely need to know how to handle the None case appropriately in their Aliasers implementation.

if Config::Aliasers::contains(origin, &target) {
self.context.origin = Some(target);
Ok(())
} else {
Err(XcmError::NoPermission)
}
},
UnpaidExecution { check_origin, .. } => {
ensure!(
check_origin.is_none() || self.context.origin == check_origin,
Expand Down