New reserve transfer XCM parsing

[alistair-singh]

Resolves: SNO-734, SNO-645
PolkadotSDK companion: Snowfork/polkadot-sdk#19

The XCM message that the converter now matches:

WithdrawAsset(MultiAssets([MultiAsset { id: Concrete(MultiLocation { parents: 0, interior: X1(AccountKey20 { network: None, key: [135, 209, 247, 253, 254, 231, 246, 81, 250, 188, 139, 252, 182, 224, 134, 194, 120, 183, 122, 125] }) }), fun: Fungible(1000000000) }])), 
ClearOrigin, 
BuyExecution { fees: MultiAsset { id: Concrete(MultiLocation { parents: 0, interior: X1(AccountKey20 { network: None, key: [135, 209, 247, 253, 254, 231, 246, 81, 250, 188, 139, 252, 182, 224, 134, 194, 120, 183, 122, 125] }) }), fun: Fungible(1000000000) }, weight_limit: Unlimited }, 
DepositAsset { assets: Wild(AllCounted(1)), beneficiary: MultiLocation { parents: 0, interior: X1(AccountKey20 { network: None, key: [68, 165, 126, 226, 242, 252, 203, 133, 253, 162, 176, 177, 142, 189, 13, 141, 35, 51, 112, 14] }) } },
SetTopic([199, 103, 189, 131, 99, 176, 102, 142, 225, 227, 240, 175, 195, 94, 8, 217, 140, 50, 143, 240, 62, 231, 58, 198, 235, 145, 192, 53, 73, 163, 98, 210])

The only mandatory instructions are WithdrawAsset, DepositAsset and SetTopic. ClearOrigin is optional and will be skipped if present. BuyExecution is also skipped if it is present however when it is present then the fee asset and amounts are validated against the reserve assets.

The smallest valid xcm that can be converted is:

WithdrawAsset(MultiAssets([MultiAsset { id: Concrete(MultiLocation { parents: 0, interior: X1(AccountKey20 { network: None, key: [135, 209, 247, 253, 254, 231, 246, 81, 250, 188, 139, 252, 182, 224, 134, 194, 120, 183, 122, 125] }) }), fun: Fungible(1000000000) }])), 
DepositAsset { assets: Wild(AllCounted(1)), beneficiary: MultiLocation { parents: 0, interior: X1(AccountKey20 { network: None, key: [68, 165, 126, 226, 242, 252, 203, 133, 253, 162, 176, 177, 142, 189, 13, 141, 35, 51, 112, 14] }) } },
SetTopic([199, 103, 189, 131, 99, 176, 102, 142, 225, 227, 240, 175, 195, 94, 8, 217, 140, 50, 143, 240, 62, 231, 58, 198, 235, 145, 192, 53, 73, 163, 98, 210]),

Tests pass. e2e fails on bridgehub with:

xcm::execute: [Parachain] !!! ERROR: NotHoldingFees

There is not enough fees in holding.
Required fees(2.20 DOT):
MultiAsset { id: Concrete(MultiLocation { parents: 1, interior: Here }), fun: Fungible(2200016903333) }
Provided fees(0.028 DOT):
MultiAsset { id: Concrete(MultiLocation { parents: 1, interior: Here }), fun: Fungible( 28376733291) }

Busy looking into this. However this PR successfully parses the new xcm format so its ready for review.

TODO:

• Tweak fees
• e2e tests pass

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (c9196e1) 83.25% compared to head (9c27d53) 83.31%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #995      +/-   ##
==========================================
+ Coverage   83.25%   83.31%   +0.06%     
==========================================
  Files          51       51              
  Lines        2060     2038      -22     
  Branches       71       71              
==========================================
- Hits         1715     1698      -17     
+ Misses        330      325       -5     
  Partials       15       15              

Flag	Coverage Δ
rust	83.82% <98.14%> (+0.08%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
parachain/primitives/router/src/outbound/mod.rs	90.09% <98.14%> (+2.12%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[yrong]

@alistair-singh Could you please also attach the original reserve transfer call you test with? The emulated test failed with an Unroutable error in pattern matching.

cargo test -p bridge-hub-rococo-integration-tests --lib tests::snowbridge::reserve_transfer_token -- --nocapture
...
Nov 07 15:05:19.723 ERROR xcm::ethereum_blob_exporter: unroutable due to pattern matching error 'WithdrawExpected'.
...
Nov 07 15:05:19.724 DEBUG events::BridgeHubRococo: RuntimeEvent::XcmpQueue(Event::Fail { message_hash: [133, 89, 208, 179, 197, 251, 209, 235, 153, 117, 207, 85, 6, 43, 184, 104, 154, 144, 137, 12, 1, 102, 193, 187, 139, 253, 165, 184, 34, 249, 240, 233], message_id: [108, 76, 161, 165, 176, 210, 20, 172, 223, 136, 91, 196, 110, 27, 85, 7, 83, 0, 0, 39, 140, 106, 141, 222, 156, 175, 222, 111, 123, 204, 137, 134], error: Unroutable, weight: Weight { ref_time: 504529372, proof_size: 5122 } })

[claravanstaden]

Looks great!

The message format I have includes a RefundSurplus instruction. Just checking if we need to parse that or if it is outdated.

I am looking into the fee discrepancy. I think this is fine to merge - I'll make a follow-up PR to update the fee in the polkadot-sdk.

[claravanstaden]

Does gateway_network still makes sense here? Maybe expected_network or something?

[alistair-singh]

Resolved in da15bbc

[alistair-singh]

@alistair-singh Could you please also attach the original reserve transfer call you test with? The emulated test failed with an Unroutable error in pattern matching.

cargo test -p bridge-hub-rococo-integration-tests --lib tests::snowbridge::reserve_transfer_token -- --nocapture
...
Nov 07 15:05:19.723 ERROR xcm::ethereum_blob_exporter: unroutable due to pattern matching error 'WithdrawExpected'.
...
Nov 07 15:05:19.724 DEBUG events::BridgeHubRococo: RuntimeEvent::XcmpQueue(Event::Fail { message_hash: [133, 89, 208, 179, 197, 251, 209, 235, 153, 117, 207, 85, 6, 43, 184, 104, 154, 144, 137, 12, 1, 102, 193, 187, 139, 253, 165, 184, 34, 249, 240, 233], message_id: [108, 76, 161, 165, 176, 210, 20, 172, 223, 136, 91, 196, 110, 27, 85, 7, 83, 0, 0, 39, 140, 106, 141, 222, 156, 175, 222, 111, 123, 204, 137, 134], error: Unroutable, weight: Weight { ref_time: 504529372, proof_size: 5122 } })

I have added a comments in your PR to fix this test.

[vgeddes]

Actually, can we mandate that the message has a SetTopic in it? I am preparing a related PR that allows to track messages across chains, and so we need to be able to extract the topic.

[alistair-singh]

Actually, can we mandate that the message has a SetTopic in it? I am preparing a related PR that allows to track messages across chains, and so we need to be able to extract the topic.

Addressed in 4011aa5

I left _topic_id unused (See commit) as I am not sure if you want to embed it in AgentExecuteCommand::TransferToken or in the lower level message format. SetTopic will only be here for AgentExecuteCommand::TransferToken.