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

Refactor: use SDK amount.Amount type to store token amounts #2868

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

artursapek
Copy link
Collaborator

@artursapek artursapek commented Oct 21, 2024

Fixes #2853

Changes

  • Change Balance type, and amount values in Redux, to store an amount.Amount, not a number
  • Update validation code accordingly
  • Remove formatting hacks (removing commas, etc)
  • Remove formatAmount helper function (not required anymore)
  • Remove dead code in SDKv2 adapter

Overall cleans things up and net removal of code

https://www.loom.com/share/3b0d63df72c04b7182a41e0ca173e9cc

Copy link

netlify bot commented Oct 21, 2024

👷 Deploy request for wormhole-connect pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit a12c549

Copy link

netlify bot commented Oct 21, 2024

👷 Deploy request for wormhole-connect-mainnet pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit a12c549

@artursapek artursapek marked this pull request as draft October 21, 2024 17:13
@artursapek artursapek marked this pull request as ready for review October 21, 2024 21:51
@artursapek artursapek changed the title Refactor: use SDK amount.Amount type to store balances Refactor: use SDK amount.Amount type to store token amounts Oct 21, 2024
[props.routes, props.quotesMap],
);
const allRoutesFailed = useMemo(() => {
if (Object.keys(props.quotesMap).length === 0) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this fixes a bug unrelated to amount.Amount refactor: if you run every on an empty array, it always returns true.

@@ -39,7 +39,6 @@ const useFetchSupportedRoutes = (): HookReturn => {
supported = await route.isRouteSupported(
token,
destToken,
amount,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this argument was never used by isRouteSupported

@@ -1,37 +0,0 @@
import { TransferInfo } from '../utils/sdkv2';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we delete this file since it's now empty?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thought I had. thanks

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.

Refactor balances, store them as amount.Amount not string
2 participants