Open pandres95 opened 1 month ago
I can probably squeeze it in later this week. 👍
Opened for review with a small caveat: tests will keep failing, until #472 is merged and can be merged back to this branch.
@pandres95 if you have anything related to the Bridges, just tell me, at least I can see that migrations are missing here:
pallet_bridge_messages::migration::v1::MigrationToV1<
bridge_to_kusama/polkadot_config::migration::StaticToDynamicLanes, // check this for bridge_to_westend_config::migration::StaticToDynamicLanes
frame_support::migrations::RemoveStorage< // check bridge-hub-westend for removing BridgeRococoMessagesPalletName
Bridge***MessagesPalletName,
OutboundLanesCongestedSignalsKey,
RocksDbWeight,
>,
pallet_bridge_relayers::migration::v1::MigrationToV1<Runtime, ()>,
and also I can help with fixing (integration) tests
Is it reasonable to assume that we are going to use 2409-1? I would update encointer directly to that version then.
@acatangiu to help me setting up the Bridge Hubs, as well as adding the respective transition layer (a.k.a. preparing the migrations from fixed Lanes to LegacyLane-based lanes storage).
I suggest we keep this SDK as light as possible:
@Szegoo @seadanda I just applied the changes you suggested. @bkontur I would love to have some help on the integration tests. As of now, I think some tests will fail due to the issue of not having foreign assets with xcmv4 format in asset hubs (see #472). Once that's done, we can begin with those integration tests.
@pandres95 did you see that I opened https://github.com/pandres95/runtimes/pull/1 into your PR?
Failed to update to stable2409-1
(see commit f94520c
) due to new versions of the updated crates not yet published (see polkadot-sdk#493).
@acatangiu I found that in kah integration tests, the chain 11155111
is being used as the origin chain for wETH. So, I made this change:
https://github.com/pandres95/runtimes/blob/f8adb35e0efda35c96b5e3913e60f7f201aecfcb/system-parachains/asset-hubs/asset-hub-kusama/src/xcm_config.rs#L516-L521
Is this alright? Or should we change that value in the tests and point to the chain 1
?
/cmd bench --runtime bridge-hub-kusama --continue-on-fail
Command "--runtime bridge-hub-kusama --continue-on-fail" has started 🚀 See logs here
Command "--runtime bridge-hub-kusama --continue-on-fail" has failed ❌! See logs here
@acatangiu I found that in kah integration tests, the chain
11155111
is being used as the origin chain for wETH. So, I made this change: https://github.com/pandres95/runtimes/blob/f8adb35e0efda35c96b5e3913e60f7f201aecfcb/system-parachains/asset-hubs/asset-hub-kusama/src/xcm_config.rs#L516-L521Is this alright? Or should we change that value in the tests and point to the chain
1
?
The Rococo/Westend testnets are bridged to Ethereum Sepolia testnet which is using chain_id = 11155111
.
But Ethereum chain_id
should be 1
throughout this repo, as that's Ethereum Mainnet which Polkadot is bridged to.
@pandres95 I added the Snowbridge Polkadot-native token transfer to Ethereum integration tests here: https://github.com/pandres95/runtimes/pull/2
@acatangiu we need to set the ReserveAssetDeposited weight in PBH to something realistic for PNA to work.
@acatangiu we need to set the ReserveAssetDeposited weight in PBH to something realistic for PNA to work.
why? where would BridgeHub receive/execute a ReserveAssetDeposited
from? BH only handles DOT which cannot be transferred into BH as a reserve transfer with a remote reserve.
why? where would BridgeHub receive/execute a
ReserveAssetDeposited
from? BH only handles DOT which cannot be transferred into BH as a reserve transfer with a remote reserve.
Our EthereumBlobExporter expects a message that contains ReserveAssetDeposited
. The exporter then parses the xcm payload to send a command to Ethereum. So it does not actually execute ReserveAssetDeposited
with the XCM executor, but I think the message is still weighed per instruction.
Our EthereumBlobExporter expects a message that contains
ReserveAssetDeposited
. The exporter then parses the xcm payload to send a command to Ethereum. So it does not actually executeReserveAssetDeposited
with the XCM executor, but I think the message is still weighed per instruction.
AFAIK, the XCM is weighed as a first step of execution. If BridgeHub is not executing the XCM, why would it weigh it?
I was expecting the XCM is captured, parsed and consumed by the Snowbridge exporter before being weighed or executed. ~Ideally it should be captured even before trying to go through the BH XCM barriers, but I'm not sure about that, needs checking.~ L.E.: barriers are used during execution, so they should also not be a problem.
@acatangiu you are right, its not the weight that is a problem. It is related to withdrawing the fee asset it seems:
2024-10-29T10:24:55.407179Z TRACE xcm::bridge-hub-router: Router with bridged_network_id Kusama does not support bridging to network Ethereum { chain_id: 1 }!
2024-10-29T10:24:55.407185Z TRACE xcm::bridge-hub-router: validate - ViaBridgeHubExporter - error: NotApplicable
2024-10-29T10:24:55.407318Z TRACE xcm::pallet_xcm::note_unknown_version: XCM version is unknown for destination: Location { parents: 1, interior: X1([Parachain(1002)]) }
2024-10-29T10:24:55.407504Z TRACE xcm::contains: AllSiblingSystemParachains location: Location { parents: 0, interior: X1([AccountId32 { network: Some(Polkadot), id: [212, 53, 147, 199, 21, 253, 211, 28, 97, 20, 26, 189, 4, 169, 159, 214, 130, 44, 133, 88, 133, 76, 205, 227, 154, 86, 132, 231, 165, 109, 162, 125] }]) }
2024-10-29T10:24:55.408142Z TRACE xcm::fungible_adapter: withdraw_asset what: Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(340282366920938463463374607431768211455) }, who: Location { parents: 0, interior: X1([AccountId32 { network: Some(Polkadot), id: [212, 53, 147, 199, 21, 253, 211, 28, 97, 20, 26, 189, 4, 169, 159, 214, 130, 44, 133, 88, 133, 76, 205, 227, 154, 86, 132, 231, 165, 109, 162, 125] }]) }
@pandres95 @acatangiu test fixes over here: https://github.com/pandres95/runtimes/pull/4 Thanks @alistair-singh for the BridgeHubEthereumBaseFee
tip, you were right. 🏆
@acatangiu @bkontur any clue about this error?
https://github.com/polkadot-fellows/runtimes/actions/runs/11574125246/job/32217719672#step:11:244
@acatangiu @bkontur any clue about this error?
https://github.com/polkadot-fellows/runtimes/actions/runs/11574125246/job/32217719672#step:11:244
I think it should be related to the frame-omni-bencher bug, let me check that and try it tomorrow
Inviting @Szegoo @seadanda and @bkontur to give it a second review.
What's the state of this? Only needs reviews?
Only reviews @franciscoaguirre. There are some "failing" migrations, but the failure is related to the spec version. Once we bump, those errors should disappear.
/cmd help
Command "help" has started 🚀 See logs here
Command "help" has failed ❌! See logs here
/cmd --help
@SBalaguer @anaelleltd just to mention this PR aims to update the Polkadot SDK version of the runtimes to the latest stable one (stable2409-1
).
To reviewers: Please take care to double check this update carefully and advise if there are potential breaking changes or disruptions for ecosystem builders. If any, please ping @anaelleltd or @SBalaguer in your detailed commentary so that we can notify ecosystem teams with the critical information.
Closes #457 Closes #469
Updating the runtimes to SDK version
stable2409-1
. CHANGELOG mentions all relevant changes for UI and Walletbuilders.Checklist
stable2409
stable2409-1
Dependencies
This PR depends on certain PRs to be merged before it can properly work.
Impacts
The success of this PR impacts directly on the feasibility of executing certain issues, or closing certain PRs.
Help wanted
Initially, I request the help of:
XCMv4
.Lanes
toLegacyLane
-based lanes storage).