Skip to content

Conversation

@girazoki
Copy link
Collaborator

@girazoki girazoki commented Jan 31, 2022

What does it do?

Enables transfer of MOVR through XCM to moonriver. For now it is not enabled in Moonbeam, but if suggested I can add it too.

Basically adds to AssetTransactors both XcmCurrencyAdapter for incoming assets matching SelfReserveRepresentations, which include OldAnchoringSelfReserve and NewAnchoringSelfReserve.

The first matches incoming assets to

pub OldAnchoringSelfReserve: MultiLocation = MultiLocation {
		parents:1,
		interior: Junctions::X2(
			Parachain(ParachainInfo::parachain_id().into()),
			PalletInstance(<Balances as PalletInfoAccess>::index() as u8)
		)
};

while the second matches incoming assets to:

pub NewAnchoringSelfReserve: MultiLocation = MultiLocation {
		parents:0,
		interior: Junctions::X1(
			PalletInstance(<Balances as PalletInfoAccess>::index() as u8)
		)
	};

Both represent our balances pallet, which is how we refer to our MOVR token. Both representations are necessary because of paritytech/polkadot#4470, which was introduced in 0.9.16 and which changes the way we receive our own token from absolute to relative view. Hence, we need to handle both pre and post 0.9.16 versions.

Additionally, we change the usage of PalletInfo trait to PalletInfoAccess, because the latter does not use unwraps.

Also, tests are added to runtime/moonriver/xcm_tests.ts sending the MOVR token.

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

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

What value does it bring to the blockchain users?

@girazoki girazoki added A0-pleasereview Pull request needs code review. 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. labels Jan 31, 2022
…post-0.9.16-anchoring-logics-for-native-token' into girazoki-enable-native-asset-cross-chain-transfer-in-moonriver
@girazoki girazoki marked this pull request as ready for review February 1, 2022 08:54
@girazoki girazoki changed the base branch from girazoki-support-both-pre-and-post-0.9.16-anchoring-logics-for-native-token to master February 11, 2022 10:06
…ative-asset-cross-chain-transfer-in-moonriver
@crystalin crystalin requested a review from nanocryk February 14, 2022 16:30
@librelois librelois mentioned this pull request Feb 17, 2022
20 tasks

pub const TRANSACTION_BYTE_FEE: Balance = 10 * MICROMOVR * SUPPLY_FACTOR;
pub const STORAGE_BYTE_FEE: Balance = 100 * MICROMOVR * SUPPLY_FACTOR;
pub const WEIGHT_FEE: Balance = 50 * KILOWEI * SUPPLY_FACTOR;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should do some basic testing to ensure this is reasonable for your use case. It appears to be taken from #1218, which is still a WIP.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right in the sense that a token transfer through XCM would cost 50_000_000_000_000, or essentially, 50microMOVR. Maybe this is too low since for KSM we are charging 0.1KSM. I will increase it to match it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I increased it to 100KILOWEI. This means that for 1e9 weight (roughly, the amount needed for a token transfer in XCM) we charge 0.1milliMOVR, which I think gets us closer to what we charge for other assets

Copy link
Contributor

@notlesh notlesh left a comment

Choose a reason for hiding this comment

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

Nothing looks wrong to me here, but I don't have much context for the change that you're making. Maybe a short description of the PR (and any links to related upstream PRs) would help.

@girazoki
Copy link
Collaborator Author

Updated the description, let me know if it feels better

@girazoki girazoki requested a review from librelois February 22, 2022 12:06
@girazoki girazoki merged commit 75e1dc7 into master Feb 22, 2022
@girazoki girazoki deleted the girazoki-enable-native-asset-cross-chain-transfer-in-moonriver branch February 22, 2022 18:08
@notlesh notlesh added D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants