Skip to content

[stable2503] Backport #8240#8432

Merged
acatangiu merged 4 commits intostable2503from
backport-8240-to-stable2503
May 9, 2025
Merged

[stable2503] Backport #8240#8432
acatangiu merged 4 commits intostable2503from
backport-8240-to-stable2503

Conversation

@paritytech-release-backport-bot
Copy link
Copy Markdown

Backport #8240 into stable2503 from claravanstaden.

See the documentation on how to use this bot.

@paritytech-release-backport-bot
Copy link
Copy Markdown
Author

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-8240-to-stable2503
git worktree add --checkout .worktree/backport-8240-to-stable2503 backport-8240-to-stable2503
cd .worktree/backport-8240-to-stable2503
git reset --hard HEAD^
git cherry-pick -x 632eed96b4560a45ce41c6f6bedec9a4c745a604
git push --force-with-lease

@github-actions github-actions bot added the A3-backport Pull request is already reviewed well in another branch. label May 5, 2025
@acatangiu acatangiu marked this pull request as ready for review May 7, 2025 14:06
@paritytech-review-bot paritytech-review-bot bot requested a review from a team May 7, 2025 14:07
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 7, 2025

This pull request is amending an existing release. Please proceed with extreme caution,
as to not impact downstream teams that rely on the stability of it. Some things to consider:

  • Backports are only for 'patch' or 'minor' changes. No 'major' or other breaking change.
  • Should be a legit fix for some bug, not adding tons of new features.
  • Must either be already audited or not need an audit.
Emergency Bypass

If you really need to bypass this check: add validate: false to each crate
in the Prdoc where a breaking change is introduced. This will release a new major
version of that crate and all its reverse dependencies and basically break the release.

@acatangiu acatangiu enabled auto-merge (squash) May 7, 2025 15:09
@acatangiu acatangiu requested a review from a team May 7, 2025 15:09
Comment on lines +228 to +230
let message = Message {
origin,
id: Default::default(),
id: frame_system::unique((origin, &command, fee)).into(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd assume the ID should be generated by the system-frontend pallet on AH here, along with an additional setTopic instruction using a provided ID.

Another issue is that the current register_token call is free, which may not be the intended behavior.

To address this, we could consider introducing a higher-level extrinsic in the system-v2 pallet, and have system-frontend call that entry point - passing in the necessary context such as the fee (for burning) and topic_id (for tracing). This wrapper would internally call functions like register_token or add_tip, as seen in PR #8271.

This isn’t specific to this backport PR and is non-blocking, just my two cents; we can revisit and improve it later.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ron can you open a ticket to track this?

Copy link
Copy Markdown
Contributor

@yrong yrong May 8, 2025

Choose a reason for hiding this comment

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

GatewayProxyAddress,
EthereumUniversalLocation,
GlobalAssetHubLocation,
AssetHubFromEthereum,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Can you add a ticket to document the difference between AssetHubFromEthereum and AssetHubUniversalLocation, with an example values?

The other generic parameters could also do with more documentation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

@yrong yrong May 9, 2025

Choose a reason for hiding this comment

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

In addition to updating the documentation, I'd suggest refactoring and reorganizing the generics here for clarity.
Original comment: #8240 (comment) and I've created a ticket for tracking: https://linear.app/snowfork/issue/SNO-1498

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agree, some refactoring is needed 👍

@acatangiu acatangiu merged commit 443a75d into stable2503 May 9, 2025
278 of 322 checks passed
@acatangiu acatangiu deleted the backport-8240-to-stable2503 branch May 9, 2025 11:35
alstjd0921 pushed a commit to bifrost-platform/polkadot-sdk that referenced this pull request Aug 14, 2025
Backport paritytech#8240 into `stable2503` from claravanstaden.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

---------

Co-authored-by: Clara van Staden <claravanstaden64@gmail.com>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A3-backport Pull request is already reviewed well in another branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants