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

Cross chain pure proxy replication #5106

Closed
wants to merge 2 commits into from
Closed

Conversation

muharem
Copy link
Contributor

@muharem muharem commented Jul 22, 2024

Attempt to fix #3288

This PR explores a method to replicate a pure account and its proxy relations from the chain where the pure account was created to some remote chain.

Pure accounts are keyless accounts (only a public key is known) that are created with a proxy account to control them. The same pure account cannot be created on another chain, so to replicate the same relationship, it should be copied via XCM.

The solution demonstrated in the PR is based on alias origins (the AliasOrigin XCM instruction). It establishes a relationship where any account such as Location{ parent: {a}, X2(Parachain({b}), AccountId32({c})) } from an authorized chain has its alias on the authorizing chain with the same account ID: Location{ parent: 0, X1(AccountId32({c})) }.

Refer to cross_chain_pure_proxy test for example and SystemAccountId32Aliases type for alias configuration example.

Con: the authorizing chain assumes risks if the authorized chain is compromised.

Alternative solution allowing replication only for pure proxy

An extension pallet for the proxy pallet sends an XCM program to a remote chain on behalf of itself to replicate a pure proxy. This pallet will be authorized by the same proxy extension pallet on a receiving chain to perform replication. To replicate a pure proxy a caller must provide a witness data (preimage of the pure account: spawner, block, ext index, index) and the pure proxy must exist within original proxy pallet. The receiving chain can check the witness data and accept the replication of the pure proxy (with same account ids as on the source chain).

Con: users/clients have to provide a witness data

@franciscoaguirre
Copy link
Contributor

Aliasing an account from another chain to the same account on the local chain can be a security issue, since if the other chain is compromised the impact is not only limited to that chain. See the discussion here.

I think it might make sense to do this on the system chains since if one is compromised all of Polkadot is compromised? But still I wouldn't open this pattern up to the community.

@@ -349,6 +349,27 @@ pub type TrustedTeleporters = (
IsForeignConcreteAsset<FromSiblingParachain<parachain_info::Pallet<Runtime>>>,
);

pub struct SystemAccountId32Aliases;
impl ContainsPair<Location, Location> for SystemAccountId32Aliases {
fn contains(original: &Location, target: &Location) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why original and not origin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they both represent origins. but no strong opinion, I will rename

Comment on lines +356 to +360
let original_id = match original.unpack() {
(1, [Parachain(1001), AccountId32 { network: None, id }]) => id,
(1, [Parachain(1001), AccountId32 { network, id }])
if *network == RelayNetwork::get() =>
id,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite follow this. Does this mean it only recognizes origins from Parachain(1001) as the source? So each chain would have to configure all the chains it recognizes as source chains? Makes sense, just want to make sure I understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, relation where an origin of location { parent: 1, [Parachain(%para_id%), AccountId32(%id%)]} has it's alias as location { parent: 0, [AccountId32(%id%)]} is true only for authorised list of %para_id%s
This was implemented only for demonstration purpose, if we move forward with the solution, I probably move this to xcm builder and make it accept the list of para ids.

@joepetrowski
Copy link
Contributor

Aliasing an account from another chain to the same account on the local chain can be a security issue, since if the other chain is compromised the impact is not only limited to that chain. See the discussion here.

Or perhaps we limit it somehow to the creation of proxy relationships and not allow generic execution?

I think it might make sense to do this on the system chains since if one is compromised all of Polkadot is compromised? But still I wouldn't open this pattern up to the community.

Each chain should be able to opt in to the other chains it recognizes for this. So yeah system chains could all recognize each other, and a non-system chain could choose to recognize all system chains plus maybe a few select others where they want to accept proxy relationships from.

@xlc
Copy link
Contributor

xlc commented Jul 23, 2024

pure proxy are generate from a hash of a payload. I will be more willing to accept a replication of a pure proxy provided that it is a hash of a preimage that can be validated. it also eliminates some attack scenario (e.g. take control of a system account that is not yet enabled) as it will require a hash collision, which shouldn't be possible

@@ -349,6 +349,27 @@ pub type TrustedTeleporters = (
IsForeignConcreteAsset<FromSiblingParachain<parachain_info::Pallet<Runtime>>>,
);

pub struct SystemAccountId32Aliases;
impl ContainsPair<Location, Location> for SystemAccountId32Aliases {
fn contains(original: &Location, target: &Location) -> bool {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

they both represent origins. but no strong opinion, I will rename

Comment on lines +356 to +360
let original_id = match original.unpack() {
(1, [Parachain(1001), AccountId32 { network: None, id }]) => id,
(1, [Parachain(1001), AccountId32 { network, id }])
if *network == RelayNetwork::get() =>
id,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, relation where an origin of location { parent: 1, [Parachain(%para_id%), AccountId32(%id%)]} has it's alias as location { parent: 0, [AccountId32(%id%)]} is true only for authorised list of %para_id%s
This was implemented only for demonstration purpose, if we move forward with the solution, I probably move this to xcm builder and make it accept the list of para ids.

@muharem
Copy link
Contributor Author

muharem commented Jul 23, 2024

pure proxy are generate from a hash of a payload. I will be more willing to accept a replication of a pure proxy provided that it is a hash of a preimage that can be validated. it also eliminates some attack scenario (e.g. take control of a system account that is not yet enabled) as it will require a hash collision, which shouldn't be possible

@xlc something like:

An extension pallet for the proxy pallet sends an XCM program to a remote chain on behalf of itself to replicate a pure proxy. This pallet will be authorized by the same proxy extension pallet on a receiving chain to perform replication. To replicate a pure proxy a caller must provide a witness data (preimage of the pure account: spawner, block, ext index, index) and the pure proxy must exist within original proxy pallet. The receiving chain can check the witness data and accept the replication of the pure proxy (with same account ids as on the source chain).

Would not be an issue to provide an original preimage for a user/client?

@muharem
Copy link
Contributor Author

muharem commented Jul 23, 2024

Or perhaps we limit it somehow to the creation of proxy relationships and not allow generic execution?

@joepetrowski we cannot really do it, after a proxy is created you can execute anything. I think in principal, we allow a replication of any proxy (current PR) or only pure proxy (alternative solution from the comment above).

I now prefer the alternative solution because it can be adopted as a pattern and is associated with fewer security risks.

@muharem muharem marked this pull request as draft July 23, 2024 16:12
@xlc
Copy link
Contributor

xlc commented Jul 24, 2024

The only downside is that the dapp have to use a block indexer/explorer to fetch the witness data as they are not stored onchain. But that shouldn't be a real issue.

@burdges
Copy link

burdges commented Jul 24, 2024

In future, we could explore a purely off-chain design pattern here..

At the parachain level, our sending/origin chain A could (a) provide some mechanism by which users request state proofs, or block data proofs, and (b) define a pure function for recognizing that state proof type. We call the parablock hash for these proofs the parachain parent.

At the relay chain level, we provide proof updater tooling that extends these proofs first from the parachain parent to its relay chain inclusion block, and then from the relay chain inclusion block to some desired relay parent using the realy chain MMR/MMB.

At this point, the user's agent aka UX code requests a state proof from a sending/origin chain full node, extends this to the inclusion parachain block using a full node of the relay chain, and then submits this as a transaction to the recieving/target chain collator, who being a relay chain full node themselves, completes this to its currently desired relay parent.

In this way, the user's transaction proves not their exact current state on the sending/oirigin chain, but their recent past state, so the recieving/target chain needs logic that makes the past state sufficent.

Ideally, our recieving/target chain would unify these merkle proofs somewhat, just like it unifies its own state proofs, although different users picked multiple parachain parents. All this depends somewhat upon the exact storage used, and polkadot's storage remains really stupid, so efforts like NOMT would cause churn in designs like this.

@muharem
Copy link
Contributor Author

muharem commented Jul 24, 2024

#722 this issue describes how the AliasOrigin instruction is meant to be setup. this PR uses it in a wrong way.

@muharem muharem closed this Jul 24, 2024
@bkchr bkchr deleted the muharem-transfer-pure-proxy branch July 24, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Any Proxies Copyable from a Configured Source
5 participants