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.

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

fix: resolve tx calls based on current runtime #375

Merged

Conversation

marshacb
Copy link
Contributor

@marshacb marshacb commented Feb 29, 2024

Description

This PR refactors existing call selection logic into the resolveCall method on the AssetTransferAPI which determines the correct call type and call based on information obtained from the current runtime.

Changes

  • Refactored call selection logic into the resolveCall method
  • Added resolveCall unit tests for ParaTo,* SystemTo* and RelayTo* XCM directions
  • Fixed parachain primary asset tx construction to allow for xTokens transfers of parachain native assets (e.g. SDN -> Moonriver, MOVR -> Bifrost, etc.) which were previously constructed using only polkadotXcm.
  • Added RuntimeCallNotFound error variant
  • Updated parachain integration tests

closes: #368

@marshacb marshacb marked this pull request as ready for review February 29, 2024 16:57
@marshacb marshacb changed the title fix: resolve supported calls based on current runtime fix: resolve tx calls based on current runtime Feb 29, 2024
src/errors/BaseError.ts Outdated Show resolved Hide resolved
src/types.ts Outdated Show resolved Hide resolved
src/types.ts Show resolved Hide resolved
src/types.ts Show resolved Hide resolved
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

Overall, amazing job. Just some small nits but its clearly much cleaner and a nice touch for future work 🚀

Copy link
Contributor

@Imod7 Imod7 left a comment

Choose a reason for hiding this comment

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

There are some errors when I run yarn lint but they were not introduced from this PR. Just in case you would like to fix them.

Copy link
Contributor

@Imod7 Imod7 left a comment

Choose a reason for hiding this comment

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

Great work! 💯 It is much more clean & clear with the resolveCall method 👏

src/AssetTransferApi.ts Outdated Show resolved Hide resolved
src/createXcmCalls/util/establishXcmPallet.ts Outdated Show resolved Hide resolved
@TarikGul
Copy link
Member

TarikGul commented Mar 1, 2024

There are some errors when I run yarn lint but they were not introduced from this PR. Just in case you would like to fix them.

It's probably something specific to your local machine since the CI is passing.

remove unneeded undefined assignment
removed named tuple syntax for map types
@marshacb marshacb merged commit d55fa29 into main Mar 1, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolve calls based on a chains active runtime, and whether or not it is supported
4 participants