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): Send back xcDOT to AssetHub from Moonbeam #2938

Merged
merged 14 commits into from
Sep 12, 2024

Conversation

Agusrodri
Copy link
Contributor

@Agusrodri Agusrodri commented Sep 6, 2024

What does it do?

This PR takes care of fixing xcDOTs transfers between Moonbeam chains and AssetHub, specially the case in which we want to send xcDOTs tokens from Moonbeam back to AssetHub.

The fix basically consists on modifying some code inside Xtokens pallet (in our ORML fork), for it to properly recognize AssetHub as a reserve for DOTs at the moment of executing the transfer.

What important points reviewers should know?

From what tests show, there is no need to modify our xTokens precompile for this kind of transfer to work.

Given that, I reverted the initial changes that modified the precompile adding helper functions to convert the currency_id/location.

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

moonbeam-foundation/open-runtime-module-library#7

Copy link
Contributor

github-actions bot commented Sep 6, 2024

WASM runtime size check:

Compared to target branch

Moonbase runtime: 2176 KB (no changes) ✅

Moonbeam runtime: 2140 KB (no changes) ✅

Moonriver runtime: 2140 KB (no changes) ✅

Compared to latest release (runtime-3102)

Moonbase runtime: 2176 KB (+236 KB compared to latest release) ⚠️

Moonbeam runtime: 2140 KB (+240 KB compared to latest release) ⚠️

Moonriver runtime: 2140 KB (+240 KB compared to latest release) ⚠️

Copy link
Contributor

github-actions bot commented Sep 6, 2024

Coverage Report

@@                      Coverage Diff                       @@
##           master   agustin-xcDot-assetHub-fix      +/-   ##
==============================================================
+ Coverage   78.70%                       79.40%   +0.70%     
  Files         294                          294              
+ Lines       84425                        87275    +2850     
==============================================================
+ Hits        66440                        69300    +2860     
- Misses      17985                        17975      -10     
Files Changed Coverage
/pallets/moonbeam-xcm-benchmarks/src/weights/generic.rs 4.26% (+1.30%) 🔼
/pallets/moonbeam-xcm-benchmarks/src/weights/mod.rs 13.61% (+1.57%) 🔼
/runtime/moonbase/tests/xcm_mock/statemint_like.rs 63.38% (+1.62%) 🔼
/runtime/moonbase/tests/xcm_tests.rs 99.94% (+0.02%) 🔼
/runtime/moonbeam/tests/xcm_mock/statemint_like.rs 63.38% (+1.62%) 🔼
/runtime/moonbeam/tests/xcm_tests.rs 99.95% (+0.01%) 🔼
/runtime/moonriver/tests/xcm_mock/statemine_like.rs 63.38% (+1.62%) 🔼
/runtime/moonriver/tests/xcm_tests.rs 99.96% (+0.01%) 🔼

Coverage generated Thu Sep 12 15:51:35 UTC 2024

@RomarQ RomarQ added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. breaking Needs to be mentioned in breaking changes labels Sep 7, 2024
@Agusrodri Agusrodri marked this pull request as ready for review September 9, 2024 21:18
Cargo.toml Outdated Show resolved Hide resolved
@noandrea noandrea added the A8-mergeoncegreen Pull request is reviewed well. label Sep 12, 2024
RomarQ
RomarQ previously approved these changes Sep 12, 2024
librelois
librelois previously approved these changes Sep 12, 2024
@Agusrodri Agusrodri dismissed stale reviews from librelois and RomarQ via 3fe32b4 September 12, 2024 13:50
@Agusrodri Agusrodri merged commit 7623155 into master Sep 12, 2024
39 checks passed
@Agusrodri Agusrodri deleted the agustin-xcDot-assetHub-fix branch September 12, 2024 16:52
@noandrea noandrea mentioned this pull request Sep 17, 2024
18 tasks
noandrea pushed a commit that referenced this pull request Sep 17, 2024
* initial setup

* fix comments

* start testing

* add one more step in test

* add more tests

* revert changes in xtokens precompile

* remove para_event() unused function

* fix costs

* fix more costs

* use proper branch

* add tests to moonriver and moonbeam

* rename struct to ConcreteAssetFromRelay and add more docs

* remove unneeded transfer in test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A8-mergeoncegreen Pull request is reviewed well. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants