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.

Sign up for GitHub

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

Jump to bottom

Integration of pallet orlm xtoken #701

Merged
merged 23 commits into from Mar 3, 2023
Merged

Integration of pallet orlm xtoken #701

merged 23 commits into from Mar 3, 2023

Conversation

Hounsette
Copy link
Contributor

@Hounsette Hounsette commented Feb 1, 2023 •
edited

  • CurrencyId conversion for incoming xcm (hard coded)

@Hounsette Hounsette changed the title Integration of pallet orlm xtoken [WIP] Integration of pallet orlm xtoken Feb 1, 2023
@codecov
Copy link

codecov bot commented Feb 27, 2023 •
edited

Codecov Report

Merging #701 (3e15ace) into master (5632a16) will decrease coverage by 0.02%.
The diff coverage is 64.70%.

@@            Coverage Diff             @@
##           master     #701      +/-   ##
==========================================
- Coverage   78.20%   78.18%   -0.02%     
==========================================
  Files          43       43              
  Lines        7580     7596      +16     
==========================================
+ Hits         5928     5939      +11     
- Misses       1652     1657       +5
Impacted Files Coverage Δ
runtimes/eden/src/lib.rs 2.94% <ø> (ø)
runtimes/eden/src/xcm_config.rs 52.38% <64.70%> (+52.38%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Hounsette Hounsette changed the title [WIP] Integration of pallet orlm xtoken Integration of pallet orlm xtoken Feb 27, 2023
aliXsed
aliXsed reviewed Feb 28, 2023
View reviewed changes
runtimes/eden/src/xcm_config.rs
type MaxAssetsForTransfer = MaxAssetsForTransfer;
type MinXcmFee = ParachainMinFee;
type MultiLocationsFilter = Everything;
type ReserveProvider = RelativeReserveProvider;
Copy link
Contributor

Choose a reason for hiding this comment

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

If AbsoluteReserveProvider can work here, it could be a better choice as it's not altering the way a local/native asset is addressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am having an error with Asset has no reserve error when switching to an absolute reserver provider. Must be something missing in the config.

simonsso
simonsso reviewed Mar 2, 2023
View reviewed changes
runtimes/eden/Cargo.toml Outdated Show resolved Hide resolved
runtimes/eden/src/xcm_config.rs
// One XCM operation is 1_000_000_000 weight - almost certainly a conservative estimate.
pub UnitWeightCost: u64 = 1_000_000_000;
// One XCM operation is 200_000_000 weight - almost certainly a conservative estimate.
pub UnitWeightCost: u64 = 200_000_000;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a weight that should be benchmarked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

@aliXsed @simonsso
Update ParachainMinFee
3e15ace 
Co-authored-by: Fredrik Simonsson <fredrik@nodle.com>
aliXsed
aliXsed approved these changes Mar 3, 2023
View reviewed changes
Copy link
Contributor

@aliXsed aliXsed left a comment •
edited

Choose a reason for hiding this comment

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

LGTM. There will be a separate PR for benchmarking.

@Hounsette Hounsette merged commit 5121804 into master Mar 3, 2023
@Hounsette Hounsette deleted the houns-orlm-xtoken branch March 3, 2023 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonsso simonsso simonsso left review comments

@aliXsed aliXsed aliXsed approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Hounsette @simonsso @aliXsed