feat: enable bridged reserves from AssetHub #784

dmoka · 2024-03-12T15:13:14Z

Description

Assets bridged from other consensus systems using the new Bridge Hub system parachain are held in reserve in Asset Hub. Asset Hub contains a second instance of the pallet-assets called the foreignAssets where bridged assets are registered. Foreign assets have a parents field > 1.

Also adding reserve support for DOT coming from AssetHub.

Related Issue

Motivation and Context

Make swaps and pools using foreign assets on HydraDX, specifically assets from Snowbridge. https://github.com/Snowfork/snowbridge

How Has This Been Tested?

TODO: Add a new integration test to integration-tests/src/cross_chain_transfer.rs which tests receiving a foreign asset from assethub.

Checklist:

I have updated the documentation if necessary.
I have added tests to cover my changes, regression test if fixing an issue.

…hydra

github-actions · 2024-03-12T15:14:52Z

Crate versions that have been updated:

runtime-integration-tests: v1.21.4 -> v1.21.5
hydradx-runtime: v234.0.0 -> v235.0.0

Runtime version has been increased.

…e test network

…sset hub

alistair-singh

Thanks for picking up this PR. I left some comments about the specifics of the XCM program we are sending.

alistair-singh · 2024-03-14T11:31:08Z

integration-tests/src/cross_chain_transfer.rs

+		//ReserveAssetDeposited(assets.clone()),
+		BuyExecution { fees, weight_limit },
+		DepositAsset {
+			assets: Wild(AllCounted(max_assets)),


Here is the exact xcm we send. https://github.com/Snowfork/polkadot-sdk/blob/snowbridge/bridges/snowbridge/primitives/router/src/inbound/mod.rs#L259-L271

We reserve deposit both DOT and asset being transferred. We always send both DOT and the asset together in one xcm program.

Buy execution with DOT only.

Deposit the transfer asset only.

Thanks for the clarification @alistair-singh .

I am planning to send this message then:

// executed on local (AssetHub) let message = Xcm(vec![ WithdrawAsset(vec![dot_as_fee.clone(), assets.clone().into()].into()), DepositReserveAsset { assets: Definite(vec![dot_as_fee.clone(), assets.clone().into()].into()), dest, xcm: Xcm(vec![ // executed on remote (on hydra) BuyExecution { fees: dot_as_fee, weight_limit, }, DepositAsset { assets: Definite(assets.into()), beneficiary, }, ]), }, ]);

Do you think it represents your usecase? At first I really need to do a WithdrawAsset, otherwise I have nothing in the holding.

This looks correct and supports our case.

WithdrawAsset is one way to get assets in holding, but then you would need to create the asset, mint it and send some to a user before you execute the message as that user. Another way you can get assets into holding is by abusing the ReceiveTeleportedAsset and ReserveAssetDeposited instructions to force the assets into holding.

alistair-singh · 2024-03-14T11:31:15Z

integration-tests/src/cross_chain_transfer.rs

+
+	let max_assets = assets.len() as u32 + 1;
+	let context = X2(GlobalConsensus(NetworkId::Polkadot), Parachain(ACALA_PARA_ID));
+	let fees = assets


Fee will always be DOT. We always buy execution with DOT.

alistair-singh · 2024-03-14T11:33:23Z

integration-tests/src/cross_chain_transfer.rs

+		let foreign_asset: MultiAssets = MultiAsset::from((
+			MultiLocation {
+				parents: 1,
+				interior: Junctions::Here,
+			},
+			100 * UNITS,
+		))
+		.into();


We will never transfer DOT on its own. We will always transfer DOT and the asset selected by the user. The DOT is only there to pay for execution.

This is a different use case from the foreign asset case, here we want to allow AssetHub to send DOT directly.

So we just implemented it in the same PR.

Maybe @green-jay can hop in here, as he requested this feature from me.

alistair-singh

Looks good!

green-jay · 2024-03-21T16:33:07Z

@dmoka can you please add test of sending DOT from HydraDX to Asset Hub?

…tWithFee

jak-pan · 2024-04-09T08:50:44Z

Can we pls remove the DOT<>AH reserve chain and merge the original PR?

dmoka · 2024-04-09T10:35:20Z

@alistair-singh After some discussion with our team members, we realized that we don't want to allow DOT reserve deposited in our chain because of security concerns.

We still want to accept foreign assets tho. In your code you said you would reserve deposit the foreign asset together with DOT as fee, so it would not be the best for us. What do you think, would it be possible do the same thing on your side but without reserve depositing DOT?

alistair-singh · 2024-04-10T19:49:27Z

We still want to accept foreign assets tho. In your code you said you would reserve deposit the foreign asset together with DOT as fee, so it would not be the best for us. What do you think, would it be possible do the same thing on your side but without reserve depositing DOT?

Hey @dmoka, The issue is that we need to pay for execution some how and as a bridge we have three options to pay for execution on the destination parachain (HydraDX in this case is the destination).

Pay for execution with DOT.
Pay for execution with the Asset that is being transferred via ReserveDeposit. Requires a Trader on your xcm-executor?
Allow unpaid execution from AssetHub. (Probably not an option)

So we currently chose DOT as it is the only thing we can guarantee most chains will accept for payment. Is the issue that we are using ReserveDepositAsset for DOT instead of Teleporting DOT?

alistair-singh · 2024-04-11T10:17:01Z

@dmoka To help my understanding of the core issue, why do we consider allowing DOT as a reserve a security risk?

I also want to note that the DOT being transferred here is not a bridged or wrapped version of DOT from Ethereum. It is from Asset Hub's sovereign account on the Bridge Hub parachain. This account and its funds do not leave the security domain provided by the relaychain and only exists to pay bridge-related fees. It will be topped up by governance regularly from the treasury so it is a closed loop.

jak-pan · 2024-04-19T14:05:47Z

To summarize also offchain conversations,

...we can't currently allow AH to be DOT reserve because it breaks reserve backed transfer accounting. 
Imagine Alice sending 1000 DOT from Polkadot to HydraDX and then trying to withdraw to AH. 
She can't because our parachain accont doesn't have 1000DOT. 
Teleporting is not an option as AH + Polkadot would need to trust us as reserve chain.

You can only send DOT through it's reserve chain currently which is Polkadot,
and I guess this will be the case for all of the parachains

So the only viable option ATM is to get some asset that is supported (not DOT) on AH and transfer this with the message.
I guess we accept most of the common assets (all of the Omnipool assets) as fee payment asset.

The proposed solution for now is to use the transported asset as fee payment asset. For this to work we need to register this asset as sufficient / fee payment asset.

For other assets we can still support them but it would need to end at AH and users would need to transport to hydra from there using some other sufficient asset as fee.

Alternatively use assetConversion on AH to change to any compatible asset to pay the fees and ED. i.e. HDX

How should we proceed?

alistair-singh · 2024-04-22T10:17:57Z

How should we proceed?

Since you cannot allow Asset Hub as a reserve we cannot use our "Forward assets to destination parachain" functionality as it requires Asset Hub to be the reserve for DOT. We will probably patch this feature in the future to accept either the destination chain's native asset or the asset being transferred.

This means now that users must do 2 transfers to get assets to Hydra. One transfer from Ethereum to Asset Hub using the bridge. The second transfer using pallet-xcm to transfer the asset from Asset Hub to Hydra.

With that in mind, I think from the perspective of this PR we need to support the following cases for Users:

Using the asset being transferred as fee if it is sufficient.
Using your native asset (Hydra) as fee.
Any existing asset that is sufficient (USDT?) as fee.

Are all these cases supported from Asset Hub?

alistair-singh · 2024-04-22T22:13:40Z

@jak-pan related discussions: https://forum.polkadot.network/t/managing-sas-on-multiple-reserve-chains-for-same-asset/7538

paritytech/polkadot-sdk#3695 (comment)

jak-pan · 2024-04-23T02:22:53Z

I have chimed in the discussion and we'll check these cases but they should be supported.

dmoka · 2024-04-26T14:00:47Z

So for now as the discussions are ongoing and might take for a while, we are going to get rid of the support of accepting DOT as reserve asset.

And going to merge the PR with just accepting foreign currencies.

@alistair-singh Let me know if you have objection on this.

alistair-singh · 2024-04-29T22:21:22Z

runtime/hydradx/src/xcm.rs

+			&& matches!(
+				asset,
+				MultiAsset {
+					id: Concrete(MultiLocation { parents: 2, .. }),


This filter is potentially too open. Maybe checking that the interior of the multi-location starts with GlobalConsensus(Ethereum{..}) and matches any chain_id, rather than accept any asset with Parents > 1. This will limit the assets to Snowbridge assets as opposed to any asset. But this is just a suggestion.

I don't think we really have issue with accepting any asset. You'll need to pay fees anyway and we allow to permissionlessly register any token for users already. Unless there can be some way to exploit this on xcm level...

Do you see any potential risks from top of your head?

cc @mrq1911

alistair-singh · 2024-04-29T22:22:15Z

Let me know if you have objection on this.

No objections. I think this is the right way forward for now.

green-jay · 2024-05-07T07:57:49Z

LGTM

runtime/hydradx/src/xcm.rs

alistair-singh

LGTM

alistair-singh and others added 4 commits February 28, 2024 00:57

fmt

cb403f1

added comments

6a7e12a

add integration test for transfering reserve asset from asset hub to …

7c34496

…hydra

merge master to alistair/enable-bridged-reserves

5b107bb

dmoka self-assigned this Mar 12, 2024

dmoka mentioned this pull request Mar 12, 2024

feat: Allow Asset Hub as a reserve for Foreign Assets #774

Closed

3 tasks

dmoka added 6 commits March 12, 2024 16:20

replace tests to correct test file

fb78b0c

bump versions

099c9a4

the relay chain number increases as we added one more parachain to th…

15999d5

…e test network

add test checking that we dont accept reserve from other chain than a…

b7e033e

…sset hub

Add reserve check for DOT coming from asset hub

2d255d0

adjust comment

ec2f316

alistair-singh reviewed Mar 14, 2024

View reviewed changes

dmoka added 3 commits March 14, 2024 15:57

replicate the real usecase from asset hub

7065a2d

fix test to use more realistic xcm calls coming from asset hub

a3be1ab

merge master to alistair/enable-bridged-reserves

50d0546

alistair-singh approved these changes Mar 20, 2024

View reviewed changes

bump min fee because it is not enough to do XTokens.TransferMultiAsse…

0fffd06

…tWithFee

alistair-singh suggested changes Apr 29, 2024

View reviewed changes

dmoka added 2 commits May 1, 2024 10:05

remove accepting DOT From AH

5c7acc0

merge master to alistair/enable-bridged-reserves

ebb7141

mrq1911 reviewed May 8, 2024

View reviewed changes

runtime/hydradx/src/xcm.rs Outdated Show resolved Hide resolved

dmoka added 2 commits May 14, 2024 10:42

merge master to alistair/enable-bridged-reserves

b3cc5c5

revert back min parachan min fee as we don't want to send dot to AH

cbdbd5d

alistair-singh approved these changes May 20, 2024

View reviewed changes

dmoka merged commit e747819 into master May 20, 2024
4 checks passed

dmoka deleted the alistair/enable-bridged-reserves branch May 20, 2024 13:41

ggonzalez94 mentioned this pull request Sep 2, 2024

🎁 [Feature Request]: Snowbridge Integration OpenZeppelin/polkadot-runtime-templates#303

Closed

3 tasks

ozgunozerk mentioned this pull request Sep 9, 2024

🎁 [Feature Request]: Accept asset hub as a reserve for bridged assets (EVM) OpenZeppelin/polkadot-runtime-templates#308

Closed

3 tasks

ozgunozerk mentioned this pull request Oct 18, 2024

🎁 [Feature Request]: Accept asset hub as a reserve for bridged assets (Generic) OpenZeppelin/polkadot-runtime-templates#336

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: enable bridged reserves from AssetHub #784

feat: enable bridged reserves from AssetHub #784

dmoka commented Mar 12, 2024 •

edited

Loading

github-actions bot commented Mar 12, 2024 •

edited

Loading

alistair-singh left a comment

alistair-singh Mar 14, 2024

dmoka Mar 14, 2024 •

edited

Loading

alistair-singh Mar 20, 2024

alistair-singh Mar 14, 2024

alistair-singh Mar 14, 2024

dmoka Mar 14, 2024

dmoka Mar 14, 2024

alistair-singh left a comment

green-jay commented Mar 21, 2024

jak-pan commented Apr 9, 2024

dmoka commented Apr 9, 2024

alistair-singh commented Apr 10, 2024

alistair-singh commented Apr 11, 2024

jak-pan commented Apr 19, 2024 •

edited

Loading

alistair-singh commented Apr 22, 2024

alistair-singh commented Apr 22, 2024

jak-pan commented Apr 23, 2024

dmoka commented Apr 26, 2024

alistair-singh Apr 29, 2024 •

edited

Loading

jak-pan Apr 29, 2024

alistair-singh commented Apr 29, 2024

green-jay commented May 7, 2024

alistair-singh left a comment

feat: enable bridged reserves from AssetHub #784

feat: enable bridged reserves from AssetHub #784

Conversation

dmoka commented Mar 12, 2024 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Checklist:

github-actions bot commented Mar 12, 2024 • edited Loading

alistair-singh left a comment

Choose a reason for hiding this comment

alistair-singh Mar 14, 2024

Choose a reason for hiding this comment

dmoka Mar 14, 2024 • edited Loading

Choose a reason for hiding this comment

alistair-singh Mar 20, 2024

Choose a reason for hiding this comment

alistair-singh Mar 14, 2024

Choose a reason for hiding this comment

alistair-singh Mar 14, 2024

Choose a reason for hiding this comment

dmoka Mar 14, 2024

Choose a reason for hiding this comment

dmoka Mar 14, 2024

Choose a reason for hiding this comment

alistair-singh left a comment

Choose a reason for hiding this comment

green-jay commented Mar 21, 2024

jak-pan commented Apr 9, 2024

dmoka commented Apr 9, 2024

alistair-singh commented Apr 10, 2024

alistair-singh commented Apr 11, 2024

jak-pan commented Apr 19, 2024 • edited Loading

alistair-singh commented Apr 22, 2024

alistair-singh commented Apr 22, 2024

jak-pan commented Apr 23, 2024

dmoka commented Apr 26, 2024

alistair-singh Apr 29, 2024 • edited Loading

Choose a reason for hiding this comment

jak-pan Apr 29, 2024

Choose a reason for hiding this comment

alistair-singh commented Apr 29, 2024

green-jay commented May 7, 2024

alistair-singh left a comment

Choose a reason for hiding this comment

dmoka commented Mar 12, 2024 •

edited

Loading

github-actions bot commented Mar 12, 2024 •

edited

Loading

dmoka Mar 14, 2024 •

edited

Loading

jak-pan commented Apr 19, 2024 •

edited

Loading

alistair-singh Apr 29, 2024 •

edited

Loading