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: ensure token lists and balances are loaded for new safe or network #128

Merged
merged 4 commits into from
Feb 22, 2024

Conversation

gsteenkamp89
Copy link

fixes UMA-2395 UMA-2388

motivation

There was a bug in the transaction builder where the app would load all tokens you have balances for (your safe on a certain network), then when you switch safes, that list of tokens would not update. We also were not including the balance of the native token in the token chooser modal. This was because the balance pulled from gnosis, if that balance was for a native token it would be stripped out by the app, then later we just hardcoded in the native asset for the chain as an option, but without the balance info.

This PR rectifies some of the state management so that we relaod token balances and token lists when we switch safes.
It also ensures we keep the safe's balance for native tokens so we can display this data to the user.

Copy link

vercel bot commented Feb 22, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
snapshot ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 22, 2024 3:27pm
snapshot-goerli ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 22, 2024 3:27pm

@gsteenkamp89 gsteenkamp89 changed the base branch from uma to master February 22, 2024 13:00
@gsteenkamp89 gsteenkamp89 requested a review from daywiss February 22, 2024 13:00
@gsteenkamp89 gsteenkamp89 changed the title Gerhard/uma 2395 osnap snapshot tx builder bug fix: ensure token lists and balances are loaded for new safe or network Feb 22, 2024
const amount = ref(props.transaction.amount ?? '');
const recipient = ref(props.transaction.recipient ?? '');
const tokens = ref<Token[]>([nativeAsset, ...props.tokens]);
const tokens = ref<Token[]>(props.tokens);
Copy link

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 keep the native asset as first token in the list. is that possible?

Copy link
Author

Choose a reason for hiding this comment

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

yeah that's possible 👍

Copy link

@daywiss daywiss left a comment

Choose a reason for hiding this comment

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

great, i will pr this

@daywiss
Copy link

daywiss commented Feb 22, 2024

prd to snapshot-labs/snapshot-v1#4576

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.

2 participants