Skip to content
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

Fix delivery fees issue #3085

Merged
merged 5 commits into from
Jan 29, 2024

Conversation

franciscoaguirre
Copy link
Contributor

@franciscoaguirre franciscoaguirre commented Jan 26, 2024

This fix aims to solve an issue in Kusama that resulted in failed reserve asset transfers.
It will be ported later to master as well.
The issue is that during multi-hop XCMs, like reserve asset transfers where the reserve is not the sender nor the destination, there are no assets available to pay for delivery fees.
This fix makes sure to deduct delivery fees from the assets being transferred.

  • code changes required for fix now complete
  • local testing done shows issue as fixed
  • regression test added
  • PR reviewed
  • CI passing - currently issue with CI not running on non-master PR
  • xcm-executor patch version 4.0.1 released
  • xcm-executor patch version 4.0.1 included in https://github.com/polkadot-fellows/runtimes/
  • Kusama and Polkadot runtimes patch released

@franciscoaguirre franciscoaguirre added the R0-silent Changes should not be mentioned in any release notes label Jan 26, 2024
Signed-off-by: Adrian Catangiu <adrian@parity.io>
Signed-off-by: Adrian Catangiu <adrian@parity.io>
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.

We need to run full CI before merging/releasing

vec![ReserveAssetDeposited(to_weigh.into()), ClearOrigin];
message_to_weigh.extend(xcm.0.clone().into_iter());
let (_, fee) =
validate_send::<Config::XcmSender>(dest.clone(), Xcm(message_to_weigh))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

should we just modify send to not charge fee and have another method that charges send fee and call it here? so that we don't need to calculate the send fee multiple times

@bkchr
Copy link
Member

bkchr commented Jan 29, 2024

@franciscoaguirre what are we waiting for?

@franciscoaguirre
Copy link
Contributor Author

@franciscoaguirre what are we waiting for?

On it

@bkchr bkchr merged commit 993dfbd into release-crates-io-v1.3.0 Jan 29, 2024
10 of 11 checks passed
@bkchr bkchr deleted the for-real-fix-delivery-fees-issue branch January 29, 2024 12:03
@Hawkarhf92
Copy link

Hello dears , I need your help for back my tokens ,it stucks and here's the extrinsic hash
0x78540d5b0b3831468c7c4a70ef5a81bccb3c7cb96e44ad81e39c96e73a9ebcff
Please

franciscoaguirre added a commit that referenced this pull request Feb 5, 2024
This fix aims to solve an issue in Kusama that resulted in failed
reserve asset transfers.
It will be ported later to master as well.
The issue is that during multi-hop XCMs, like reserve asset transfers
where the reserve is not the sender nor the destination, there are no
assets available to pay for delivery fees.
This fix makes sure to deduct delivery fees from the assets being
transferred.

- [x] code changes required for fix now complete
- [x] local testing done shows issue as fixed
- [x] regression test added
- [x] PR reviewed
- [x] CI passing - currently issue with CI not running on non-master PR
- [ ] xcm-executor patch version 4.0.1 released
- [ ] xcm-executor patch version 4.0.1 included in
https://github.com/polkadot-fellows/runtimes/
- [ ] Kusama and Polkadot runtimes patch released

---------

Signed-off-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
girazoki pushed a commit to moondance-labs/polkadot-sdk that referenced this pull request Mar 19, 2024
This fix aims to solve an issue in Kusama that resulted in failed
reserve asset transfers.
It will be ported later to master as well.
The issue is that during multi-hop XCMs, like reserve asset transfers
where the reserve is not the sender nor the destination, there are no
assets available to pay for delivery fees.
This fix makes sure to deduct delivery fees from the assets being
transferred.

- [x] code changes required for fix now complete
- [x] local testing done shows issue as fixed
- [x] regression test added
- [x] PR reviewed
- [x] CI passing - currently issue with CI not running on non-master PR
- [ ] xcm-executor patch version 4.0.1 released
- [ ] xcm-executor patch version 4.0.1 included in
https://github.com/polkadot-fellows/runtimes/
- [ ] Kusama and Polkadot runtimes patch released

---------

Signed-off-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants