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

feat: enable IBC withdrawals for all tokens #1243

Merged
merged 1 commit into from
Nov 14, 2024
Merged

Conversation

emccorson
Copy link
Collaborator

@emccorson emccorson commented Nov 12, 2024

This PR allows IBC withdrawals for all assets, instead of just NAM.

There is still zero indication to the user about whether the transaction failed or succeeded, but for testing this information should be displayed in the console.


Changed

  • Use base denom amounts when building IBC transfers with JS SDK
  • Use unknown gas limit value for txs without an indexer type

Added

Fixed

@emccorson emccorson force-pushed the ibc-withdraw-all-assets branch 2 times, most recently from 5a4c920 to cbf960c Compare November 12, 2024 10:04
@emccorson emccorson marked this pull request as ready for review November 12, 2024 10:13
@emccorson emccorson changed the title Support all assets for IBC Withdrawal + bump indexer to new package feat: enable IBC withdrawals for all tokens Nov 12, 2024
@emccorson emccorson force-pushed the ibc-withdraw-all-assets branch 2 times, most recently from a54e752 to bf36c9b Compare November 12, 2024 13:08
Copy link
Contributor

@pedrorezende pedrorezende left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -0,0 +1,13 @@
export const txKinds = [
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to discuss how we're going to organize the types in the codebase. One advantage of having them in a root folder is that is easy to find, import, and maintain. Do you think we should break the types in different files?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I actually did this just because I needed a const and I couldn't do that in types.d.ts. Now that your PR changes it to types.ts, we could still use that but I'll leave that until we discuss how to handle it.

apps/namadillo/src/atoms/fees/services.ts Outdated Show resolved Hide resolved
Closes #1242

- Use base denom amounts when building IBC transfers with JS SDK
- Use unknown gas limit value for txs without an indexer type
- Use same gas limit and price for IBC withdrawal as rest of Namadillo
- Reveal public key if needed for IBC withdrawal. Fixes #1222
@emccorson emccorson merged commit 14106eb into main Nov 14, 2024
9 checks passed
@emccorson emccorson deleted the ibc-withdraw-all-assets branch November 14, 2024 07:26
@github-actions github-actions bot temporarily deployed to production November 14, 2024 07:32 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants