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

Backport XCM fixes to 1.5.0 #3174

Merged
merged 10 commits into from
Feb 8, 2024

Conversation

franciscoaguirre
Copy link
Contributor

  • Transactional processing
  • Delivery fees taken from holding

…sets (#3142)

This fix aims to solve an issue in Kusama that resulted in failed
reserve asset transfers.

During multi-hop XCMs, like reserve asset transfers where the reserve is
not the sender nor the destination, but a third remote chain, the origin
is not available to pay for delivery fees out of their account directly,
so delivery fees should be paid out of transferred assets.

This commit also adds an xcm-emulator regression test that validates
this scenario is now working.

Signed-off-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
@franciscoaguirre franciscoaguirre changed the base branch from master to release-crates-io-v1.5.0 February 1, 2024 15:46
Moved from: paritytech/polkadot#6951

closes #490

- [x] update cumulus

---
This PR introduces transactional processing of certain xcm instructions.
For the list of instructions checkout
#490. The transactional
processing is implemented as an xcm-executor config item. The two
implementations in this PR are `FrameTransactionalProcessor` and `()`.
The `()` implementation does no transactional processing. Each
implementation of the `ProcessTransaction` trait has an
`IS_TRANSACTIONAL` const that tells the XCVM if transactional processing
is actually implemented. If Transactional processing is implemented,
changes to touched registers should also be rolled back to prevent
inconsistencies.

Note for reviewers:
Check out the following safety assumption:
https://github.com/paritytech/polkadot-sdk/pull/1222/files#diff-4effad7d8c1c9de19fd27e18661cbf2128c8718f3b2420a27d2f816e0749ea53R30

---------

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Co-authored-by: command-bot <>
@franciscoaguirre franciscoaguirre added R0-silent Changes should not be mentioned in any release notes T6-XCM This PR/Issue is related to XCM. labels Feb 1, 2024
@franciscoaguirre
Copy link
Contributor Author

bot fmt

@command-bot
Copy link

command-bot bot commented Feb 1, 2024

@franciscoaguirre https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5100450 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 2-75fb32f6-c004-4b19-a150-99f2a3cb2d36 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Feb 1, 2024

@franciscoaguirre Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5100450 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5100450/artifacts/download.

@xlc
Copy link
Contributor

xlc commented Feb 1, 2024

will there be one to back port to 1.6.0 or it is already done?

@franciscoaguirre
Copy link
Contributor Author

will there be one to back port to 1.6.0 or it is already done?

Yes, there will be

@bkontur
Copy link
Contributor

bkontur commented Feb 2, 2024

@franciscoaguirre you need at least these two commits (xcm benchmarks fix + rust 1.7.4) - see last two commits in my backport to 1.5.0 https://github.com/paritytech/polkadot-sdk/pull/3094/commits

acatangiu and others added 4 commits February 2, 2024 14:49
For some reason original PR passed CI - when it shouldn't have. Fix
`pallet-xcm` test benchmarks.
cc paritytech/ci_cd#900

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Dónal Murray <donal.murray@parity.io>
Co-authored-by: Bastian Köcher <info@kchr.de>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: command-bot <>
@acatangiu
Copy link
Contributor

hmm.. what are the CI issues currently blocking us here? is it the upgraded toolchain in CI vs old codebase?

@bkontur
Copy link
Contributor

bkontur commented Feb 6, 2024

@EgorPopelyaev
is target branch ok? release-crates-io-v1.5.0 vs release-polkadot-v1.5.0?
this #3094 was merged to the release-polkadot-v1.5.0

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 2/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5152424

@franciscoaguirre franciscoaguirre merged commit 6bce1f9 into release-crates-io-v1.5.0 Feb 8, 2024
16 of 84 checks passed
@franciscoaguirre franciscoaguirre deleted the xcm-backport-1.5.0 branch February 8, 2024 12:35
bkchr pushed a commit to polkadot-fellows/runtimes that referenced this pull request Feb 14, 2024
Attached result of `cargo upgrade -v --pinned --incompatible`
[cargo-upgrade-version-bump.log](https://github.com/polkadot-fellows/runtimes/files/13873056/cargo-upgrade-version-bump.log)

E.g.: `frame-support` from `25.0.0` to `27.0.0`

_Note: Encointer was not upgraded (because its pallet references
`polkadot@1.3.0` release)._

## TODO

- [x] fix compilation
- [x] fix integration tests
- [x] fix benchmarks (also try them)
- [ ] patch for `pallet-nomination-pools` migration fix
paritytech/polkadot-sdk#3094
- [x] patch for `xcm-executor` fix
paritytech/polkadot-sdk#3174
- [x] search for `TODO:(PR#137) - wait for xcm-executor patch` (missing
`FrameTransactionalProcessor`)
- [x] check/verify `MaxExposurePageSize = 512` / `MaxNominators = 512`
for Kusama/Polkadot
- [x] check with SRLabs (Jakob)
paritytech-secops/srlabs_findings#327


Closes: #113

---------

Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
bkchr pushed a commit to polkadot-fellows/runtimes that referenced this pull request Feb 27, 2024
Based on bump to
[`polkadot-sdk@1.5.0`](#137).

Attached result of `cargo upgrade -v --pinned --incompatible`
[cargo-upgrade-version-bump.log](https://github.com/polkadot-fellows/runtimes/files/14044160/cargo-upgrade-version-bump.log)

_Note: Encointer was not upgraded (because its pallet references
`polkadot@1.3.0` release)._

## ~~For reviewers~~

~~This PR is against `polkadot-fellows`'s main to bring it to the
fellows repo, but if you want to see a real diff relevant to the
`polkadot-sdk@1.6.0` update please check:
bkontur/runtimes@bko-bump-to-1.5...bkontur:runtimes:bko-bump-to-1.6.~~


## TODO

- [x] fix compilation
- [x] fix integration tests
- [x] fix benchmarks (also try them) - `collectives-polkadot` `payout`
- [ ] ~~Does not require a CHANGELOG entry~~
- [x] `warning: use of deprecated struct
`staging_xcm_builder::CurrencyAdapter`: Use `FungibleAdapter` instead`
- [ ] search for `TODO:(PR#159) change to FungibleAdapter` and/or wait
for paritytech/polkadot-sdk#3287
- [x] patch for `pallet-nomination-pools` migration fix
paritytech/polkadot-sdk#3093
- will be fixed here
#188 (comment)
- [x] patch for `xcm-executor` fix (for 1.6.0) e.g.
paritytech/polkadot-sdk#3174
- [x] check/fix coretime stuff for Kusama/Polkadot - search for `//
TODO:(PR#159)(PR#1694)` - see
[comment](#159 (comment))
- fixed by bkontur#3
- [x] check the
`MaxControllersInDeprecationBatch`https://github.com/polkadot-fellows/runtimes/pull/159/files#r1492361038
- [x] check `pallet_identity::Config` for Kusama and Polkadot
https://github.com/polkadot-fellows/runtimes/pull/159/files#r1492363866

---------

Signed-off-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Co-authored-by: Ross Bulat <ross@parity.io>
Co-authored-by: Alain Brenzikofer <alain@integritee.network>
Co-authored-by: eskimor <eskimor@no-such-url.com>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: fellowship-merge-bot[bot] <151052383+fellowship-merge-bot[bot]@users.noreply.github.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 T6-XCM This PR/Issue is related to XCM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants