From 3d0d7e494dd426179b122fa9a5eda2b1be241106 Mon Sep 17 00:00:00 2001 From: Gabriel Facco de Arruda Date: Thu, 5 Oct 2023 13:55:10 -0300 Subject: [PATCH 1/4] RFC-34: Introduce Tinkernet XCM configs to Kusama and Asset Hub --- ...rnet-xcm-configs-to-kusama-and-assethub.md | 193 ++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 text/0034-introduce-tinkernet-xcm-configs-to-kusama-and-assethub.md diff --git a/text/0034-introduce-tinkernet-xcm-configs-to-kusama-and-assethub.md b/text/0034-introduce-tinkernet-xcm-configs-to-kusama-and-assethub.md new file mode 100644 index 000000000..9e9096147 --- /dev/null +++ b/text/0034-introduce-tinkernet-xcm-configs-to-kusama-and-assethub.md @@ -0,0 +1,193 @@ +# RFC-34: Introduce Tinkernet XCM configs to Kusama and Asset Hub + +| | | +| --------------- | ------------------------------------------------------------------------------------------- | +| **Start Date** | 05 October 2023 | +| **Description** | Introduce Tinkernet multisig XCM configs to Kusama and Asset Hub | +| **Authors** | Gabriel Facco de Arruda | + +## Summary + +This RFC proposes the introduction of Tinkernet's multisig XCM configs to both Kusama and the Kusama Asset Hub. These configs are spcifically a MultiLocation to AccountId converter and a MultiLocation to Origin converter. This enables the multisigs managed by the Tinkernet Kusama parachain to operate under Kusama and Asset Hub using their proper accounts, which are derived locally using a static derivation function with the goal to preserve the same account address across the origin and any destination chains, hence the need for the converter configs to be implemented directly in the receiver chains. + +## Motivation + +The integration of these simple XCM MultiLocation converters into Kusama and Kusama Asset Hub would allow for InvArch Tinker Network to integrate the relay and Asset Hub into the Saturn Multisig protocol, which intends to give users a unifying multisig and DAO experience that creates an intuitive user experience, one of the features that makes this possible is the shared account address across all networks, similar to how externally owned accounts would experience the many networks that compose the Kusama ecosystem of blockchains. + +## Stakeholders + +- Parachain users +- Ecosystem DAOs and communities +- Institutions managing funds across multiple chains + +## Explanation + +Saturn is a multi-chain multisig protocol built to utilize XCM for cross-chain execution, it works by defining the multisig and all of the necessary logic for operation in the Tinkernet parachain. The mulisig entities are defined at their lowest level as an integer id, which is then used to derive an AccountId locally or externally at an XCM connected chain. + +The structure of the MultiLocation origin (from the perspective of the relay) is the following: +```rust +MultiLocation { + parents: 0, + interior: Junctions::X3( + Junction::Parachain(2125), // Tinkernet ParaId in Kusama. + Junction::PalletInstance(71), // Pallet INV4, from which the multisigs originate. + Junction::GeneralIndex(index) // Index from which the multisig account is derived. + ) +} +``` + +This structure's use of an integer id is what allows the deriation of the exact same account id in the destination chains without having to rehash an account id, and since this is happening in the receiver's runtime there's also no risk of account impersonation, eliminating any trust requirements. + +These are the two XCM converters that need to be implemented in the receiver chain (Kusama in this case): +```rust +// Kusama's MultiLocation -> AccountId converters +pub type SovereignAccountOf = ( + // We can convert a child parachain using the standard `AccountId` conversion. + ChildParachainConvertsVia, + // We can directly alias an `AccountId32` into a local account. + AccountId32Aliases, + // Mapping Tinkernet multisig to the correctly derived AccountId. + TinkernetMultisigAsAccountId, +); + +// Kusama's MultiLocation -> Origin converters +type LocalOriginConverter = ( + // A `Signed` origin of the sovereign account that the original location controls. + SovereignSignedViaLocation, + // A child parachain, natively expressed, has the `Parachain` origin. + ChildParachainAsNative, + // The AccountId32 location type can be expressed natively as a `Signed` origin. + SignedAccountId32AsNative, + // A system child parachain, expressed as a Superuser, converts to the `Root` origin. + ChildSystemParachainAsSuperuser, + // Derives signed AccountId origins for Tinkernet multisigs. + TinkernetMultisigAsNativeOrigin, +); + +// Below is the definition of TinkernetMultisigAsAccountId, TinkernetMultisigAsNativeOrigin and the supporting code. + +/// Tinkernet ParaId used when matching Multisig MultiLocations. +const TINKERNET_PARA_ID: u32 = 2125; + +/// Tinkernet Multisig pallet instance used when matching Multisig MultiLocations. +const TINKERNET_MULTISIG_PALLET: u8 = 71; + +/// Constant derivation function for Tinkernet Multisigs. +/// Uses the Tinkernet genesis hash as a salt. +pub fn derive_tinkernet_multisig(id: u128) -> Result { + AccountId::decode(&mut TrailingZeroInput::new( + &( + // The constant salt used to derive Tinkernet Multisigs, this is Tinkernet's genesis + // hash. + H256([ + 212, 46, 150, 6, 169, 149, 223, 228, 51, 220, 121, 85, 220, 42, 112, 244, 149, 243, + 80, 243, 115, 218, 162, 0, 9, 138, 232, 68, 55, 129, 106, 210, + ]), + // The actual multisig integer id. + u32::try_from(id).map_err(|_| ())?, + ) + .using_encoded(blake2_256), + )) + .map_err(|_| ()) +} + +/// Convert a Tinkernet Multisig `MultiLocation` value into a local `AccountId`. +pub struct TinkernetMultisigAsAccountId(PhantomData); +impl ConvertLocation + for TinkernetMultisigAsAccountId +{ + fn convert_location(location: &MultiLocation) -> Option { + match location { + MultiLocation { + parents: 0, + interior: + X3( + Parachain(TINKERNET_PARA_ID), + PalletInstance(TINKERNET_MULTISIG_PALLET), + // Index from which the multisig account is derived. + GeneralIndex(id), + ), + } => derive_tinkernet_multisig(*id).ok(), + _ => None, + } + } +} + +/// Convert a Tinkernet Multisig `MultiLocation` value into a `Signed` origin. +pub struct TinkernetMultisigAsNativeOrigin(PhantomData); +impl ConvertOrigin + for TinkernetMultisigAsNativeOrigin +where + RuntimeOrigin::AccountId: Decode, +{ + fn convert_origin( + origin: impl Into, + kind: OriginKind, + ) -> Result { + let origin = origin.into(); + match (kind, origin) { + ( + OriginKind::Native, + MultiLocation { + parents: 0, + interior: + X3( + Junction::Parachain(TINKERNET_PARA_ID), + Junction::PalletInstance(TINKERNET_MULTISIG_PALLET), + // Index from which the multisig account is derived. + Junction::GeneralIndex(id), + ), + }, + ) => Ok(RuntimeOrigin::signed(derive_tinkernet_multisig(id).map_err(|_| origin)?)), + (_, origin) => Err(origin), + } + } +} +``` + +The PR implementing this RFC can be found [here](https://github.com/polkadot-fellows/runtimes/pull/52). + +This has been proposed and approved previously in [paritytech/polkadot PR 7165](https://github.com/paritytech/polkadot/pull/7165), however it was later reverted because the PR implemented these changes by defining the XCM types in the `xcm-builder` crate, which is intended to host generic XCM types instead of usecase specific types like these ones. As a solution, this proposal declares the types directly in the runtime, rather than using a shared library like `xcm-builder`. + +## Drawbacks + +An identified potential drawback is the definition of the types directly in the runtimes, which might introduce a small amount of bloat to the source code, but this is done to avoid using libraries like `xcm-builder` which are intended for generic types rather than usecase specific ones like this. + +## Testing, Security, and Privacy + +Since this proposal introduces two simple and static MultiLocation converters, testing can easily be done be inputting the expected MultiLocation with unique integer ids in the last junction and comparing the output account id and origin to the expected values. + +This proposal does not introduce any privacy considerations and security is not affected due to the approach of deriving the accounts locally rather than simply allowing account ids to map to themselves. + +## Performance, Ergonomics, and Compatibility + +Describe the impact of the proposal on the exposed functionality of Polkadot. + +### Performance + +This proposal should not introduce a meaningful performance overhead, as it intends to declare the new MultiLocation converters at the very end of the tuples, meaning that previously used valid XCM origins should continue to match before it ever reaches the new converters, and invalid origins will fail to match regardless. + +### Ergonomics + +The proposal enables a new valid XCM origin in the Kusama and Kusama Asset Hub runtimes, however this is an origin that comes from a parachain and is controlled by the runtime, so it doesn't affect any existing interfaces. + +### Compatibility + +As mentioned previously in the performance section, this proposal implements the new converters at the end of the tuples, which means compatibility is not affected negatively. + +## Prior Art and References + +- Saturn Multisig Kusama Integration explainer on the Polkadot Forum: https://forum.polkadot.network/t/saturn-xcm-multisig-integration-on-kusama-a-technical-discussion/2694/1 +- Fellowship runtimes repository PR implementing this RFC: https://github.com/polkadot-fellows/runtimes/pull/52 +- Previous PR in the now deprecated paritytech/polkadot repository: https://github.com/paritytech/polkadot/pull/7165 +- Higher level article about the Saturn protocol: https://invarch.medium.com/saturn-the-future-of-multi-party-ownership-ac7190f86a7b + +## Unresolved Questions + +There is one question about the current proposed implementation that might be discussed: + +Should the types be declared in a different file rather than in the `xcm_config.rs` file? + +## Future Directions and Related Material + +In the future, with proper usage of the Saturn protocol within Kusama and Kusama Asset Hub and good community feedback, a continuation of this proposal can be made to extend functionality to Polkadot and the Polkadot Asset Hub through InvArch, the Polkadot counterpart to Tinkernet. From 345f18edea12728ec49438ba104951a37acf0daf Mon Sep 17 00:00:00 2001 From: Gabriel Facco de Arruda Date: Sat, 28 Oct 2023 00:00:28 -0300 Subject: [PATCH 2/4] Rewrite RFC-34 as a generic solution --- ...rnet-xcm-configs-to-kusama-and-assethub.md | 193 ----------------- ...cm-absolute-location-account-derivation.md | 199 ++++++++++++++++++ 2 files changed, 199 insertions(+), 193 deletions(-) delete mode 100644 text/0034-introduce-tinkernet-xcm-configs-to-kusama-and-assethub.md create mode 100644 text/0034-xcm-absolute-location-account-derivation.md diff --git a/text/0034-introduce-tinkernet-xcm-configs-to-kusama-and-assethub.md b/text/0034-introduce-tinkernet-xcm-configs-to-kusama-and-assethub.md deleted file mode 100644 index 9e9096147..000000000 --- a/text/0034-introduce-tinkernet-xcm-configs-to-kusama-and-assethub.md +++ /dev/null @@ -1,193 +0,0 @@ -# RFC-34: Introduce Tinkernet XCM configs to Kusama and Asset Hub - -| | | -| --------------- | ------------------------------------------------------------------------------------------- | -| **Start Date** | 05 October 2023 | -| **Description** | Introduce Tinkernet multisig XCM configs to Kusama and Asset Hub | -| **Authors** | Gabriel Facco de Arruda | - -## Summary - -This RFC proposes the introduction of Tinkernet's multisig XCM configs to both Kusama and the Kusama Asset Hub. These configs are spcifically a MultiLocation to AccountId converter and a MultiLocation to Origin converter. This enables the multisigs managed by the Tinkernet Kusama parachain to operate under Kusama and Asset Hub using their proper accounts, which are derived locally using a static derivation function with the goal to preserve the same account address across the origin and any destination chains, hence the need for the converter configs to be implemented directly in the receiver chains. - -## Motivation - -The integration of these simple XCM MultiLocation converters into Kusama and Kusama Asset Hub would allow for InvArch Tinker Network to integrate the relay and Asset Hub into the Saturn Multisig protocol, which intends to give users a unifying multisig and DAO experience that creates an intuitive user experience, one of the features that makes this possible is the shared account address across all networks, similar to how externally owned accounts would experience the many networks that compose the Kusama ecosystem of blockchains. - -## Stakeholders - -- Parachain users -- Ecosystem DAOs and communities -- Institutions managing funds across multiple chains - -## Explanation - -Saturn is a multi-chain multisig protocol built to utilize XCM for cross-chain execution, it works by defining the multisig and all of the necessary logic for operation in the Tinkernet parachain. The mulisig entities are defined at their lowest level as an integer id, which is then used to derive an AccountId locally or externally at an XCM connected chain. - -The structure of the MultiLocation origin (from the perspective of the relay) is the following: -```rust -MultiLocation { - parents: 0, - interior: Junctions::X3( - Junction::Parachain(2125), // Tinkernet ParaId in Kusama. - Junction::PalletInstance(71), // Pallet INV4, from which the multisigs originate. - Junction::GeneralIndex(index) // Index from which the multisig account is derived. - ) -} -``` - -This structure's use of an integer id is what allows the deriation of the exact same account id in the destination chains without having to rehash an account id, and since this is happening in the receiver's runtime there's also no risk of account impersonation, eliminating any trust requirements. - -These are the two XCM converters that need to be implemented in the receiver chain (Kusama in this case): -```rust -// Kusama's MultiLocation -> AccountId converters -pub type SovereignAccountOf = ( - // We can convert a child parachain using the standard `AccountId` conversion. - ChildParachainConvertsVia, - // We can directly alias an `AccountId32` into a local account. - AccountId32Aliases, - // Mapping Tinkernet multisig to the correctly derived AccountId. - TinkernetMultisigAsAccountId, -); - -// Kusama's MultiLocation -> Origin converters -type LocalOriginConverter = ( - // A `Signed` origin of the sovereign account that the original location controls. - SovereignSignedViaLocation, - // A child parachain, natively expressed, has the `Parachain` origin. - ChildParachainAsNative, - // The AccountId32 location type can be expressed natively as a `Signed` origin. - SignedAccountId32AsNative, - // A system child parachain, expressed as a Superuser, converts to the `Root` origin. - ChildSystemParachainAsSuperuser, - // Derives signed AccountId origins for Tinkernet multisigs. - TinkernetMultisigAsNativeOrigin, -); - -// Below is the definition of TinkernetMultisigAsAccountId, TinkernetMultisigAsNativeOrigin and the supporting code. - -/// Tinkernet ParaId used when matching Multisig MultiLocations. -const TINKERNET_PARA_ID: u32 = 2125; - -/// Tinkernet Multisig pallet instance used when matching Multisig MultiLocations. -const TINKERNET_MULTISIG_PALLET: u8 = 71; - -/// Constant derivation function for Tinkernet Multisigs. -/// Uses the Tinkernet genesis hash as a salt. -pub fn derive_tinkernet_multisig(id: u128) -> Result { - AccountId::decode(&mut TrailingZeroInput::new( - &( - // The constant salt used to derive Tinkernet Multisigs, this is Tinkernet's genesis - // hash. - H256([ - 212, 46, 150, 6, 169, 149, 223, 228, 51, 220, 121, 85, 220, 42, 112, 244, 149, 243, - 80, 243, 115, 218, 162, 0, 9, 138, 232, 68, 55, 129, 106, 210, - ]), - // The actual multisig integer id. - u32::try_from(id).map_err(|_| ())?, - ) - .using_encoded(blake2_256), - )) - .map_err(|_| ()) -} - -/// Convert a Tinkernet Multisig `MultiLocation` value into a local `AccountId`. -pub struct TinkernetMultisigAsAccountId(PhantomData); -impl ConvertLocation - for TinkernetMultisigAsAccountId -{ - fn convert_location(location: &MultiLocation) -> Option { - match location { - MultiLocation { - parents: 0, - interior: - X3( - Parachain(TINKERNET_PARA_ID), - PalletInstance(TINKERNET_MULTISIG_PALLET), - // Index from which the multisig account is derived. - GeneralIndex(id), - ), - } => derive_tinkernet_multisig(*id).ok(), - _ => None, - } - } -} - -/// Convert a Tinkernet Multisig `MultiLocation` value into a `Signed` origin. -pub struct TinkernetMultisigAsNativeOrigin(PhantomData); -impl ConvertOrigin - for TinkernetMultisigAsNativeOrigin -where - RuntimeOrigin::AccountId: Decode, -{ - fn convert_origin( - origin: impl Into, - kind: OriginKind, - ) -> Result { - let origin = origin.into(); - match (kind, origin) { - ( - OriginKind::Native, - MultiLocation { - parents: 0, - interior: - X3( - Junction::Parachain(TINKERNET_PARA_ID), - Junction::PalletInstance(TINKERNET_MULTISIG_PALLET), - // Index from which the multisig account is derived. - Junction::GeneralIndex(id), - ), - }, - ) => Ok(RuntimeOrigin::signed(derive_tinkernet_multisig(id).map_err(|_| origin)?)), - (_, origin) => Err(origin), - } - } -} -``` - -The PR implementing this RFC can be found [here](https://github.com/polkadot-fellows/runtimes/pull/52). - -This has been proposed and approved previously in [paritytech/polkadot PR 7165](https://github.com/paritytech/polkadot/pull/7165), however it was later reverted because the PR implemented these changes by defining the XCM types in the `xcm-builder` crate, which is intended to host generic XCM types instead of usecase specific types like these ones. As a solution, this proposal declares the types directly in the runtime, rather than using a shared library like `xcm-builder`. - -## Drawbacks - -An identified potential drawback is the definition of the types directly in the runtimes, which might introduce a small amount of bloat to the source code, but this is done to avoid using libraries like `xcm-builder` which are intended for generic types rather than usecase specific ones like this. - -## Testing, Security, and Privacy - -Since this proposal introduces two simple and static MultiLocation converters, testing can easily be done be inputting the expected MultiLocation with unique integer ids in the last junction and comparing the output account id and origin to the expected values. - -This proposal does not introduce any privacy considerations and security is not affected due to the approach of deriving the accounts locally rather than simply allowing account ids to map to themselves. - -## Performance, Ergonomics, and Compatibility - -Describe the impact of the proposal on the exposed functionality of Polkadot. - -### Performance - -This proposal should not introduce a meaningful performance overhead, as it intends to declare the new MultiLocation converters at the very end of the tuples, meaning that previously used valid XCM origins should continue to match before it ever reaches the new converters, and invalid origins will fail to match regardless. - -### Ergonomics - -The proposal enables a new valid XCM origin in the Kusama and Kusama Asset Hub runtimes, however this is an origin that comes from a parachain and is controlled by the runtime, so it doesn't affect any existing interfaces. - -### Compatibility - -As mentioned previously in the performance section, this proposal implements the new converters at the end of the tuples, which means compatibility is not affected negatively. - -## Prior Art and References - -- Saturn Multisig Kusama Integration explainer on the Polkadot Forum: https://forum.polkadot.network/t/saturn-xcm-multisig-integration-on-kusama-a-technical-discussion/2694/1 -- Fellowship runtimes repository PR implementing this RFC: https://github.com/polkadot-fellows/runtimes/pull/52 -- Previous PR in the now deprecated paritytech/polkadot repository: https://github.com/paritytech/polkadot/pull/7165 -- Higher level article about the Saturn protocol: https://invarch.medium.com/saturn-the-future-of-multi-party-ownership-ac7190f86a7b - -## Unresolved Questions - -There is one question about the current proposed implementation that might be discussed: - -Should the types be declared in a different file rather than in the `xcm_config.rs` file? - -## Future Directions and Related Material - -In the future, with proper usage of the Saturn protocol within Kusama and Kusama Asset Hub and good community feedback, a continuation of this proposal can be made to extend functionality to Polkadot and the Polkadot Asset Hub through InvArch, the Polkadot counterpart to Tinkernet. diff --git a/text/0034-xcm-absolute-location-account-derivation.md b/text/0034-xcm-absolute-location-account-derivation.md new file mode 100644 index 000000000..c0060e53d --- /dev/null +++ b/text/0034-xcm-absolute-location-account-derivation.md @@ -0,0 +1,199 @@ +# RFC-34: XCM Absolute Location Account Derivation + +| | | +| --------------- | ------------------------------------------------------------------------------------------- | +| **Start Date** | 05 October 2023 | +| **Description** | XCM Absolute Location Account Derivation | +| **Authors** | Gabriel Facco de Arruda | + +## Summary + +This RFC proposes a change to the `WithComputedOrigin` XCM barrier and a backwards-compatible change to the `DescribeFamily` MultiLocation descriptor with the end goal of enabling the use of Universal Locations for XCM MultiLocation to AccountId conversion, which allows the use of absolute locations to maintain the same derivation result in any runtime, regardless of it's position in the family hierarchy. + +## Motivation + +These changes would allow protocol builders to leverage absolute locations to maintain the exact same derived account address across alls network in the ecosystem, thus enhancing user experience. + +One such protocol, that is the original motivation for this proposal, is InvArch's Saturn Multisig, which gives users a unifying multisig and DAO experience across all XCM connected chains. + +## Stakeholders + +- Ecosystem developers + +## Explanation + +This proposal requires the modification of two XCM types defined in the `xcm-builder` crate: The `WithComputedOrigin` barrier and the `DescribeFamily` MultiLocation descriptor. + +This proposal will go through the actual code changes that should be made, however the code provided will only serve to illustrate the goal of the changes and the actual implementation code is subject to further discussions. + +#### WithComputedOrigin + +The `WtihComputedOrigin` barrier serves as a wrapper around other barriers, consuming origin modification instructions and applying them to the message origin before passing to the inner barriers. One of the origin modifying instructions is `UniversalOrigin`, which serves the purpose of signaling that the origin should be a Universal Origin that represents the location as an absolute path with the interior prefixed by the `GlobalConsensus` junction. + +The change that needs to be made here is to remove the current relative location conversion that is made and replace it with an absolute location, represented as `{ parents: 2, interior: [GlobalConsensus(...), ...] }`. + +```diff +pub struct WithComputedOrigin( + PhantomData<(InnerBarrier, LocalUniversal, MaxPrefixes)>, +); +impl< + InnerBarrier: ShouldExecute, + LocalUniversal: Get, + MaxPrefixes: Get, + > ShouldExecute for WithComputedOrigin +{ + fn should_execute( + origin: &MultiLocation, + instructions: &mut [Instruction], + max_weight: Weight, + properties: &mut Properties, + ) -> Result<(), ProcessMessageError> { + log::trace!( + target: "xcm::barriers", + "WithComputedOrigin origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", + origin, instructions, max_weight, properties, + ); + let mut actual_origin = *origin; + let skipped = Cell::new(0usize); + // NOTE: We do not check the validity of `UniversalOrigin` here, meaning that a malicious + // origin could place a `UniversalOrigin` in order to spoof some location which gets free + // execution. This technical could get it past the barrier condition, but the execution + // would instantly fail since the first instruction would cause an error with the + // invalid UniversalOrigin. + instructions.matcher().match_next_inst_while( + |_| skipped.get() < MaxPrefixes::get() as usize, + |inst| { + match inst { + UniversalOrigin(new_global) => { +- // Note the origin is *relative to local consensus*! So we need to escape +- // local consensus with the `parents` before diving in into the +- // `universal_location`. +- actual_origin = X1(*new_global).relative_to(&LocalUniversal::get()); ++ // Grab the GlobalConsensus junction of LocalUniversal. ++ if let Ok(this_global) = LocalUniversal::get().global_consensus() { ++ // Error if requested GlobalConsensus is not this location's GlobalConsensus. ++ if *new_global != this_global.into() { ++ return Err(ProcessMessageError::Unsupported); ++ } ++ ++ // Build a location with 2 parents. ++ actual_origin = MultiLocation::grandparent() ++ // Start the interior with the GLobalConsensus junction. ++ .pushed_front_with_interior(this_global) ++ .map_err(|_| ProcessMessageError::Unsupported)? ++ // Finish the interior with the remainder. ++ .appended_with(actual_origin) ++ .map_err(|_| ProcessMessageError::Unsupported)?; ++ } + }, + DescendOrigin(j) => { + let Ok(_) = actual_origin.append_with(*j) else { + return Err(ProcessMessageError::Unsupported) + }; + }, + _ => return Ok(ControlFlow::Break(())), + }; + skipped.set(skipped.get() + 1); + Ok(ControlFlow::Continue(())) + }, + )?; + InnerBarrier::should_execute( + &actual_origin, + &mut instructions[skipped.get()..], + max_weight, + properties, + ) + } +} +``` + +#### DescribeFamily + +The `DescribeFamily` location descriptor is part of the `HashedDescription` MultiLocation hashing system and exists to describe locations in an easy format for encoding and hashing, so that an AccountId can be derived from this MultiLocation. + +The change that's needed in the `DescribeFamily` type is the inclusion of a match arm for absolute locations with the structure `{ parents: 2, interior: [GlobalConsensus(...), Parachain(...), ...] }`. + +```diff +pub struct DescribeFamily(PhantomData); +impl DescribeLocation for DescribeFamily { + fn describe_location(l: &MultiLocation) -> Option> { + match (l.parents, l.interior.first()) { + (0, Some(Parachain(index))) => { + let tail = l.interior.split_first().0; + let interior = Suffix::describe_location(&tail.into())?; + Some((b"ChildChain", Compact::::from(*index), interior).encode()) + }, + (1, Some(Parachain(index))) => { + let tail = l.interior.split_first().0; + let interior = Suffix::describe_location(&tail.into())?; + Some((b"SiblingChain", Compact::::from(*index), interior).encode()) + }, + (1, _) => { + let tail = l.interior.into(); + let interior = Suffix::describe_location(&tail)?; + Some((b"ParentChain", interior).encode()) + }, ++ // Absolute location. ++ (2, Some(GlobalConsensus(network_id))) => { ++ let tail = l.interior.split_first().0; ++ match tail.first() { ++ // Second junction is a Parachain. ++ Some(Parachain(index)) => { ++ let tail = tail.split_first().0; ++ let interior = Suffix::describe_location(&tail.into())?; ++ Some( ++ ( ++ b"GlobalConsensus", ++ *network_id, // First prefix is the GlobalConsensus. ++ b"Parachain", ++ Compact::::from(*index), // Second prefix is the parachain. ++ interior, // Suffixed with the tail. ++ ) ++ .encode(), ++ ) ++ } + _ => return None, + } + } + _ => return None, + } + } +} +``` + +## Drawbacks + +No drawbacks have been identified with this proposal. + +## Testing, Security, and Privacy + +Tests can be done using simple unit tests, as this is not a change to XCM itself but rather to types defined in `xcm-builder`. + +Security considerations should be taken with the implementation to make sure no unwanted behavior is introduced. + +This proposal does not introduce any privacy considerations. + +## Performance, Ergonomics, and Compatibility + +### Performance + +Depending on the final implementation, this proposal should not introduce much overhead to performance. + +### Ergonomics + +This proposal introduces a new path in `DescribeFamily` for absolute locations, thus allowing for more protocols to be built around XCM. The ergonomics of this change follow how the rest ofthe type is implemented. + +### Compatibility + +Backwards compatibility is unchanged for `DescribeFamily`, as changing it's implemented match arms should never happen, this proposal instead introduces a new match arm. + +The changes to the `WithComputedOrigin` barrier affect how `UniversalOrigin` computes the origin, but with the changes made to `DescribeFamily` this should have a lesser impact. + +## Prior Art and References + +- `DescirbeFamily` type: https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/xcm-builder/src/location_conversion.rs#L122 +- `WithComputedOrigin` type: https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/xcm-builder/src/barriers.rs#L153 + +## Unresolved Questions + +Implementation details and overall code is still up to discussion, the proposal suggests code to base discussions on. From a6484c4d386eee3305a2617d89f8af7423569af9 Mon Sep 17 00:00:00 2001 From: Gabriel Facco de Arruda Date: Mon, 13 Nov 2023 13:04:45 -0300 Subject: [PATCH 3/4] Update text/0034-xcm-absolute-location-account-derivation.md Co-authored-by: Keith Yeung --- text/0034-xcm-absolute-location-account-derivation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0034-xcm-absolute-location-account-derivation.md b/text/0034-xcm-absolute-location-account-derivation.md index c0060e53d..4719f180b 100644 --- a/text/0034-xcm-absolute-location-account-derivation.md +++ b/text/0034-xcm-absolute-location-account-derivation.md @@ -8,7 +8,7 @@ ## Summary -This RFC proposes a change to the `WithComputedOrigin` XCM barrier and a backwards-compatible change to the `DescribeFamily` MultiLocation descriptor with the end goal of enabling the use of Universal Locations for XCM MultiLocation to AccountId conversion, which allows the use of absolute locations to maintain the same derivation result in any runtime, regardless of it's position in the family hierarchy. +This RFC proposes a change to the `WithComputedOrigin` XCM barrier and a backwards-compatible change to the `DescribeFamily` MultiLocation descriptor with the end goal of enabling the use of Universal Locations for XCM MultiLocation to AccountId conversion, which allows the use of absolute locations to maintain the same derivation result in any runtime, regardless of its position in the family hierarchy. ## Motivation From 5829c60ce73fef1588c715653ada09cd322fd294 Mon Sep 17 00:00:00 2001 From: Gabriel Facco de Arruda Date: Wed, 6 Dec 2023 11:32:04 -0300 Subject: [PATCH 4/4] Update RFC-34 --- ...cm-absolute-location-account-derivation.md | 191 ++++++------------ 1 file changed, 57 insertions(+), 134 deletions(-) diff --git a/text/0034-xcm-absolute-location-account-derivation.md b/text/0034-xcm-absolute-location-account-derivation.md index 4719f180b..cac41d953 100644 --- a/text/0034-xcm-absolute-location-account-derivation.md +++ b/text/0034-xcm-absolute-location-account-derivation.md @@ -8,11 +8,11 @@ ## Summary -This RFC proposes a change to the `WithComputedOrigin` XCM barrier and a backwards-compatible change to the `DescribeFamily` MultiLocation descriptor with the end goal of enabling the use of Universal Locations for XCM MultiLocation to AccountId conversion, which allows the use of absolute locations to maintain the same derivation result in any runtime, regardless of its position in the family hierarchy. +This RFC proposes changes that enable the use of absolute locations in AccountId derivations, which allows protocols built using XCM to have static account derivations in any runtime, regardless of its position in the family hierarchy. ## Motivation -These changes would allow protocol builders to leverage absolute locations to maintain the exact same derived account address across alls network in the ecosystem, thus enhancing user experience. +These changes would allow protocol builders to leverage absolute locations to maintain the exact same derived account address across all networks in the ecosystem, thus enhancing user experience. One such protocol, that is the original motivation for this proposal, is InvArch's Saturn Multisig, which gives users a unifying multisig and DAO experience across all XCM connected chains. @@ -22,144 +22,69 @@ One such protocol, that is the original motivation for this proposal, is InvArch ## Explanation -This proposal requires the modification of two XCM types defined in the `xcm-builder` crate: The `WithComputedOrigin` barrier and the `DescribeFamily` MultiLocation descriptor. - -This proposal will go through the actual code changes that should be made, however the code provided will only serve to illustrate the goal of the changes and the actual implementation code is subject to further discussions. - -#### WithComputedOrigin +This proposal aims to make it possible to derive accounts for absolute locations, enabling protocols that require the ability to maintain the same derived account in any runtime. This is done by deriving accounts from the hash of described absolute locations, which are static across different destinations. -The `WtihComputedOrigin` barrier serves as a wrapper around other barriers, consuming origin modification instructions and applying them to the message origin before passing to the inner barriers. One of the origin modifying instructions is `UniversalOrigin`, which serves the purpose of signaling that the origin should be a Universal Origin that represents the location as an absolute path with the interior prefixed by the `GlobalConsensus` junction. +The same location can be represented in relative form and absolute form like so: +```rust +// Relative location (from own perspective) +{ + parents: 0, + interior: Here +} -The change that needs to be made here is to remove the current relative location conversion that is made and replace it with an absolute location, represented as `{ parents: 2, interior: [GlobalConsensus(...), ...] }`. +// Relative location (from perspective of parent) +{ + parents: 0, + interior: [Parachain(1000)] +} -```diff -pub struct WithComputedOrigin( - PhantomData<(InnerBarrier, LocalUniversal, MaxPrefixes)>, -); -impl< - InnerBarrier: ShouldExecute, - LocalUniversal: Get, - MaxPrefixes: Get, - > ShouldExecute for WithComputedOrigin +// Relative location (from perspective of sibling) { - fn should_execute( - origin: &MultiLocation, - instructions: &mut [Instruction], - max_weight: Weight, - properties: &mut Properties, - ) -> Result<(), ProcessMessageError> { - log::trace!( - target: "xcm::barriers", - "WithComputedOrigin origin: {:?}, instructions: {:?}, max_weight: {:?}, properties: {:?}", - origin, instructions, max_weight, properties, - ); - let mut actual_origin = *origin; - let skipped = Cell::new(0usize); - // NOTE: We do not check the validity of `UniversalOrigin` here, meaning that a malicious - // origin could place a `UniversalOrigin` in order to spoof some location which gets free - // execution. This technical could get it past the barrier condition, but the execution - // would instantly fail since the first instruction would cause an error with the - // invalid UniversalOrigin. - instructions.matcher().match_next_inst_while( - |_| skipped.get() < MaxPrefixes::get() as usize, - |inst| { - match inst { - UniversalOrigin(new_global) => { -- // Note the origin is *relative to local consensus*! So we need to escape -- // local consensus with the `parents` before diving in into the -- // `universal_location`. -- actual_origin = X1(*new_global).relative_to(&LocalUniversal::get()); -+ // Grab the GlobalConsensus junction of LocalUniversal. -+ if let Ok(this_global) = LocalUniversal::get().global_consensus() { -+ // Error if requested GlobalConsensus is not this location's GlobalConsensus. -+ if *new_global != this_global.into() { -+ return Err(ProcessMessageError::Unsupported); -+ } -+ -+ // Build a location with 2 parents. -+ actual_origin = MultiLocation::grandparent() -+ // Start the interior with the GLobalConsensus junction. -+ .pushed_front_with_interior(this_global) -+ .map_err(|_| ProcessMessageError::Unsupported)? -+ // Finish the interior with the remainder. -+ .appended_with(actual_origin) -+ .map_err(|_| ProcessMessageError::Unsupported)?; -+ } - }, - DescendOrigin(j) => { - let Ok(_) = actual_origin.append_with(*j) else { - return Err(ProcessMessageError::Unsupported) - }; - }, - _ => return Ok(ControlFlow::Break(())), - }; - skipped.set(skipped.get() + 1); - Ok(ControlFlow::Continue(())) - }, - )?; - InnerBarrier::should_execute( - &actual_origin, - &mut instructions[skipped.get()..], - max_weight, - properties, - ) - } + parents: 1, + interior: [Parachain(1000)] } + +// Absolute location +[GlobalConsensus(Kusama), Parachain(1000)] ``` +Using `DescribeFamily`, the above relative locations would be described like so: +```rust +// Relative location (from own perspective) +// Not possible. + +// Relative location (from perspective of parent) +(b"ChildChain", Compact::::from(*index)).encode() + +// Relative location (from perspective of sibling) +(b"SiblingChain", Compact::::from(*index)).encode() + +``` + +The proposed description for absolute location would follow the same pattern, like so: +```rust +( + b"GlobalConsensus", + network_id, + b"Parachain", + Compact::::from(para_id), + tail +).encode() +``` + +This proposal requires the modification of two XCM types defined in the `xcm-builder` crate: The `WithComputedOrigin` barrier and the `DescribeFamily` MultiLocation descriptor. + +#### WithComputedOrigin + +The `WtihComputedOrigin` barrier serves as a wrapper around other barriers, consuming origin modification instructions and applying them to the message origin before passing to the inner barriers. One of the origin modifying instructions is `UniversalOrigin`, which serves the purpose of signaling that the origin should be a Universal Origin that represents the location as an absolute path prefixed by the `GlobalConsensus` junction. + +In it's current state the barrier transforms locations with the `UniversalOrigin` instruction into relative locations, so the proposed changes aim to make it return absolute locations instead. + #### DescribeFamily The `DescribeFamily` location descriptor is part of the `HashedDescription` MultiLocation hashing system and exists to describe locations in an easy format for encoding and hashing, so that an AccountId can be derived from this MultiLocation. -The change that's needed in the `DescribeFamily` type is the inclusion of a match arm for absolute locations with the structure `{ parents: 2, interior: [GlobalConsensus(...), Parachain(...), ...] }`. - -```diff -pub struct DescribeFamily(PhantomData); -impl DescribeLocation for DescribeFamily { - fn describe_location(l: &MultiLocation) -> Option> { - match (l.parents, l.interior.first()) { - (0, Some(Parachain(index))) => { - let tail = l.interior.split_first().0; - let interior = Suffix::describe_location(&tail.into())?; - Some((b"ChildChain", Compact::::from(*index), interior).encode()) - }, - (1, Some(Parachain(index))) => { - let tail = l.interior.split_first().0; - let interior = Suffix::describe_location(&tail.into())?; - Some((b"SiblingChain", Compact::::from(*index), interior).encode()) - }, - (1, _) => { - let tail = l.interior.into(); - let interior = Suffix::describe_location(&tail)?; - Some((b"ParentChain", interior).encode()) - }, -+ // Absolute location. -+ (2, Some(GlobalConsensus(network_id))) => { -+ let tail = l.interior.split_first().0; -+ match tail.first() { -+ // Second junction is a Parachain. -+ Some(Parachain(index)) => { -+ let tail = tail.split_first().0; -+ let interior = Suffix::describe_location(&tail.into())?; -+ Some( -+ ( -+ b"GlobalConsensus", -+ *network_id, // First prefix is the GlobalConsensus. -+ b"Parachain", -+ Compact::::from(*index), // Second prefix is the parachain. -+ interior, // Suffixed with the tail. -+ ) -+ .encode(), -+ ) -+ } - _ => return None, - } - } - _ => return None, - } - } -} -``` +This implementation contains a match statement that does not match against absolute locations, so changes to it involve matching against absolute locations and providing appropriate descriptions for hashing. ## Drawbacks @@ -181,13 +106,11 @@ Depending on the final implementation, this proposal should not introduce much o ### Ergonomics -This proposal introduces a new path in `DescribeFamily` for absolute locations, thus allowing for more protocols to be built around XCM. The ergonomics of this change follow how the rest ofthe type is implemented. +The ergonomics of this proposal depend on the final implementation details. ### Compatibility -Backwards compatibility is unchanged for `DescribeFamily`, as changing it's implemented match arms should never happen, this proposal instead introduces a new match arm. - -The changes to the `WithComputedOrigin` barrier affect how `UniversalOrigin` computes the origin, but with the changes made to `DescribeFamily` this should have a lesser impact. +Backwards compatibility should remain unchanged, although that depend on the final implementation. ## Prior Art and References @@ -196,4 +119,4 @@ The changes to the `WithComputedOrigin` barrier affect how `UniversalOrigin` com ## Unresolved Questions -Implementation details and overall code is still up to discussion, the proposal suggests code to base discussions on. +Implementation details and overall code is still up to discussion.