[DNM] Coretime revenue: asset teleport draft #4811
Conversation
| @@ -88,6 +88,17 @@ impl WeightInfo for TestWeightInfo { | |||
| } | |||
| } | |||
|
|
|||
| pub struct RevenueClaim<T: Config> { | |||
There was a problem hiding this comment.
Some docs would be good. What is the purpose of this struct? What is a split_off? ..
| check_origin: None, | ||
| }, | ||
| ReceiveTeleportedAsset(parent_assets.clone()), | ||
| BurnAsset(parent_assets), |
There was a problem hiding this comment.
Nice, LGTM. We just had one level of optimisation to try to minimise the XCMs by burning at some fixed period, instead of being done at the point of revenue coming in. So at the minute on Kusama we dump it into an account on unbalanced, then with the upcoming coretime-allocator pallet we'll burn the contents of the pot - this is the XCM we'll use for it though. I'm sure this is fine on Rococo and Westend and we can update when the new pallet is ready
There was a problem hiding this comment.
In fact, Rococo and Westend are probably fine with the old CreditToCollatorPot for now
| type SendXcm: SendXcm; | ||
| /// The asstet transactor. |
There was a problem hiding this comment.
| /// The asstet transactor. | |
| /// The asset transactor. |
| let parent_assets: Assets = | ||
| vec![Asset { id: AssetId(Location::here()), fun: Fungible(value) }].into(); | ||
|
|
||
| let send_res = PolkadotXcm::send_xcm( |
There was a problem hiding this comment.
Does the check_out logic get called somewhere else before this?
There was a problem hiding this comment.
Not sure I follow. It's the parachain side, so we don't have to check out assets here IIUC. They're checked out at the relay chain, and here, we want the RC to check them back in and burn them immediately.
There was a problem hiding this comment.
Yeah I understand, but it's up to the implementation of AssetTransactor.
Follow the code paths in do_teleport_assets. You will see it ends up executing a teleport_assets_program locally. IMO the system teleporting assets should behave like other teleports. If the Config::AssetTransactor changes in a runtime upgrade, it will be strange if these are accounted for differently.
We probably need some refactor to make system teleports more ergonomic with less code duplication, but I would treat these like other teleports for now (except that we can get away with UnpaidExecution and extra privilege here). cc @franciscoaguirre @acatangiu
There was a problem hiding this comment.
yeah I was thinking the same, but the first bump in the road is that both InitiateTeleport XCM instructions and/or pallet-xcm/xtokens/etc build XCMs assuming BuyExecution and do not "out of the box" support UnpaidExecution.
We should def fix that, but I can't think of a way to do it without breaking pallet APIs (and/or without new XCM version) (as I'm writing this, obviously we can add new unpaid transfer extrinsics - ugly, yes)
I will file an issue
There was a problem hiding this comment.
Opened #4854 - lemme know if anyone can pick it up, or I should add it to our priority queue.
|
The CI pipeline was cancelled due to failure one of the required jobs. |
|
Merged into #3940 |
No description provided.