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: Fix useBalance delay when switching networks #1755

Merged
merged 11 commits into from
Jul 19, 2024

Conversation

chrstph-dvx
Copy link
Contributor

@chrstph-dvx chrstph-dvx commented Jul 15, 2024

Summary

We have an issue when user would switch source network from network A to network B, and balance from useBalanceOnSourceChain would be the balance on network A for a second. This led to useGasEstimate call despite the balance check.

Steps to test

@chrstph-dvx chrstph-dvx self-assigned this Jul 15, 2024
Copy link

vercel bot commented Jul 15, 2024

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

Name Status Preview Updated (UTC)
arbitrum-token-bridge ✅ Ready (Inspect) Visit Preview Jul 19, 2024 2:20pm

@chrstph-dvx chrstph-dvx force-pushed the fs-652-fix-useBalanceOnSourceChain-hook branch from 4d3c52b to 98f6e67 Compare July 17, 2024 15:35
@chrstph-dvx chrstph-dvx marked this pull request as ready for review July 18, 2024 08:57
@@ -82,54 +87,8 @@ describe('useBalance', () => {
expect(erc20Balances).toBeNull()
})

it('getter return null for missing chainId', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

why is this test deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It became invalid after the changes in useBalance. We accept chainId, not provider anymore

@chrstph-dvx chrstph-dvx changed the title fix: Fix useBalanceOnSourceChain hook fix: Fix useBalance delay when switching networks Jul 19, 2024
fionnachan
fionnachan previously approved these changes Jul 19, 2024
1
)
jest.mock('../../token-bridge-sdk/utils', () => ({
getProviderForChainId: function A() {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's function A?
Can't we just use anonymous function () => provider here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

100% yes, it's a leftover from debug (I would probably rename it to getProviderForChainIdMock rather than use anonymous function, it's easier to debug

@fionnachan fionnachan enabled auto-merge (squash) July 19, 2024 14:18
@fionnachan fionnachan merged commit 9e45ad4 into master Jul 19, 2024
24 checks passed
@fionnachan fionnachan deleted the fs-652-fix-useBalanceOnSourceChain-hook branch July 19, 2024 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants