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

Allow to use evm foreign assets to pay XCM fees #2908

Merged
merged 33 commits into from
Sep 13, 2024

Conversation

librelois
Copy link
Collaborator

@librelois librelois commented Aug 20, 2024

What does it do?

Create a new pallet that register all supported asset for XCM fees with their price relative to the native asset (MOVR or GLMR).

This PR completes several goals at once:

  • Prepares the ground for the future removal of the asset-manager pallet by moving the management of data related to assets supported to pay XCM fees into a new pallet.
  • Adapt the design of the new pallet for potential future support of dynamic XCM fees (store a price relative to the native asset rather than a number of units to be paid per second of execution).
  • Provide a new WeightTrader implementation to support new evm foreign assets (and still support old ones)
  • Adds functionality to pause/resume asset support without deleting their data
  • Adds more granularity to the origin that can perform each operation (for example, it becomes possible to authorize a specific origin to modify the price of an asset, without being able to do anything else).

⚠️ breaking changes ⚠️

The way to add/edit/remove XCM fees support for a given asset has changed.

New way to register an asset price (formerly units per second)

Call: xcmWeightTrader.addAsset(location, relative_price)

Allowed origins: GeneralAdmin, 5/9 of OpenTechCommittee, Root

Parameters

  • location: Location scale encoded xcm v4 location of asset reserve relative to moonbeam
  • relative_price: u128 18 decimals balance needed to equal 1 unit of native asset (MOVR for moonriver and GLMR for moonbeam). For example, if the asset price is twice as low as the GLMR price, enter 500_000_000_000_000_000.

Substrate event

xcmWeightTrader.SupportedAssetAdded(location, relativePrice)

New way to update an asset price (formerly units per second)

Call: xcmWeightTrader.editAsset(location, relative_price)

Allowed origins: FastGeneralAdmin, 5/9 of OpenTechCommittee, Root

Parameters

  • location: Location scale encoded xcm v4 location of asset reserve relative to moonbeam
  • relative_price: u128 18 decimals balance needed to equal 1 unit of native asset (MOVR for moonriver and GLMR for moonbeam). For example, if the asset price is twice as low as the GLMR price, enter 500_000_000_000_000_000.

Substrate event

xcmWeightTrader.SupportedAssetEdited(location, relativePrice)

New feature: pause fee support for a given asset

Call: xcmWeightTrader.pauseAssetSupport(location)

Allowed origins: FastGeneralAdmin, 5/9 of OpenTechCommittee, Root

Parameters

  • location: Location scale encoded xcm v4 location of asset reserve relative to moonbeam

Substrate event

xcmWeightTrader.PauseAssetSupport(location)

New feature: resume fee support for a given asset

Call: xcmWeightTrader.resumeAssetSupport(location)

Allowed origins: 5/9 of OpenTechCommittee, Root_

Parameters

  • location: Location scale encoded xcm v4 location of asset reserve relative to moonbeam

Substrate event

xcmWeightTrader.ResumeAssetSupport(location)

New way to remove fee support for a given asset

Call: xcmWeightTrader.removeAsset(location)

Allowed origins: 5/9 of OpenTechCommittee, Root_

Parameters

  • location: Location scale encoded xcm v4 location of asset reserve relative to moonbeam

Substrate event

xcmWeightTrader.SupportedAssetRemoved(location)

What important points reviewers should know?

TODO

  • Refactor XCM runtime api to use the new pallet
  • Add rust tests for trader implementation
  • Add pallet to moonbase runtime config
  • Add rust integration tests for moonbase runtime
  • Add some dev-tests

@librelois librelois 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 Aug 20, 2024
Copy link
Contributor

github-actions bot commented Aug 21, 2024

Coverage Report

@@                    Coverage Diff                     @@
##           master   elois-evm-foreign-fees      +/-   ##
==========================================================
+ Coverage   78.70%                   79.33%   +0.63%     
+ Files         294                      298       +4     
+ Lines       84425                    87119    +2694     
==========================================================
+ Hits        66440                    69114    +2674     
+ Misses      17985                    18005      +20     
Files Changed Coverage
/pallets/asset-manager/src/lib.rs 78.18% (-6.59%) 🔽
/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/common/src/apis.rs 29.83% (+1.60%) 🔼
/runtime/common/src/migrations.rs 72.20% (-25.75%) 🔽
/runtime/moonbase/src/lib.rs 52.85% (-0.08%) 🔽
/runtime/moonbase/src/xcm_config.rs 58.65% (-5.35%) 🔽
/runtime/moonbase/tests/xcm_mock/parachain.rs 60.85% (+0.63%) 🔼
/runtime/moonbase/tests/xcm_mock/statemint_like.rs 63.38% (+1.62%) 🔼
/runtime/moonbase/tests/xcm_tests.rs 99.91% (-0.01%) 🔽
/runtime/moonbeam/src/lib.rs 46.77% (-0.08%) 🔽
/runtime/moonbeam/src/xcm_config.rs 47.47% (-5.16%) 🔽
/runtime/moonbeam/tests/xcm_mock/parachain.rs 60.85% (+0.63%) 🔼
/runtime/moonbeam/tests/xcm_mock/statemint_like.rs 63.38% (+1.62%) 🔼
/runtime/moonbeam/tests/xcm_tests.rs 99.93% (-0.01%) 🔽
/runtime/moonriver/src/lib.rs 46.99% (-0.08%) 🔽
/runtime/moonriver/src/xcm_config.rs 45.45% (-5.08%) 🔽
/runtime/moonriver/tests/xcm_mock/parachain.rs 61.57% (+0.62%) 🔼
/runtime/moonriver/tests/xcm_mock/statemine_like.rs 63.38% (+1.62%) 🔼
/runtime/moonriver/tests/xcm_tests.rs 99.93% (-0.02%) 🔽

Coverage generated Fri Sep 13 15:08:38 UTC 2024

Copy link
Contributor

github-actions bot commented Aug 28, 2024

WASM runtime size check:

Compared to target branch

Moonbase runtime: 2192 KB (no changes) ✅

Moonbeam runtime: 2144 KB (no changes) ✅

Moonriver runtime: 2140 KB (no changes) ✅

Compared to latest release (runtime-3102)

Moonbase runtime: 2192 KB (+252 KB compared to latest release) ⚠️

Moonbeam runtime: 2144 KB (+244 KB compared to latest release) ⚠️

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

@librelois librelois changed the title WIP: Allow to use evm foreign assets to pay XCM fees Allow to use evm foreign assets to pay XCM fees Aug 28, 2024
@librelois librelois marked this pull request as ready for review August 29, 2024 15:21
gonzamontiel
gonzamontiel previously approved these changes Aug 30, 2024
Copy link
Contributor

@gonzamontiel gonzamontiel left a comment

Choose a reason for hiding this comment

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

Changes are great, also good coverage! I left some comments, maybe we could add a missing test (even though there are many), other than that LGTM.

pallets/xcm-weight-trader/src/lib.rs Show resolved Hide resolved

//***** Start mutate storage *****//

// Write asset metadat in new pallet_xcm_weight_trader
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Write asset metadat in new pallet_xcm_weight_trader
// Write asset metadata in new pallet_xcm_weight_trader

runtime/moonbeam/src/lib.rs Outdated Show resolved Hide resolved
runtime/moonbeam/tests/xcm_tests.rs Show resolved Hide resolved
runtime/common/src/apis.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@RomarQ RomarQ left a comment

Choose a reason for hiding this comment

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

Added some remarks

@noandrea noandrea added the A8-mergeoncegreen Pull request is reviewed well. label Sep 5, 2024
@librelois
Copy link
Collaborator Author

ts dev-tests suceed locally, there is something wrong with the dev-test CI job

@RomarQ
Copy link
Contributor

RomarQ commented Sep 6, 2024

ts dev-tests suceed locally, there is something wrong with the dev-test CI job

There are still 2 todos!() in the migrations file.

@RomarQ
Copy link
Contributor

RomarQ commented Sep 6, 2024

Apart from the todo!(), the changes look good 👍

@RomarQ
Copy link
Contributor

RomarQ commented Sep 9, 2024

The zombienet tests are failing with:

ERROR tokio-runtime-worker pallet_migrations::pallet: [🌗] Migration MM_MigrateXcmFeesAssetsMetadata consumed more weight than it was given! (Weight(ref_time: 18446744073709551615, proof_size: 18446744073709551615) > Weight(ref_time: 1999918717000, proof_size: 5242880))   

@librelois
Copy link
Collaborator Author

Migration MM_MigrateXcmFeesAssetsMetadata consumed more weight than it was given

This log make no sense anymore since we pause xcm and extrinsics at the block that execute migrations, so we do'nt care how many weights the migration consume as long as if fit in a block. That's why the migration return Weight::MAX

RomarQ
RomarQ previously approved these changes Sep 12, 2024
Agusrodri
Agusrodri previously approved these changes Sep 12, 2024
@librelois librelois merged commit dd3a4a4 into master Sep 13, 2024
38 of 39 checks passed
@librelois librelois deleted the elois-evm-foreign-fees branch September 13, 2024 15:06
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.

6 participants