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

Relax Copy to Clone for AssetId #12731

Closed
wants to merge 3 commits into from
Closed

Conversation

gilescope
Copy link
Contributor

@gilescope gilescope commented Nov 17, 2022

For the bridgehub we want to use a second Asset pallet for bridged assets. The suggestion is to have the AssetId of this instance be a MultiLocation. However Multilocation is not Copy, so this PR is exploring the impact of relaxing the Copy bound to only require Clone.

cumulus companion: paritytech/cumulus#1900

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Nov 17, 2022
@gilescope gilescope added B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Nov 17, 2022
Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

Looks ok to me

@gilescope
Copy link
Contributor Author

gilescope commented Nov 18, 2022

Maybe this is not needed - MulitiLocation v3 is Copy. We could potentially wait for that to land.

@joepetrowski
Copy link
Contributor

Maybe this is not needed - MulitiLocation v3 is Copy. We could potentially wait for that to land.

I agree focusing on v3 is probably the best strategy as it's already needed for related goals with the second Assets pallet.

@gilescope
Copy link
Contributor Author

if we're doing #12740 then maybe we should as then its not delayed by xcm3. We can have it all hit master which is going to be less complicated.

@stale
Copy link

stale bot commented Dec 21, 2022

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Dec 21, 2022
@gilescope
Copy link
Contributor Author

I think we can get away with keeping copy.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants