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

[XCM] add generic location to account converter for locations in external ecosystems #7129

Open
acatangiu opened this issue Jan 13, 2025 · 4 comments · May be fixed by #7313
Open

[XCM] add generic location to account converter for locations in external ecosystems #7129

acatangiu opened this issue Jan 13, 2025 · 4 comments · May be fixed by #7313
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. T6-XCM This PR/Issue is related to XCM.

Comments

@acatangiu
Copy link
Contributor

We need a location to account converter that can be used by any parachain in Polkadot/Kusama and it can convert any location from an external GlobalConsensus.

We basically want a converter that encompasses GlobalConsensusConvertsFor and GlobalConsensusParachainConvertsFor, but also enhances them with DescribeFamily-like support.

The implementation is simply a concatenation of them.

@acatangiu acatangiu added T6-XCM This PR/Issue is related to XCM. C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. labels Jan 13, 2025
@acatangiu acatangiu moved this to Todo in Bridges + XCM Jan 13, 2025
@Nathy-bajo
Copy link

Working on this @acatangiu

@acatangiu
Copy link
Contributor Author

Ok, let me know if you need any help 👍

@Nathy-bajo
Copy link

Hi @acatangiu I tried attempting this.
Please review, I'm writing a test for the fix.

@acatangiu
Copy link
Contributor Author

Ideally we want a single new convertor that can replace these 3 convertors in a fully backwards compatible way. Meaning any AccountId->Location conversion currently covered by these 3 will also be covered by the new one and provide the exact same output/conversion.

Example tests here that can be enhanced to ensure backwards compatibility.

On top of covering the existing set of Locations as the 3 convertors mentioned above, it should also cover external global consensus locations.

For example, GlobalConsensusParachainConvertsFor currently converts Location { parents: 2, interior: X2([GlobalConsensus(bla), Parachain(X)]), but does not support child locations of it, such as Location { parents: 2, interior: X3([GlobalConsensus(bla), Parachain(X), AccountId(Alice)]).

One way is to create a new tuple that starts with existing converters and ends with a new one for the new set of covered locations:

pub type GlobalLocationToAccount<UniversalLocation, AccountId> = (
	// Foreign locations alias into accounts according to a hash of their standard description.
	HashedDescription<AccountId, DescribeFamily<DescribeAllTerminal>>,
	// Different global consensus parachain sovereign account.
	// (Used for over-bridge transfers and reserve processing)
	GlobalConsensusParachainConvertsFor<UniversalLocation, AccountId>,
	// Ethereum contract sovereign account.
	// (Used to convert ethereum contract locations to sovereign accounts)
	EthereumLocationsConverterFor<AccountId>,
	// Used to convert any other external global consensus locations to sovereign accounts.
	ExternalConsensusLocationsConverterFor<UniversalLocation, AccountId>,
);

ExternalConsensusLocationsConverterFor should:

  • validates the location is from different consensus
  • derives some string/bytes from that external consensus (smth like GlobalConsensusParachainConvertsFor)
  • derives some string/bytes from the sublocation remaining after processing above (smth like DescribeAllTerminal)
  • concatenates and hashes above for a final derived AccountId

(also because EthereumLocationsConverterFor is already used and should be supported, but cannot be directly added to the tuple because it lives in a different downstream crate, you should just hardcode it inside ExternalConsensusLocationsConverterFor).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. T6-XCM This PR/Issue is related to XCM.
Projects
Status: Todo
2 participants