Skip to content

Snowbridge: enforce fee when registering Polkadot native asset#8725

Merged
acatangiu merged 26 commits intoparitytech:masterfrom
yrong:register-pna-with-fee
Jun 9, 2025
Merged

Snowbridge: enforce fee when registering Polkadot native asset#8725
acatangiu merged 26 commits intoparitytech:masterfrom
yrong:register-pna-with-fee

Conversation

@yrong
Copy link
Copy Markdown
Contributor

@yrong yrong commented Jun 3, 2025

@yrong
Copy link
Copy Markdown
Contributor Author

yrong commented Jun 3, 2025

@claravanstaden Please check this commit, looks like we missed applying the burn fee when adding a tip?

@claravanstaden
Copy link
Copy Markdown
Contributor

@yrong good catch, I definitely missed that. 🫤

Copy link
Copy Markdown
Contributor

@claravanstaden claravanstaden left a comment

Choose a reason for hiding this comment

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

Nice!

@yrong yrong marked this pull request as ready for review June 3, 2025 15:25
@paritytech-review-bot paritytech-review-bot bot requested a review from a team June 3, 2025 15:26
@yrong yrong changed the title Snowbridge: register polkadot native asset with fee Snowbridge: enforce fee when registering Polkadot native asset Jun 3, 2025
@paritytech-review-bot paritytech-review-bot bot requested a review from a team June 4, 2025 13:29
let remote_xcm = Self::build_remote_xcm(&call);
let message_id = Self::send_xcm(origin_location, dest.clone(), remote_xcm.clone())
.map_err(|error| Error::<T>::from(error))?;
let ether_gained = Self::swap_fee_asset_and_burn(origin_location.clone(), fee_asset)?;
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.

This will fail for Root origin when fee_asset is non-zero.

Suggested change
let ether_gained = Self::swap_fee_asset_and_burn(origin_location.clone(), fee_asset)?;
let ether_gained = if origin_location.is_here() {
// Root origin/location does not pay any fees/tip.
0
} else {
Self::swap_fee_asset_and_burn(origin_location.clone(), fee_asset)?
};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice catch!
5df1115

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 would still prefer not to call this function at all for Root, but not going to block on it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@acatangiu acatangiu moved this to To be released (SDK) in fellowship/runtimes integrations queue Jun 5, 2025
@yrong yrong requested a review from claravanstaden June 5, 2025 14:10
Copy link
Copy Markdown
Contributor

@claravanstaden claravanstaden left a comment

Choose a reason for hiding this comment

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

Looks like you need to fix the PR doc versions, otherwise looks good!

@yrong
Copy link
Copy Markdown
Contributor Author

yrong commented Jun 5, 2025

Looks like you need to fix the PR doc versions, otherwise looks good!

I've added validate: false to supress the semver check but it does not seem to work — not sure if I did it correctly.

I remember Adrian mentioned that since we're the only end user of these packages, the semver check does't really matter.

@acatangiu acatangiu added the A4-backport-stable2503 Pull request must be backported to the stable2503 release branch label Jun 9, 2025
decimals: 12,
};

let fee_asset = Asset::from((Location::parent(), 1_000_000u128));
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: maybe 1_000_000u128 should be a const with some descriptive name?

@acatangiu acatangiu added the A4-backport-stable2506 Pull request must be backported to the stable2506 release branch label Jun 9, 2025
@acatangiu acatangiu added this pull request to the merge queue Jun 9, 2025
Merged via the queue into paritytech:master with commit 36c3039 Jun 9, 2025
395 of 398 checks passed
@paritytech-release-backport-bot
Copy link
Copy Markdown

Created backport PR for stable2503:

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

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

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

Backport failed for stable2506: couldn't find remote ref stable2506.
Please ensure that this Github repo has a branch named stable2506.

@acatangiu acatangiu removed the A4-backport-stable2506 Pull request must be backported to the stable2506 release branch label Jun 10, 2025
acatangiu added a commit that referenced this pull request Jun 11, 2025
Backport #8725 into `stable2503` from yrong.

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: Ron <yrong1997@gmail.com>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
ordian added a commit that referenced this pull request Jun 12, 2025
* master: (62 commits)
  release/build-macos-binaries: add missing FEATURES argument  (#8816)
  Add XCM Precompile to `pallet-xcm` (#8693)
  [Release|CI/CD] Exclude test runtimes from the runtimes build (#8820)
  Add freebsd sysinfo for telemetry (#7985)
  release-reusable-rc-build: add optional `features` input that can be considered for nodes building (#8755)
  [Staking] Cleanups and some improvements (#8701)
  Fix typos in 3 files in Implementers Guide (#8799)
  Update `RemoteExporter` docs to reflect removal of `forward_id_for` (#8795)
  Snowbridge: enforce fee when registering Polkadot native asset (#8725)
  Bump the ci_dependencies group across 1 directory with 7 updates (#8788)
  Docker hub 'master' image short sha (#8790)
  [Release|CI/CD] Combine branch-off and RC automation flows (#8754)
  Move Transaction depth limit checks (#8750)
  Add genesis presets for remaining runtimes in polkadot-parachain-bin (#8426)
  Do not make pallet-identity benchmarks signature-dependent (#8179)
  Introduction of Approval Slashes [Disabling Strategy Stage 4] (#6827)
  [AHM] Prepare For Westend Cleanup (#8715)
  Actually use RP offset in YAP parachain (#8745)
  [AHM] Relax the requirement for RC-Client to receive +1 session reports (#8702)
  Don't read storage items in logging (#8749)
  ...
alstjd0921 pushed a commit to bifrost-platform/polkadot-sdk that referenced this pull request Aug 14, 2025
Backport paritytech#8725 into `stable2503` from yrong.

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: Ron <yrong1997@gmail.com>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A4-backport-stable2503 Pull request must be backported to the stable2503 release branch T15-bridges This PR/Issue is related to bridges.

Projects

Status: Done
Status: To be released (SDK)

Development

Successfully merging this pull request may close these issues.

5 participants