Snowbridge: Add support for Ether #548

alistair-singh · 2025-01-16T23:01:12Z

Adds support for the bridging Ether.

TODO

Update snowbridge-router-primitives crate 2409-1 0.16.1
Add a test to test round trip of Ether.

acatangiu · 2025-01-23T08:00:10Z

Doesn't look good, new crates bring new minor or major versions of deps, doesn't build.

I see the snowfork crates have semver-minor bump, I expected patch bumps to have everything working ok.

This reverts commit 2ea7fd3.

yrong · 2025-01-24T02:41:15Z

integration-tests/emulated/tests/bridges/bridge-hub-polkadot/src/tests/snowbridge.rs

+	AssetHubPolkadot::fund_accounts(vec![
+		(AssetHubPolkadotReceiver::get(), INITIAL_FUND),
+		(ethereum_sovereign_account(), INITIAL_FUND),
+	]);


This may not be necessary.

The test failed without (AssetHubPolkadotReceiver::get(), INITIAL_FUND), so I pulled it in from the WETH test. The fee assertions of the test are what failed.

yrong · 2025-01-24T02:48:26Z

integration-tests/emulated/tests/bridges/bridge-hub-polkadot/src/tests/snowbridge.rs

+		let message = VersionedMessage::V1(MessageV1 {
+			chain_id: CHAIN_ID,
+			command: Command::SendToken {
+				token: [0; 20].into(),


Adding a comment like [0;20] representing Ether is better.

Addressed in a0a5211

yrong · 2025-01-24T02:49:12Z

integration-tests/emulated/tests/bridges/bridge-hub-polkadot/src/tests/snowbridge.rs

+			<AssetHubPolkadot as AssetHubPolkadotPallet>::Balances::free_balance(
+				AssetHubPolkadotReceiver::get(),
+			);
+		// Send the Weth back to Ethereum


Should be ether here.

Addressed in 7d39a0a

yrong

+1

github-actions · 2025-01-24T07:33:28Z

Review required! Latest push from author must always be reviewed

claravanstaden

@alistair-singh adding ether does not require any runtime config, except the new snowbridge-router-primitives crate version, right?

alistair-singh · 2025-01-24T07:50:49Z

@alistair-singh adding ether does not require any runtime config, except the new snowbridge-router-primitives crate version, right?

No, we just need the new code in the crate which interprets 0x00... as Ether's multi-location. And the feature is blocked from the solidity side as it wont let you register and submit that address. Deploying the new version of the contract code will enable it by registering Ether at upgrade time.

yrong

+1

bkontur · 2025-01-24T10:02:15Z

integration-tests/emulated/tests/bridges/bridge-hub-polkadot/src/tests/snowbridge.rs

+		<<BridgeHubPolkadot as BridgeHubPolkadotPallet>::Balances as frame_support::traits::fungible::Inspect<_>>::balance(&RelayTreasuryPalletAccount::get())
+	});
+
+	// Send Ether from Asset Hub.


Suggested change

// Send Ether from Asset Hub.

// Send Ether from Asset Hub back to Ethereum.

Addressed in e946e28

bkontur · 2025-01-24T10:06:34Z

integration-tests/emulated/tests/bridges/bridge-hub-polkadot/src/tests/snowbridge.rs

+		assert!(free_balance_diff > AH_BASE_FEE);
+	});
+
+	// Recieve Ether on Bridge Hub and dispatch


Suggested change

// Recieve Ether on Bridge Hub and dispatch

// Check that message with Ether was queued on the BridgeHub

Addressed in e946e28

bkontur · 2025-01-24T10:06:56Z

integration-tests/emulated/tests/bridges/bridge-hub-polkadot/src/tests/snowbridge.rs

+		// Check that the transfer token back to Ethereum message was queue in the Ethereum
+		// Outbound Queue


Suggested change

// Check that the transfer token back to Ethereum message was queue in the Ethereum

// Outbound Queue

// check the outbound queue

Addressed in e946e28

bkontur · 2025-01-24T10:07:24Z

integration-tests/emulated/tests/bridges/bridge-hub-polkadot/src/tests/snowbridge.rs

+		);
+	});
+
+	// Receive ether on Asset Hub.


Suggested change

// Receive ether on Asset Hub.

// Receive Ether on Asset Hub.

Addressed in e946e28

bkontur · 2025-01-24T10:09:50Z

integration-tests/emulated/tests/bridges/bridge-hub-polkadot/src/tests/snowbridge.rs

+		assert_expected_events!(
+			AssetHubPolkadot,
+			vec![
+				RuntimeEvent::ForeignAssets(pallet_assets::Event::Issued { .. }) => {},


we could/should be exact here:

Suggested change

RuntimeEvent::ForeignAssets(pallet_assets::Event::Issued { .. }) => {},

RuntimeEvent::ForeignAssets(pallet_assets::Event::Issued { asset_id, .. }) => {

asset_id: *asset_id == ether_location,

},

Addressed in 798229c

bkontur · 2025-01-24T10:11:06Z

integration-tests/emulated/tests/bridges/bridge-hub-polkadot/src/tests/snowbridge.rs

+		);
+	});
+
+	// Send Ether from Bridge Hub


Suggested change

// Send Ether from Bridge Hub

// Send Ether from Bridge Hub (simulates received Command from Ethereum)

Addressed in e946e28

bkontur · 2025-01-24T10:12:16Z

integration-tests/emulated/tests/bridges/bridge-hub-polkadot/src/tests/snowbridge.rs

+
+	let ether_location: Location = (Parent, Parent, EthereumNetwork::get()).into();
+
+	AssetHubPolkadot::execute_with(|| {


Suggested change

AssetHubPolkadot::execute_with(|| {

// Register Ether as foreign asset on AH.

AssetHubPolkadot::execute_with(|| {

Addressed in e946e28

bkontur · 2025-01-24T10:17:29Z

integration-tests/emulated/tests/bridges/bridge-hub-polkadot/src/tests/snowbridge.rs

@@ -459,6 +461,184 @@ fn send_token_from_ethereum_to_asset_hub() {
 	});
 }

+/// Tests sending ether from Ethereum to Asset Hub and back to Ethereum
+#[test]
+fn send_eth_asset_from_asset_hub_to_ethereum() {


I can imagine to create some generic helper function:

fn send_token_from_ethereum_to_asset_hub_and_back_works(token: Location) { // copy the code below }

and then reuse for test-cases, something like:

#[test] fn send_weth_from_ethereum_to_asset_hub() { send_token_from_ethereum_to_asset_hub_and_back_works(WETH) } #[test] fn send_eth_from_ethereum_to_asset_hub() { let ether_location: Location = (Parent, Parent, EthereumNetwork::get()).into(); send_token_from_ethereum_to_asset_hub_and_back_works(ether_location) }

Addressed in 798229c

alistair-singh mentioned this pull request Jan 16, 2025

Release v1.4.0 #527

Open

6 tasks

alistair-singh force-pushed the alistair/support-native-ether branch from 042c49e to 2ea7fd3 Compare January 23, 2025 09:58

alistair-singh added 9 commits January 24, 2025 01:40

add test

8b2b6db

fix compilation errors

ebe65f4

Update changelog

d481950

updated snowbridge-router-primitives

5bd7602

update snowbridge crates

c6b4f93

Revert "update snowbridge crates"

78836bb

This reverts commit 2ea7fd3.

Use the correct version

8b5247f

fix storage corruption bug

923264e

fix tests

d46f577

alistair-singh force-pushed the alistair/support-native-ether branch from 61541b1 to d46f577 Compare January 23, 2025 23:40

alistair-singh marked this pull request as ready for review January 23, 2025 23:41

revert token amount

9d217a4

yrong reviewed Jan 24, 2025

View reviewed changes

yrong approved these changes Jan 24, 2025

View reviewed changes

make Ether Token Address explicit

a0a5211

github-actions bot requested a review from yrong January 24, 2025 07:33

fix up comments

7d39a0a

claravanstaden approved these changes Jan 24, 2025

View reviewed changes

yrong approved these changes Jan 24, 2025

View reviewed changes

bkontur reviewed Jan 24, 2025

View reviewed changes

all comment related changes

e946e28

github-actions bot requested review from claravanstaden and yrong January 24, 2025 10:50

alistair-singh added 2 commits January 24, 2025 13:27

factor test

798229c

fix test

24b82e2

bkontur approved these changes Jan 24, 2025

View reviewed changes

franciscoaguirre approved these changes Jan 24, 2025

View reviewed changes

alistair-singh mentioned this pull request Jan 26, 2025

Snowbridge: Register Ether Asset and Lower Bridge Fees #560

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Snowbridge: Add support for Ether #548

Snowbridge: Add support for Ether #548

alistair-singh commented Jan 16, 2025 •

edited

Loading

acatangiu commented Jan 23, 2025

yrong Jan 24, 2025

alistair-singh Jan 24, 2025

yrong Jan 24, 2025

alistair-singh Jan 24, 2025

yrong Jan 24, 2025

alistair-singh Jan 24, 2025

yrong left a comment

github-actions bot commented Jan 24, 2025

claravanstaden left a comment

alistair-singh commented Jan 24, 2025

yrong left a comment

bkontur Jan 24, 2025

alistair-singh Jan 24, 2025

bkontur Jan 24, 2025

alistair-singh Jan 24, 2025

bkontur Jan 24, 2025

alistair-singh Jan 24, 2025

bkontur Jan 24, 2025

alistair-singh Jan 24, 2025

bkontur Jan 24, 2025

alistair-singh Jan 24, 2025

bkontur Jan 24, 2025

alistair-singh Jan 24, 2025

bkontur Jan 24, 2025

alistair-singh Jan 24, 2025

bkontur Jan 24, 2025

alistair-singh Jan 24, 2025

	// Send Ether from Asset Hub.
	// Send Ether from Asset Hub back to Ethereum.

	// Recieve Ether on Bridge Hub and dispatch
	// Check that message with Ether was queued on the BridgeHub

		// Check that the transfer token back to Ethereum message was queue in the Ethereum
		// Outbound Queue

	// Check that the transfer token back to Ethereum message was queue in the Ethereum
	// Outbound Queue
	// check the outbound queue

	// Receive ether on Asset Hub.
	// Receive Ether on Asset Hub.

-				RuntimeEvent::ForeignAssets(pallet_assets::Event::Issued { .. }) => {},
+				RuntimeEvent::ForeignAssets(pallet_assets::Event::Issued { asset_id, .. }) => {
+					asset_id: *asset_id == ether_location,
+				},

	// Send Ether from Bridge Hub
	// Send Ether from Bridge Hub (simulates received Command from Ethereum)


		let ether_location: Location = (Parent, Parent, EthereumNetwork::get()).into();

		AssetHubPolkadot::execute_with(\|\| {

	AssetHubPolkadot::execute_with(\|\| {
	// Register Ether as foreign asset on AH.
	AssetHubPolkadot::execute_with(\|\| {

Snowbridge: Add support for Ether #548

Are you sure you want to change the base?

Snowbridge: Add support for Ether #548

Conversation

alistair-singh commented Jan 16, 2025 • edited Loading

acatangiu commented Jan 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yrong left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 24, 2025

claravanstaden left a comment

Choose a reason for hiding this comment

alistair-singh commented Jan 24, 2025

yrong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alistair-singh commented Jan 16, 2025 •

edited

Loading