Transact from sub to eth

[yrong]

No description provided.

[codecov]

Codecov Report

Attention: Patch coverage is 10.34483% with 26 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (bridge-next-gen@3f495e5). Click here to learn what that means.

❗ Current head 07ea7e8 differs from pull request most recent head ebcff31. Consider uploading reports for the commit ebcff31 to get more accurate results

Files	Patch %	Lines
...s/snowbridge/primitives/router/src/outbound/mod.rs	14.28%	18 Missing ⚠️
bridges/snowbridge/primitives/core/src/outbound.rs	0.00%	8 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##             bridge-next-gen     #116   +/-   ##
==================================================
  Coverage                   ?   40.23%           
==================================================
  Files                      ?       51           
  Lines                      ?     3363           
  Branches                   ?        0           
==================================================
  Hits                       ?     1353           
  Misses                     ?     2010           
  Partials                   ?        0           

Flag	Coverage Δ
rust	40.23% <10.34%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[alistair-singh]

Charge fees sends it to the treasury account. Is this the behavior we want for the delivery fees?

[yrong]

It's for local execution on Penpal. On BH For delivery we charge from sovereign of penpal in DOT.

2024-04-22T04:24:47.085187Z TRACE xcm::process: origin: Some(Location { parents: 1, interior: X1([Parachain(2000)]) }), total_surplus/refunded: Weight { ref_time: 0, proof_size: 0 }/Weight { ref_time: 0, proof_size: 0 }, error_handler_weight: Weight { ref_time: 0, proof_size: 0 }
2024-04-22T04:24:47.085200Z TRACE xcm::process_instruction: === WithdrawAsset(Assets([Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(4000000000) }]))
2024-04-22T04:24:47.085209Z TRACE xcm::ensure_can_subsume_assets: worst_case_holding_len: 1, holding_limit: 64
2024-04-22T04:24:47.085250Z TRACE xcm::fungible_adapter: withdraw_asset what: Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(4000000000) }, who: Location { parents: 1, interior: X1([Parachain(2000)]) }
2024-04-22T04:24:47.085368Z TRACE xcm::process_instruction: === BuyExecution { fees: Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(4000000000) }, weight_limit: Limited(Weight { ref_time: 729581493, proof_size: 13316 }) }
2024-04-22T04:24:47.085385Z TRACE xcm::weight: UsingComponents::buy_weight weight: Weight { ref_time: 729581493, proof_size: 13316 }, payment: AssetsInHolding { fungible: {AssetId(Location { parents: 1, interior: Here }): 4000000000}, non_fungible: {} }, context: XcmContext { origin: Some(Location { parents: 1, interior: X1([Parachain(2000)]) }), message_id: [106, 51, 55, 199, 155, 107, 230, 240, 194, 5, 253, 187, 40, 174, 94, 95, 214, 45, 219, 36, 8, 138, 134, 194, 34, 165, 69, 28, 207, 132, 148, 30], topic: None }

Basiclly it follows the same pattern transfering asset from substrate to Ethereum.

@alistair-singh So what's your preference here?

[claravanstaden]

Why is Weight::default() ok to use here? 🤔 I know this is just an example, but would parachains have to set this to a sensible value?

[yrong]

Actually we don't execute the transact on BH, it's a abi-encoded call for Ethereum so the weight here make no sense.

[alistair-singh]

Using XCMRouter directly and manually collecting fees is too low level.
We can use PolkadotXcm::send_xcm which will automatically collect fees from the User.

[yrong]

For the transact there are 2 portions of the fees collected from the User(i.e. local on BH&remote on Ethereum).
The local part is a configuration on source chain and the remote fee on Ethereum is dynamic as the parameter of the extrinsic.

So parachain needs to explicitly charge_fees from the user including the both, PolkadotXcm::send_xcm won't cover that in this case.

polkadot-sdk/cumulus/parachains/runtimes/testing/penpal/src/pallets/transact_helper.rs

Lines 128 to 134 in 6cc2175

	let mut fees: Assets = (Parent, fee).into();
	fees.push(price.get(0).unwrap().clone());
	let origin = Location::from(AccountId32 { network: None, id: who.into() });
	<T as Config>::XcmExecutor::charge_fees(origin, fees.clone().into())
	.map_err(|_| Error::<T>::FeesNotMet)?;

Anyway, this is just a refence impl for the transact. The parachain team can implement a custom extrinsic as they want.

[alistair-singh]

Yeah so PolkadotXcm::send_xcm would only handle delivery fees sent to the parachains configured treasury. How execution fees are collected are left to the app layer.