Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add support for foreign asset multilocations #224

Merged
merged 60 commits into from
Aug 2, 2023
Merged

feat: add support for foreign asset multilocations #224

merged 60 commits into from
Aug 2, 2023

Conversation

marshacb
Copy link
Contributor

@marshacb marshacb commented Jul 13, 2023 •
edited
Loading

TODOs:

  • getFeeAssetItemIndex for foreign assets
  • update multiasset sorting
  • integration tests
  • unit tests
  • docs
  • update examples
  • error validation layer
  • add foreign assets pallet instance from registry

closes: #140
closes: #229
closes: #230

@marshacb
add system to system direction
efc3743
@marshacb
Merge branch 'main' into cameron-implement-system-system
61acc9b
@marshacb
update isLocalSystemTx
cdf89fe
@marshacb
update createTransferTransaction
6748527
@marshacb
update system to system to check palletInstance only if not relay nat…
590257c 
…ive in order to allow system chains without assets pallet to create native asset xcm transfers
@marshacb
remove console logs
5bfcbb3
@marshacb
update Native direction to include SystemToSystem
1a1d9ce
@marshacb
update system to system general index check
7300c22 
lint
@marshacb
re-add cypress to lookupAssetId test
ce47c3a
@marshacb
update xcmDirections checks in AssetTransferApi to use enum
64d0c47
@marshacb
update SystemToSystem integration tests
cfba4d8
@marshacb
merge main
4bbe5e8
@marshacb
lint
5e7d332
@marshacb
update SystemToSystem fetchAssetType tests
811fc60
@marshacb
update AssetTransferApi establishDirection unit test for system to sy…
fc8048c 
…stem
@marshacb
update checkIsValidSystemChainAssetId to accept xcm direction
c452575
@marshacb
add tests for getChainidBySpecName
4cd1a9b
@marshacb
merge main
08b3622
@marshacb
lint
4c11e79
@marshacb
update docs
90a0c64
@marshacb
removed destAddr AccountId20 for SystemToSytem
234b973
@marshacb
abstract isSystemToSystemReserveTransfer conditional
cfc3a67
@marshacb
add asset hub names to SYSTEM PARACHAIN NAMES
71d26cc
@marshacb
update createPayload to get address for both local and XCM transactions
d3504a7
@marshacb
remove unecessary fields from submittable data interface
4752a80
@marshacb
add AssetsPalletType
9d2fc2f
@marshacb
Merge branch 'cameron-implement-system-to-system' into cameron-foreig…
c48b860 
…n-asset-support
@marshacb
add transferForeignAssets option
787ed53
@marshacb
add tests for foreignAssets transfer and trasnferKeepAlive
76564f2
@marshacb
add transferForeignAssets option
75816d1 
allow for multilocation string assetIds to be passed in as assetId args
@marshacb marshacb changed the title [WIP] feat: add support for foreign asset multilocations feat: add support for foreign asset multilocations Jul 21, 2023
IkerAlus
IkerAlus requested changes Jul 25, 2023
View reviewed changes
Copy link
Contributor

@IkerAlus IkerAlus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some comments regarding the examples.

I will do the code review afterwards.

examples/systemToSystemXcmForeignAssets.ts Outdated Show resolved Hide resolved
examples/systemToSystemLocalForeignAssets.ts Outdated Show resolved Hide resolved
examples/systemToParaTeleportForeignAssets.ts Outdated Show resolved Hide resolved
examples/systemToParaReserveTransferForeignAssets.ts Outdated Show resolved Hide resolved
examples/paraToSystemForeignAssetsReserveTransferAssets.ts Outdated Show resolved Hide resolved
IkerAlus
IkerAlus reviewed Jul 25, 2023
View reviewed changes
src/AssetsTransferApi.ts Outdated Show resolved Hide resolved
IkerAlus
IkerAlus reviewed Jul 25, 2023
View reviewed changes
src/AssetsTransferApi.ts Outdated Show resolved Hide resolved
@marshacb
added test cases for duplicate symbol, integer, empty string and mult…
e3b091a 
…ilocation asset ids

added checks and test cases to error when assetIds are not exclusively either symbol, integer and empty string or multilocation values
added checks and test cases to ensure assetIds length are not greater than MAX_ASSETS_FOR_TRANSFER
added test case for paysWithFeeDest to ensure xcm version is 3
@marshacb
assign MAX_ASSETS_FOR_TRANSFER as const
7a4cede
@marshacb
update dupes to account for casing
9a50ff9 
update dupes to account for white spaces in multilocations
@marshacb
lint
b821767
@marshacb
update SystemToSystem fetchCallType comment
f26a518
@marshacb
remove para to system foreign asset reserve transfer assets example
7c6bea2
@marshacb marshacb requested a review from IkerAlus July 27, 2023 23:15
@marshacb
update foreign asset multilocation to include parachain and or pallet…
1fe7f94 
… instance depending on origin
@marshacb
move constructForeignAssetMultiLocationFromAssetId to utils
86676bb 
import constructForeignAssetMultiLocationFromAssetId to ParaToSystem and SystemToPara
IkerAlus
IkerAlus requested changes Jul 28, 2023
View reviewed changes
Copy link
Contributor

@IkerAlus IkerAlus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just small suggestions, and an open discussion point about the sorting function. Great and solid work.

@IkerAlus IkerAlus self-requested a review August 2, 2023 10:40
IkerAlus
IkerAlus approved these changes Aug 2, 2023
View reviewed changes
Copy link
Contributor

@IkerAlus IkerAlus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Great work!

P.S: what is the status regarding the last to-do point "add foreign assets pallet instance from registry"? does that one belong to the registry repo?

@marshacb
Copy link
Contributor Author

marshacb commented Aug 2, 2023

LGTM! Great work!

P.S: what is the status regarding the last to-do point "add foreign assets pallet instance from registry"? does that one belong to the registry repo?

Yes, I've updated the registry to fetch the foreignAssetsPalletInstance already. After I do a release of the registry this morning it will be a simple change of pulling the pallet instance from there rather than using a constant.

@marshacb
update asset-transfer-api-registry
036397a 
update registry type to include foreignAssetsPalletInstance and poolPairsInfo
pull foreignAssetsPalletInstance from registry
@marshacb marshacb merged commit 58c776c into main Aug 2, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IkerAlus IkerAlus IkerAlus approved these changes

@TarikGul TarikGul TarikGul approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@marshacb @IkerAlus @TarikGul