Asset from polkadot #1155

yrong · 2024-03-13T10:26:35Z

codecov · 2024-03-13T10:28:58Z

Codecov Report

Attention: Patch coverage is 44.58599% with 87 lines in your changes missing coverage. Please review.

Project coverage is 70.93%. Comparing base (24b9d31) to head (6efc7a1).
Report is 98 commits behind head on main.

Files with missing lines	Patch %	Lines
contracts/src/TokenLib.sol	23.07%	40 Missing ⚠️
contracts/src/Assets.sol	63.26%	15 Missing and 3 partials ⚠️
contracts/src/Gateway.sol	48.57%	18 Missing ⚠️
contracts/src/Token.sol	42.10%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1155      +/-   ##
==========================================
- Coverage   77.83%   70.93%   -6.90%     
==========================================
  Files          14       18       +4     
  Lines         415      609     +194     
  Branches       76      112      +36     
==========================================
+ Hits          323      432     +109     
- Misses         75      165      +90     
+ Partials       17       12       -5

Flag	Coverage Δ
solidity	`70.93% <44.58%> (-6.90%)`	⬇️

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.

contracts/src/AgentExecutor.sol

yrong · 2024-03-26T10:15:13Z

@vgeddes In 0b3344c I made the change to reuse IGateway.sendToken for polkadot native asset.

But I'd still prefer to separate as different message types internally(rename as SendForeignToken). The two branches differ in several aspects(e.g. AssetHub is not involved in SendForeignToken and the xcm constructed is different) and the separating will make the code cleaner/maintainable IMO.

yrong · 2024-03-26T17:04:31Z

Add 3 smoke tests to cover the basic flow

cargo test --test register_polkadot_token // register relay token(i.e. ROC|KSM|DOT) on Ethereum
cargo test --test transfer_polkadot_token // transfer relay token from AssetHub to Ethereum
cargo test --test send_polkadot_token // send relay token from Ethereum back to AssetHub

vgeddes

Looks good! Though in retrospect, some of my previous recommendations to you were not good ideas in the end, and so I'd like you to revert those.

vgeddes · 2024-03-27T18:58:55Z

contracts/src/Types.sol

@@ -107,5 +109,8 @@ struct Ticket {

 struct TokenInfo {
    bool isRegistered;
-    bytes31 __padding;


Need to make sure that the storage layout of the old and new versions of the struct are compatible - ie. that isForeign and isRegistered are all packed into a single 32 byte slot.

Yeah, could be a problem. I'd prefer a specific migration e5d9dd8

contracts/src/interfaces/IGateway.sol

contracts/src/AgentExecutor.sol

contracts/src/Assets.sol

contracts/src/Gateway.sol

contracts/src/Assets.sol

contracts/src/Gateway.sol

* Improve PNA implementation * More improvents to ERC20 token * Fix implementation of Assets._sendForeignToken * Fix build payload for foreign token --------- Co-authored-by: ron <[email protected]>

vgeddes · 2024-08-28T22:07:14Z

contracts/src/storage/LegacyAssetsStorage.sol

i think we should preserve the existing storage layout in AssetStorage.sol rather than doing a full storage migration. The gas cost of migrating storage is unbounded (many tokens registered) so that could cost a lot and introduce a lot of errors.

We can however add new fields to the end of the Layout struct in AssetStorage.sol.

Removed the storage migration in 64b9ef4

The initial concern in #1155 (comment) is about the TokenInfo structure we changed. I'd assume it's safe to add the new foreignID without overwritten the exist storage, but not 100% sure.

From Vincent: we may need a fork testing, add a test testUpgradeGatewayForPNA that upgrades the mainnet contract, and then runs some other nested tests as a sanity check(e.g. making sure the storage layout is not corrupted after the upgrade).

# Description Adding support for send polkadot native assets(PNA) to Ethereum network through snowbridge. Asset with location in view of AH Including: - Relay token `(1,Here)` - Native asset `(0,[PalletInstance(instance),GenereIndex(index)])` managed by Assets Pallet - Native asset of Parachain `(1,[Parachain(paraId)])` managed by Foreign Assets Pallet The original PR in Snowfork#128 which has been internally reviewed by Snowbridge team. # Notes - This feature depends on the companion solidity change in Snowfork/snowbridge#1155. Currently register PNA is only allowed from [sudo](https://github.com/Snowfork/polkadot-sdk/blob/46cb3528cd8cd1394af2335a6907d7ab8647717a/bridges/snowbridge/pallets/system/src/lib.rs#L621), so it's actually not enabled. Will require another runtime upgrade to make the call permissionless together with upgrading the Gateway contract. - To make things easy multi-hop transfer(i.e. sending PNA from Ethereum through AH to Destination chain) is not support ed in this PR. For this case user can switch to 2-phases transfer instead. --------- Co-authored-by: Clara van Staden <[email protected]> Co-authored-by: Alistair Singh <[email protected]> Co-authored-by: Vincent Geddes <[email protected]> Co-authored-by: Francisco Aguirre <[email protected]> Co-authored-by: Adrian Catangiu <[email protected]>

# Description Adding support for send polkadot native assets(PNA) to Ethereum network through snowbridge. Asset with location in view of AH Including: - Relay token `(1,Here)` - Native asset `(0,[PalletInstance(instance),GenereIndex(index)])` managed by Assets Pallet - Native asset of Parachain `(1,[Parachain(paraId)])` managed by Foreign Assets Pallet The original PR in #128 which has been internally reviewed by Snowbridge team. # Notes - This feature depends on the companion solidity change in Snowfork/snowbridge#1155. Currently register PNA is only allowed from [sudo](https://github.com/Snowfork/polkadot-sdk/blob/46cb3528cd8cd1394af2335a6907d7ab8647717a/bridges/snowbridge/pallets/system/src/lib.rs#L621), so it's actually not enabled. Will require another runtime upgrade to make the call permissionless together with upgrading the Gateway contract. - To make things easy multi-hop transfer(i.e. sending PNA from Ethereum through AH to Destination chain) is not support ed in this PR. For this case user can switch to 2-phases transfer instead. --------- Co-authored-by: Clara van Staden <[email protected]> Co-authored-by: Alistair Singh <[email protected]> Co-authored-by: Vincent Geddes <[email protected]> Co-authored-by: Francisco Aguirre <[email protected]> Co-authored-by: Adrian Catangiu <[email protected]>

Asset from polkadot

7501630

Remove params unnecessary

e0c3581

vgeddes reviewed Mar 20, 2024

View reviewed changes

contracts/src/AgentExecutor.sol Outdated Show resolved Hide resolved

vgeddes reviewed Mar 20, 2024

View reviewed changes

contracts/src/AgentExecutor.sol Outdated Show resolved Hide resolved

vgeddes reviewed Mar 20, 2024

View reviewed changes

contracts/src/AgentExecutor.sol Outdated Show resolved Hide resolved

yrong added 10 commits March 20, 2024 16:59

Merge branch 'main' into ron/polkadot-assets-on-ethereum

f070e16

Experiment: return calldata from Agent instead of call Gateway directly

780c7bd

To store registries also on agent

27353f0

Merge branch 'main' into ron/polkadot-assets-on-ethereum

a543f0b

Send polkadot tokens back

6212597

Cleanup for not exceed the size limit

d28d9df

Add smoke test for register polkadot token

c23f8bb

Transfer relay token to Ethereum

8c8afac

Reuse IGateway.sendToken for polkadot native asset

0b3344c

More cleanup

2d17fca

yrong added 2 commits March 27, 2024 00:46

Fix encode substrate types

047becf

Add smoke test send relay token back

3fc6cab

yrong marked this pull request as ready for review March 26, 2024 17:04

yrong added 2 commits March 27, 2024 09:38

Rename to sendForeignToken

e53afd0

Improve ERC20.sol

5e9820b

yrong requested review from alistair-singh and claravanstaden March 27, 2024 08:30

vgeddes reviewed Mar 27, 2024

View reviewed changes

yrong added 4 commits March 29, 2024 00:12

Revert the change double register the token

fddb9cf

Migration for AssetsStorage

e5d9dd8

Rename to _sendForeignTokenCosts

9f7e99b

Use storage for less gas

724c649

yrong added 8 commits April 11, 2024 13:44

Add AgentExecuteCommand back for compatibility

fcd7539

Cleanup

dc78276

Merge branch 'main' into ron/polkadot-assets-on-ethereum

07545cf

Add rfc

03cf01c

Update polkadot-native-assets.md

e9013cd

Merge branch 'bridge-next-gen' into ron/polkadot-assets-on-ethereum

a028d5c

Update rfc

3b7e178

Update rfc with fee section

63e8322

yrong mentioned this pull request Apr 24, 2024

Transact from sub to eth #1145

Open

yrong added 2 commits May 3, 2024 22:10

Fix _calculateFee for polkadot native token

c887e7e

Merge branch 'main' into ron/polkadot-assets-on-ethereum

f892da4

yrong changed the base branch from bridge-next-gen to main August 22, 2024 15:08

Improve solidity code for PNA (#1275)

84bb52c

* Improve PNA implementation * More improvents to ERC20 token * Fix implementation of Assets._sendForeignToken * Fix build payload for foreign token --------- Co-authored-by: ron <[email protected]>

vgeddes reviewed Aug 28, 2024

View reviewed changes

yrong added 8 commits August 29, 2024 09:19

Remove storage migration

64b9ef4

Merge branch 'main' into ron/polkadot-assets-on-ethereum

de3863c

Merge branch 'main' into ron/polkadot-assets-on-ethereum

25092dd

Smoke test for register relay token

001c990

Fix fee estimation

324cf53

Fix binding

fe4cb73

Remove outdated doc

46bc45f

Fork upgrade test with sanity checks

cf596f3

yrong force-pushed the ron/polkadot-assets-on-ethereum branch from 13e8c32 to cf596f3 Compare August 31, 2024 15:01

Fix test

6efc7a1

vgeddes approved these changes Sep 2, 2024

View reviewed changes

yrong merged commit 07bea54 into main Sep 2, 2024
2 checks passed

yrong deleted the ron/polkadot-assets-on-ethereum branch September 2, 2024 09:00

yrong mentioned this pull request Sep 3, 2024

Transfer Polkadot-native assets to Ethereum paritytech/polkadot-sdk#5546

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Asset from polkadot #1155

Asset from polkadot #1155

yrong commented Mar 13, 2024 •

edited

Loading

codecov bot commented Mar 13, 2024 •

edited

Loading

yrong commented Mar 26, 2024 •

edited

Loading

yrong commented Mar 26, 2024

vgeddes left a comment

vgeddes Mar 27, 2024

yrong Mar 29, 2024

vgeddes Aug 28, 2024 •

edited

Loading

yrong Aug 29, 2024 •

edited

Loading

yrong Aug 31, 2024

Asset from polkadot #1155

Asset from polkadot #1155

Conversation

yrong commented Mar 13, 2024 • edited Loading

codecov bot commented Mar 13, 2024 • edited Loading

Codecov Report

yrong commented Mar 26, 2024 • edited Loading

yrong commented Mar 26, 2024

vgeddes left a comment

Choose a reason for hiding this comment

vgeddes Mar 27, 2024

Choose a reason for hiding this comment

yrong Mar 29, 2024

Choose a reason for hiding this comment

vgeddes Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

yrong Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

yrong Aug 31, 2024

Choose a reason for hiding this comment

yrong commented Mar 13, 2024 •

edited

Loading

codecov bot commented Mar 13, 2024 •

edited

Loading

yrong commented Mar 26, 2024 •

edited

Loading

vgeddes Aug 28, 2024 •

edited

Loading

yrong Aug 29, 2024 •

edited

Loading