Transact from eth to sub #114

yrong · 2024-02-14T02:42:55Z

No description provided.

codecov · 2024-02-14T06:16:47Z

Codecov Report

Attention: Patch coverage is 79.06977% with 9 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (bridge-next-gen@3f495e5). Click here to learn what that means.

❗ Current head 7cc0d59 differs from pull request most recent head ee6bdc5. Consider uploading reports for the commit ee6bdc5 to get more accurate results

Files	Patch %	Lines
...ridges/snowbridge/pallets/inbound-queue/src/lib.rs	50.00%	5 Missing ⚠️
.../inbound-queue/fixtures/src/send_call_to_penpal.rs	0.00%	3 Missing ⚠️
...es/snowbridge/primitives/router/src/inbound/mod.rs	96.55%	1 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##             bridge-next-gen     #114   +/-   ##
==================================================
  Coverage                   ?   40.92%           
==================================================
  Files                      ?       52           
  Lines                      ?     3372           
  Branches                   ?        0           
==================================================
  Hits                       ?     1380           
  Misses                     ?     1992           
  Partials                   ?        0

Flag	Coverage Δ
rust	`40.92% <79.06%> (?)`

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

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

cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs

…o pay fees

bridges/snowbridge/pallets/inbound-queue/src/lib.rs

bridges/snowbridge/primitives/router/src/inbound/mod.rs

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

claravanstaden

Looks great! The penpal changes just need to made generic, otherwise awesome!

bridges/snowbridge/primitives/router/src/inbound/mod.rs

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

claravanstaden · 2024-04-10T14:05:44Z

cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs


 parameter_types! {
 	pub const RelayLocation: Location = Location::parent();
-	pub const RelayNetwork: Option<NetworkId> = None;
+	pub const RelayNetwork: NetworkId = Rococo;


We cannot make this change because the polkadot-fellows/runtimes#130 uses this parachain for testing as well. Perhaps we can set the Relay chain more generically?

68abf04

As you may notice, with more bridge features like Transact added to penpal, we do need several configurations Network/Ethereum binding, basically the same as the configuration on AssetHub.

In above commit I've made them as Storage which can be changed for network like Polkadot or Ethereum mainnet, but maybe it's better to be more specific to add separate runtimes for different networks(e.g. penpal-kusama&penpal-polkadot)?

WDYT?

Perhaps, yes. But since this code belongs to Parity maybe it would be better to ask someone like @bkontur.

Or I can use another template chain for the integration.

previously we used hack like:

pub storage RelayNetwork: Option<NetworkId> = None;

but there is also new parameters pallet, so I would suggest to go configurable way instead of hard-code Rococo to penpal, because we use it for fellows also

well, Adrian already changed that to storage stuff with default to Rococo: https://github.com/paritytech/polkadot-sdk/pull/3695/files#diff-37d844ab36f3c061d846240105ddb24c0832ca0f7da21a89ea3e051c6d978b3bR64 (will be merged soon)

claravanstaden · 2024-04-10T14:07:25Z

cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs

+	pub SiblingBridgeHubWithEthereumInboundQueueInstance: Location = Location::new(
+				1,
+				[
+					Parachain(bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID),


Same here, these values need to configurable so that we can set them from any runtime. Here's an example of how the Ethereum network ID was made configurable in Penpal: paritytech#3564

#114 (comment)

…vereign account

alistair-singh · 2024-04-19T00:36:33Z

bridges/snowbridge/primitives/router/src/inbound/mod.rs

+			// Forward message id
+			SetTopic(message_id.into()),
+		];
+		(message.into(), fee.into())


I would return 0 as a fee here. The returned fee is meant to be burnt.

alistair-singh · 2024-04-19T00:51:26Z

bridges/snowbridge/primitives/router/src/inbound/mod.rs

+		payload: Vec<u8>,
+	) -> (Xcm<()>, Balance) {
+		// Fee in native token of destination chain
+		let xcm_fee: Asset = (Location::here(), fee).into();


We need to use native or DOT based on whether its a system parachain or not. But in future we may want to configure this per channel.

5a22972 configure per channel.

alistair-singh · 2024-05-02T21:00:39Z

bridges/snowbridge/primitives/router/src/inbound/mod.rs

+			WithdrawAsset(xcm_fee.clone().into()),
+			// Pay for execution.
+			BuyExecution { fees: xcm_fee, weight_limit: Unlimited },
+			Transact { origin_kind, require_weight_at_most: weight_at_most, call: payload.into() },


We need to RefundSurplus execution left over from the Transact and DepositAsset any left over fees back into the prefunded account.

Good catch!
ee6bdc5

## [0.5.0] - 2023-05-24 This is a small patch release that makes the `FindNode` command a bit more robst: - The `FindNode` command now retains the K (replication factor) best results. - The `FindNode` command has been updated to handle errors and unexpected states without panicking. ### Changed - kad: Refactor FindNode query, keep K best results and add tests ([#114](paritytech/litep2p#114)) --------- Signed-off-by: Alexandru Vasile <[email protected]>

yrong added 3 commits February 14, 2024 09:32

Fix for unit test

d68c3d6

Update for transact

307ecd6

Penpal config for transact

7c493bd

yrong mentioned this pull request Feb 14, 2024

Transact from eth to sub Snowfork/snowbridge#1141

Open

yrong added 2 commits February 14, 2024 13:35

Fix import

e55cfd1

Fix taplo

45c3372

yrong added 3 commits February 14, 2024 15:21

Fix for breaking emulated tests

26df807

Remove unused

c4fe031

Tranfer fees to BH for transact

51b76fe

alistair-singh reviewed Feb 14, 2024

View reviewed changes

cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs Show resolved Hide resolved

cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs Outdated Show resolved Hide resolved

yrong added 2 commits February 15, 2024 08:51

Remove the teleport config

3ed7260

Enable create agent/channel by xcm

772fa46

yrong requested review from vgeddes and claravanstaden February 15, 2024 07:30

yrong added 9 commits February 16, 2024 12:42

Refund surplus

540a791

Fix format

051b3b6

Replace hardcode value

e523185

Leave to user to prefund sovereign account on the destination chain t…

a8b8435

…o pay fees

Update import path

b10d19e

Merge branch 'snowbridge' into transact-from-eth-to-sub

cbd2e19

Unit test for transact

e363918

DescendOrigin to inbound-queue first

13217f1

Fix breaking test

257e75d

vgeddes reviewed Feb 22, 2024

View reviewed changes

yrong added 5 commits February 22, 2024 21:17

First match RegisterToken or SendToken

e03942a

Remove deprecated

698a9e9

Use SetAppendix and refund surplus when transact fail

9968a37

Ignore parents which is meaningless for GlobalConsensus locations

40ea01a

Reuse weight type

004afd2

yrong added 8 commits February 28, 2024 20:47

Merge branch 'snowbridge' into transact-from-eth-to-sub

050585c

Merge branch 'snowbridge' into transact-from-eth-to-sub

ece8a96

Add TransactFeeMode

7b916e5

Improve convert logic

15759a4

Fix UniversalLocation

04a339e

AllowUnpaidExecutionFromSnowBridgeWithFeeChecked

c18f771

Merge branch 'snowbridge' into transact-from-eth-to-sub

e49f077

Remove feeMode and fully charge all cost on Ethereum

362e414

claravanstaden suggested changes Apr 10, 2024

View reviewed changes

yrong added 3 commits April 11, 2024 09:37

Merge branch 'snowbridge' into transact-from-eth-to-sub

6fb78fe

Fix build error

9d0af91

Make network specific types as storage which can be changed

68abf04

yrong changed the base branch from snowbridge to bridge-next-gen April 11, 2024 02:20

yrong added 2 commits April 11, 2024 11:23

Remove the fund steps unnecessary

76b01b2

Move Transact after Origin operations

87ed097

bkontur mentioned this pull request Apr 11, 2024

pallet-xcm: add new extrinsic for asset transfers using explicit XCM transfer types paritytech/polkadot-sdk#3695

Merged

yrong added 3 commits April 11, 2024 20:50

Fix breaking test

0ea24a4

Merge branch 'bridge-next-gen' into transact-from-eth-to-sub

b85e362

Fix compile error

cd7a64a

yrong force-pushed the transact-from-eth-to-sub branch from 5077bf6 to cd7a64a Compare April 16, 2024 01:30

Use native token of destination chain to pay fee from a pre-funded so…

966abdf

…vereign account

alistair-singh reviewed Apr 19, 2024

View reviewed changes

yrong added 3 commits April 22, 2024 11:33

Use native token or DOT based on whether its a system parachain or not

1cab94c

Make fee asset as configuration parameter of the Channel

5a22972

Zero fee for transact

9829de7

yrong requested a review from claravanstaden April 28, 2024 03:09

Remove unused

2f30d46

alistair-singh reviewed May 2, 2024

View reviewed changes

Deposit any left over fees back into the pre-funded account

ee6bdc5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Transact from eth to sub #114

Transact from eth to sub #114

yrong commented Feb 14, 2024

codecov bot commented Feb 14, 2024 •

edited

Loading

claravanstaden left a comment

claravanstaden Apr 10, 2024

yrong Apr 11, 2024 •

edited

Loading

claravanstaden Apr 11, 2024

yrong Apr 11, 2024

bkontur Apr 11, 2024

bkontur Apr 11, 2024

claravanstaden Apr 10, 2024

yrong Apr 11, 2024

alistair-singh Apr 19, 2024

yrong Apr 28, 2024

alistair-singh Apr 19, 2024

yrong Apr 22, 2024

yrong Apr 28, 2024

alistair-singh May 2, 2024

yrong May 3, 2024

Transact from eth to sub #114

Are you sure you want to change the base?

Transact from eth to sub #114

Conversation

yrong commented Feb 14, 2024

codecov bot commented Feb 14, 2024 • edited Loading

Codecov Report

claravanstaden left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yrong Apr 11, 2024 • edited Loading

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

codecov bot commented Feb 14, 2024 •

edited

Loading

yrong Apr 11, 2024 •

edited

Loading