This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
[xcm] GlobalConsensusConvertsFor for remote relay chain (based on pevious GlobalConsensusParachainConvertsFor)
#7517
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…revious GlobalConsensusParachainConvertsFor)
bkontur
added
A0-please_review
…s-location-converter
…s-location-converter
…s-location-converter
|
bot rebase |
…s-location-converter
|
Rebased |
vstam1
approved these changes
Jul 26, 2023
Comment on lines
356
to
360
| /// WARNING: This results in the same `AccountId` value being generated regardless | ||
| /// of the relative security of the local chain and the Relay-chain of the input | ||
| /// location. This may not have any immediate security risks, however since it creates | ||
| /// commonalities between chains with different security characteristics, it could | ||
| /// possibly form part of a more sophisticated attack scenario. |
There was a problem hiding this comment.
Edited for clarity, however I don't particularly agree with this comment. It does not create commonalities between chains; actually it says that one chain will have the same 32-byte AccountId in other consensus systems (that use 32-byte account IDs). If this chain has a security incident, that would have effects in many other chains, but it does not put anything in common between the local chain and the "global" chain.
Unless I am missing something, I would delete this comment.
Suggested change
| /// WARNING: This results in the same `AccountId` value being generated regardless | |
| /// of the relative security of the local chain and the Relay-chain of the input | |
| /// location. This may not have any immediate security risks, however since it creates | |
| /// commonalities between chains with different security characteristics, it could | |
| /// possibly form part of a more sophisticated attack scenario. | |
| /// WARNING: This results in the same `AccountId` value being generated regardless of the relative | |
| /// security of the local chain and the security of the input location. This may not have any | |
| /// immediate security risks; however, because it creates commonalities between chains with | |
| /// different security characteristics, it could possibly form part of a more sophisticated attack | |
| /// scenario. |
…s-location-converter
Co-authored-by: joe petrowski <[email protected]>
Co-authored-by: joe petrowski <[email protected]>
Co-authored-by: joe petrowski <[email protected]>
…nverter' into bko-global-consensus-location-converter
KiChjang
reviewed
Jul 30, 2023
|
bot rebase |
…s-location-converter
|
Rebased |
|
bot rebase |
…s-location-converter
|
Rebased |
|
The CI pipeline was cancelled due to failure one of the required jobs. |
KiChjang
approved these changes
Aug 2, 2023
franciscoaguirre
approved these changes
Aug 2, 2023
|
bot merge |
paritytech-processbot
bot
deleted the
bko-global-consensus-location-converter
branch
s0me0ne-unkn0wn
pushed a commit
that referenced
this pull request
Aug 15, 2023
…evious GlobalConsensusParachainConvertsFor) (#7517) * [xcm] `GlobalConsensusConvertsFor` for remote relay chain (based on previous GlobalConsensusParachainConvertsFor) * Typo * PR fix (constants in test) * Re-export of `GlobalConsensusConvertsFor` * assert to panic * Update xcm/src/v3/multiasset.rs Co-authored-by: joe petrowski <[email protected]> * Update xcm/xcm-builder/src/location_conversion.rs Co-authored-by: joe petrowski <[email protected]> * Update xcm/xcm-builder/src/location_conversion.rs Co-authored-by: joe petrowski <[email protected]> * Review fixes --------- Co-authored-by: parity-processbot <> Co-authored-by: joe petrowski <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relates to: paritytech/parity-bridges-common#2401
Will be used to generate accountId as sovereign account of remote relaychain, e.g. for setting
pallet_assets::ForeignAssetsowner/issuer/freezer/admin for wrappedDOT/wrappedKSMe.g.: on AssetHubKusama for foreign asset with
MultiLocation { parents: 2, X1(GlobalConsensus(Polkadot)) }owner accountId will beand also this
GlobalConsensusConvertsForcould be added to theLocationToAccountIdon AssetHubs, which will allow to processxcm::Transact(pallet_assets::set_team/...)over bridge from remote relay chain as owner of its wrapped representation of native asset