Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support system to bridge txs #383

Merged
merged 36 commits into from
May 20, 2024

Conversation

marshacb
Copy link
Contributor

@marshacb marshacb commented Mar 14, 2024

Description

This PR introduces the SystemToBridge tx direction for constructing BridgeHub transactions from AssetHub.

Changes

  • adds support for the transferAssetsUsingTypeAndThen extrinsic.
  • adds SystemToBridge direction.
  • In addition to allowing Parachain IDs, the API now allows for a GlobalConsensus location (e.g. {"parents":"2","interior":{"X1":{"GlobalConsensus":{"Ethereum":{"chainId":"11155111"}}}}}) to be provided as a value for thedestChainId parameter in createTransferTransaction. If a destChainId is provided containing a known X1 GlobalConsensus junction it will be considered a Bridge tx. Currently known consensus junctions are set to polkadot, kusama, westend, rococo, and ethereum.
  • Allows for registered global consensus asset locations on AssetHub (e.g. {"parents":"2","interior":{"X2":[{"GlobalConsensus":{"Ethereum":{"chainId":"11155111"}}},{"AccountKey20":{"network":null,"key":"0xfff9976782d46cc05630d1f6ebab18b2324d6b14"}}]}} on AssetHub Rococo) to be provided as assetIds in createTransferTransaction.
  • Adds error validation checks for SystemToBridge direction
  • Adds unit and integration tests for SystemToBridge direction
  • Removed unnecessary check restricting generalKey locations to XCM version 2 in resolveMultiLocation

BridgeHub Snowbridge: (Rococo AssetHub WETH -> Ethereum Sepolia)

Example of constructing a transferAssets call sending Snowbridge WETH to Ethereum Sepolia from Rococo AssetHub:

	const { api, specName, chainName, safeXcmVersion } = await constructApiPromise(
		'wss://rococo-asset-hub-rpc.polkadot.io',
	);
	const assetApi = new AssetTransferApi(api, specName, safeXcmVersion, {
		chainName,
	});

	let callInfo = await assetApi.createTransferTransaction(
			`{"parents":"2","interior":{"X1":{"GlobalConsensus":{"Ethereum":{"chainId":"11155111"}}}}}`,
			'0x6E733286C3Dc52C67b8DAdFDd634eD9c3Fb05B5B',
			[
				`{"parents":"2","interior":{"X2":[{"GlobalConsensus":{"Ethereum":{"chainId":"11155111"}}},{"AccountKey20":{"network":null,"key":"0xfff9976782d46cc05630d1f6ebab18b2324d6b14"}}]}}`,
			],
			['1000000000000'],
			{
				format: 'call',
				xcmVersion: 4,
			},
		);

		console.log(callInfo);

Expected Output

{
  origin: 'statemine',
  dest: 'ethereum',
  direction: 'SystemToBridge',
  xcmVersion: 4,
  method: 'transferAssets',
  format: 'call',
  tx: '0x1f0b04020109079edaa80204000103006e733286c3dc52c67b8dadfdd634ed9c3fb05b5b0404020209079edaa8020300fff9976782d46cc05630d1f6ebab18b2324d6b1400070010a5d4e80000000000'
}

The following decoded tx:
 {
    "args": {
        "dest": {
            "V4": {
                "parents": "2",
                "interior": {
                    "X1": [
                        {
                            "GlobalConsensus": {
                                "Ethereum": {
                                    "chainId": "11,155,111"
                                }
                            }
                        }
                    ]
                }
            }
        },
        "beneficiary": {
            "V4": {
                "parents": "0",
                "interior": {
                    "X1": [
                        {
                            "AccountKey20": {
                                "network": null,
                                "key": "0x6e733286c3dc52c67b8dadfdd634ed9c3fb05b5b"
                            }
                        }
                    ]
                }
            }
        },
        "assets": {
            "V4": [
                {
                    "id": {
                        "parents": "2",
                        "interior": {
                            "X2": [
                                {
                                    "GlobalConsensus": {
                                        "Ethereum": {
                                            "chainId": "11,155,111"
                                        }
                                    }
                                },
                                {
                                    "AccountKey20": {
                                        "network": null,
                                        "key": "0xfff9976782d46cc05630d1f6ebab18b2324d6b14"
                                    }
                                }
                            ]
                        }
                    },
                    "fun": {
                        "Fungible": "1,000,000,000,000"
                    }
                }
            ]
        },
        "fee_asset_item": "0",
        "weight_limit": "Unlimited"
    },
    "method": "transferAssets",
    "section": "polkadotXcm"
}

BridgeHub Polkadot<>Kusama Bridge: (Kusama AssetHub KSM -> Polkadot AssetHub)

Example of constructing a transferAssets call sending KSM to Polkadot AssetHub from Kusama AssetHub:

const { api, specName, chainName, safeXcmVersion } = await constructApiPromise(
		'wss://kusama-asset-hub-rpc.polkadot.io',
	);
	const assetApi = new AssetTransferApi(api, specName, safeXcmVersion, {
		chainName,
	});

let callInfo = await assetApi.createTransferTransaction(
			`{"parents":"2","interior":{"X1":{"GlobalConsensus":"Polkadot"}}}`,
			'5EWNeodpcQ6iYibJ3jmWVe85nsok1EDG8Kk3aFg8ZzpfY1qX',
			[`{"parents":"1","interior":{"Here":""}}`],
			['1000000000000'],
			{
				format: 'call',
				xcmVersion: 4,
			},
		);

		console.log(callInfo);

Expected Output:

{
  origin: 'statemine',
  dest: 'polkadot',
  direction: 'SystemToBridge',
  xcmVersion: 4,
  method: 'transferAssets',
  format: 'call',
  tx: '0x1f0b040202090200a10f04000101006c0c32faf970eacb2d4d8e538ac0dab3642492561a1be6f241c645876c056c1d04040100000700e87648170000000000'
}

The following decoded tx:
 {
    "args": {
        "dest": {
            "V4": {
                "parents": "2",
                "interior": {
                    "X2": [
                        {
                            "GlobalConsensus": "Polkadot"
                        },
                        {
                            "Parachain": "1,000"
                        }
                    ]
                }
            }
        },
        "beneficiary": {
            "V4": {
                "parents": "0",
                "interior": {
                    "X1": [
                        {
                            "AccountId32": {
                                "network": null,
                                "id": "0x6c0c32faf970eacb2d4d8e538ac0dab3642492561a1be6f241c645876c056c1d"
                            }
                        }
                    ]
                }
            }
        },
        "assets": {
            "V4": [
                {
                    "id": {
                        "parents": "1",
                        "interior": "Here"
                    },
                    "fun": {
                        "Fungible": "100,000,000,000"
                    }
                }
            ]
        },
        "fee_asset_item": "0",
        "weight_limit": "Unlimited"
    },
    "method": "transferAssets",
    "section": "polkadotXcm"
}

Note

  • If a given asset location doesn’t exist in the ForeignAssets pallet of the origin chain the API will throw an AssetNotFound error
  • Currently assumes the user is sending an asset that is known/valid from the context of the destination chain (checks that the asset exists on chain for the origin)
  • Providing the assetTransferType option will resolve the call to transferAssetsUsingTypeAndThen if it is found in the runtime. When providing a value of RemoteReserve for assetTransferType or feesTransferType, a location value is expected for remoteReserveAssetTransferTypeLocation and/or remoteReserveFeesTransferTypeLocation respectively.

related issue: #382
closes: #400

add unit tests
add util functions for determining direction based on asset and dest locations
@IkerAlus
Copy link
Contributor

great to see this out already!

One request @marshacb : once this is ready for review, it would be really helpful if you add details to the original post of the differences in UX this feature implies (for example, destinations as multilocations and so on)

…ocations

add SystemToBridge integration tests
updated getGlobalConsensusSystemName to check X1 junction rather than entire string for valid consensus name
added consensus name check to xcm tx input checks for system to bridge direction
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/parity-tech-update-for-march/7226/1

add AssetTransferType and resolveTransferType util func
Copy link

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

Output/result looks good 👍

Does it support P<>K bridge as well?

@marshacb
Copy link
Contributor Author

marshacb commented May 9, 2024

Output/result looks good 👍

Does it support P<>K bridge as well?

Hey @acatangiu, thanks for checking this out! Yes, this feature will support P<>K bridge transaction construction as well.
Just to note, this is currently being reworked to support the newer transfer_assets_using_type_and_then call for bridge transfer cases where AssetHub is required to be treated as the remote reserve.

@marshacb marshacb changed the title feat: support system to bridge txs[WIP] feat: support system to bridge txs May 16, 2024
@marshacb marshacb requested review from TarikGul and bee344 May 16, 2024 12:27
Copy link
Contributor

@IkerAlus IkerAlus left a comment

Choose a reason for hiding this comment

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

Couple of general comments:

  • Can we add an example where we send (bridge) the native token of another ecosystem back to its native ecosystem? For example ROC from Westend AH back to Rococo AH?
  • Any reason in particular to have the example with Trappist? In principle, it is normal foreign asset transfer from AH which is already covered in other examples.

Copy link
Contributor

@bee344 bee344 left a comment

Choose a reason for hiding this comment

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

Two nits but great work

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@marshacb
Copy link
Contributor Author

Couple of general comments:

  • Can we add an example where we send (bridge) the native token of another ecosystem back to its native ecosystem? For example ROC from Westend AH back to Rococo AH?
  • Any reason in particular to have the example with Trappist? In principle, it is normal foreign asset transfer from AH which is already covered in other examples.
  • I added examples of bridging DOT back to Polkadot from Kusama (and a similar one for Kusama).
  • No reason in particular so I removed the Trappist example

Copy link
Contributor

@IkerAlus IkerAlus left a comment

Choose a reason for hiding this comment

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

Superb work, I only have a comment about an example. The rest looks good to me.

PS: I found some not accurate error messages while testing different inputs, but I will open a different issue for them since they are not directly related to this PR.

@marshacb marshacb merged commit 311ecd9 into main May 20, 2024
6 checks passed
Copy link

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

Very nice!

Left some comments for some corrections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for the new transfer_assets_using_type_and_then() call in pallet XCM
5 participants