open-runtime-module-library icon indicating copy to clipboard operation
open-runtime-module-library copied to clipboard

relay account and tests

Open zqhxuyuan opened this issue 4 years ago • 5 comments

  • add RelaychainAccountId32Aliases to xcm-support
  • add AllowRelayedPaidExecutionFromParent to xcm-support
  • add unit testcase
  • add integration test in acala: https://github.com/AcalaNetwork/Acala/pull/1614

AllowRelayedPaidExecutionFromParent is needed because in the case of relaychain account send Transact to parachain, then DescendOrigin is automatically added as the first instruction sended to parachain. so it's used as Barrier check to allow this situation passing through.

RelaychainAccountId32Aliases is used as convert relaychain account to account controlled on parachain. and it's related to above case when DescendOrigin append origin(which is Parent) with Account which result to (Parent, Account).

closes https://github.com/open-web3-stack/open-runtime-module-library/issues/636

zqhxuyuan avatar Nov 10 '21 09:11 zqhxuyuan

Codecov Report

Merging #651 (4177cd6) into master (d69f226) will decrease coverage by 0.03%. The diff coverage is 73.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #651      +/-   ##
==========================================
- Coverage   76.02%   75.99%   -0.04%     
==========================================
  Files          78       78              
  Lines        6557     6644      +87     
==========================================
+ Hits         4985     5049      +64     
- Misses       1572     1595      +23     
Impacted Files Coverage Δ
xtokens/src/mock/para.rs 52.77% <ø> (ø)
xtokens/src/tests.rs 100.00% <ø> (ø)
xcm-support/src/lib.rs 58.92% <50.00%> (-26.79%) :arrow_down:
xcm-support/src/tests.rs 93.97% <93.02%> (-1.03%) :arrow_down:
xtokens/src/mock/mod.rs 89.58% <100.00%> (+1.21%) :arrow_up:
benchmarking/src/lib.rs 30.08% <0.00%> (ø)
authority/src/mock.rs 75.51% <0.00%> (+2.04%) :arrow_up:
xcm/src/lib.rs 80.00% <0.00%> (+5.00%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d69f226...4177cd6. Read the comment docs.

codecov-commenter avatar Nov 10 '21 09:11 codecov-commenter

it is something to be used by all the runtime and potentially used by other parachains. Like https://github.com/paritytech/polkadot/blob/54d1cb9178fd41b535d8d908b021926d01d91f00/xcm/xcm-builder/src/location_conversion.rs#L104

xlc avatar Nov 10 '21 23:11 xlc

current the xcm format is WithdrawAsset | BuyExecution(limited) | Transact, this means the asset of relaychain acount(say 1 KSM) will all be first move to holding, then payment fee will be buyed for execution(say 0.1 KSM), after that unspent asset(0.9 KSM) will be in holding again. and As the xcm has its AssetTrap mechanism, the unspent asset will be lefted in the storage for later claim. Do we need add another DepositAsset to do this for users or let user to self-decide? @xlc

in my testcase the event below shows that there're 0.016 used as fee payment, and 0.98 KSM lefted in AssetsTrapped:

Event::Currencies(Event::Deposited(CurrencyId::Token(TokenSymbol::KSM), ... (5EYCAe5f...), 16128000000))

Event::PolkadotXcm(Event::AssetsTrapped(...., MultiLocation { parents: 1, interior: Here }, 
V1(MultiAssets([MultiAsset { id: Concrete(MultiLocation { parents: 1, interior: Here }),
fun: Fungible(983872000000) }]))))

zqhxuyuan avatar Nov 16 '21 08:11 zqhxuyuan

Yeah we should allow DepositAsset the unused fees back.

Also maybe should support ReserveAssetDeposited so it is possible to send some funds from relay chain for the tx fee. Otherwise user needs to fund the account on parachain first.

xlc avatar Nov 16 '21 22:11 xlc

Yeah we should allow DepositAsset the unused fees back.

Also maybe should support ReserveAssetDeposited so it is possible to send some funds from relay chain for the tx fee. Otherwise user needs to fund the account on parachain first.

it's easy add ReserveAssetDeposited to match case inside AllowRelayedPaidExecutionFromParent, but construct a xcm in relaychain side currently didn't work.

e.g. relaychain has xcm format TransferReserverAsset{xcm: BuyExecution | Transact | DepositAsset }, then parachain receives: ReserveAssetDeposited | ClearOrigin | BuyExecution | Transact | DepositAsset. As the Transact instruction need origin not null, but previous ClearOrigin set origin to None, this will cause BadOrigin error in parachain.

let alice = X1(Junction::AccountId32 {
    network: NetworkId::Kusama,
    id: ALICE.into(),
});
let call = Call::Balances(pallet_balances::Call::<Runtime>::transfer {
    dest: BOB.into(),
    value: 500,
});
Relay::execute_with(|| {
    let _ = RelayBalances::deposit_creating(&ALICE, 20_000);

    let xcm = vec![
        TransferReserveAsset {
            assets: (Here, 10_000).into(),
            dest: Parachain(1).into(),
            xcm: Xcm::<()>(vec![
                BuyExecution {
                    fees: (Parent, 10_000).into(),
                    weight_limit: Limited(6050 as u64),
                },
                Transact {
                    origin_type: OriginKind::SovereignAccount,
                    require_weight_at_most: 6_000 as u64,
                    call: call.encode().into(),
                },
                DepositAsset {
                    assets: All.into(),
                    max_assets: 1,
                    beneficiary: (0, alice.clone()).into(),
                }
            ]),
        },
    ];
    XcmExecutor::<relay::XcmConfig>::execute_xcm_in_credit(alice, Xcm(xcm), 10, 10);
});

also If change Transact to WithdrawAsset isn't going to work. because WithdrawAsset also needs origin is not null.

let alice = X1(Junction::AccountId32 {
    network: NetworkId::Kusama,
    id: ALICE.into(),
});
let bob = X1(Junction::AccountId32 {
    network: NetworkId::Kusama,
    id: BOB.into(),
});

Relay::execute_with(|| {
    let _ = RelayBalances::deposit_creating(&ALICE, 20_000);
    assert_eq!(21_000, RelayBalances::free_balance(&ALICE));

    let xcm = vec![
        TransferReserveAsset {
            assets: (Here, 10_000).into(),
            dest: Parachain(1).into(),
            xcm: Xcm::<()>(vec![
                BuyExecution {
                    fees: (Parent, 10_000).into(), // use relaychain asset as fee payment
                    weight_limit: Limited(6050 as u64),
                },
                WithdrawAsset((Here, 500).into()), // withdraw Alice on parachain
                // BuyExecution may add here too
                DepositAsset {
                    assets: All.into(),
                    max_assets: 1,
                    beneficiary: (0, bob).into(), // deposit to Bob on parachain
                }
            ]),
        },
    ];
    XcmExecutor::<relay::XcmConfig>::execute_xcm_in_credit(alice, Xcm(xcm), 10, 10);
});

if relaychain xcm format is WithdrawAsset | DepositReserveAsset{ xcm: BuyExecution | Transact | DepositAsset }, in the parachain it also has this problem too.

zqhxuyuan avatar Nov 18 '21 02:11 zqhxuyuan