Skip to content

Send Token to Kusama #1463

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

Merged
merged 52 commits into from
Jun 2, 2025
Merged

Send Token to Kusama #1463

merged 52 commits into from
Jun 2, 2025

Conversation

claravanstaden
Copy link
Contributor

@claravanstaden claravanstaden commented Apr 29, 2025

  • Adds API layer forKusama
  • Test scripts for transferring Weth and Dot from Polkadot to Kusama and back.

@claravanstaden claravanstaden marked this pull request as ready for review May 14, 2025 12:57
@claravanstaden
Copy link
Contributor Author

@yrong sure, https://assethub-kusama.subscan.io/extrinsic/9407589-2. There is a KSM amount minted yeah.

@acatangiu
Copy link

I sent WETH from Kusama to Polkadot, and my KSM balance was lowered, even though I used DOT as fee. So I think KSM makes more sense to use as fee, right? Otherwise the user needs some DOT and some KSM, which is bad UX.

yes, try to use native whenever possible, I expect requiring KSM on Kusama side to be better UX than requiring DOT there - either way, if the source is KAH or PAH, the UI could check pools and do some swap before to cover all fees using only WETH

@claravanstaden
Copy link
Contributor Author

To summarize the changes that I'll make:

  • Change the fee asset from K->P to KSM
  • Use the Balances::Minted event from the dry run (same for P->K direction) as the execution fee (so total fee would be Balances::Minted + delivery fee) @alistair-singh let me know if you agree with this? This will replace getting the XcmBridgeHubRouterBaseFee from storage (and calculating the message fee per byte, which I have not implemented)

@yrong
Copy link
Contributor

yrong commented May 22, 2025

Use the Balances::Minted event from the dry run (same for P->K direction) as the execution fee (so total fee would be Balances::Minted + delivery fee)

To be honest, after giving it more thought, I’m not sure the fee estimation justifies the complexity of introducing dry_run. The current API code already feels a bit too complicated.

In my opinion,the fee could be added as a static configuration value, which should work fine for both Snowbridge V1 and V2. I'd expect the fee is relatively stable and unlikely to change frequently, we wouldn’t need to reconfigure it often.

So, if possible, I’d prefer to do some cleanup rather than introduce more estimation logic and make things even more complex.

We can discuss it furthur tomorrow.

@claravanstaden
Copy link
Contributor Author

@yrong I changed the fee from K->P to KSM, and added KSM as a transferable asset (with scripts): http://github.com/Snowfork/snowbridge/pull/1463/commits/cd0462175656a8494ca42314758cd13f4b332a31

Awaiting feedback on how to determine the fee: https://snowfork.slack.com/archives/G01BGMGLAC9/p1747993120325679 Once decided, I'll implement it.

@yrong
Copy link
Contributor

yrong commented May 23, 2025

Awaiting feedback on how to determine the fee

Either way works for me as long as the fee estimation is accurate. I don’t have a strong preference, so I’ll leave the decision to you or Alistair.

I may do some cleanup and refactoring to support Snowbridge V2, but that will be handled independently.

Copy link
Contributor

@alistair-singh alistair-singh left a comment

Choose a reason for hiding this comment

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

Just a dangling debug log command that needs to be removed.

@@ -750,6 +856,7 @@ export function padFeeByPercentage(fee: bigint, padPercent: bigint) {
if (padPercent < 0 || padPercent > 100) {
throw Error(`padPercent ${padPercent} not in range of 0 to 100.`)
}
console.log("PAD: ", fee * ((100n + padPercent) / 100n))
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this logging?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in 90a7ddc.

Copy link
Contributor

@yrong yrong left a comment

Choose a reason for hiding this comment

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

+1

@claravanstaden claravanstaden merged commit 64e54b6 into main Jun 2, 2025
1 check passed
@claravanstaden claravanstaden deleted the clara/send-token-kusama branch June 2, 2025 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants