Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

fix: throttle balance syncer + remove WC patch #3848

Merged
merged 1 commit into from
May 4, 2022
Merged

Conversation

iamacook
Copy link
Member

@iamacook iamacook commented May 3, 2022

What it solves

Resolves eth_getBalance over-polling.

How this PR fixes it

Onboard's balance state syncer has been throttled from every 1s to every 1h. As such, the patched WC module has also been reverted to the default.

How to test it

  • Connect to WC and observe no eth_getBalance over-polling.
  • Connect to a Ledger/Trezor and observe the same.
  • Pairing should work as expected.

@iamacook iamacook self-assigned this May 3, 2022
@github-actions
Copy link

github-actions bot commented May 3, 2022

CLA Assistant Lite All Contributors have signed the CLA.

import { ChainId } from 'src/config/chain'
import { getWCWalletInterface, getWalletConnectProvider } from 'src/logic/wallets/walletConnect/utils'
Copy link
Member Author

Choose a reason for hiding this comment

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

This logic was originally abstracted as it was used in the patched WC module as well as here.

@iamacook iamacook requested review from katspaugh and usame-algan May 3, 2022 08:54
@github-actions
Copy link

github-actions bot commented May 3, 2022

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@coveralls
Copy link

Pull Request Test Coverage Report for Build 2262846367

  • 3 of 14 (21.43%) changed or added relevant lines in 5 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.04%) to 35.39%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/logic/wallets/store/middleware/index.ts 0 1 0.0%
src/logic/wallets/pairing/module.ts 0 10 0.0%
Files with Coverage Reduction New Missed Lines %
src/logic/wallets/pairing/module.ts 1 42.11%
Totals Coverage Status
Change from base Build 2262452500: 0.04%
Covered Lines: 3590
Relevant Lines: 9204

💛 - Coveralls

@github-actions
Copy link

github-actions bot commented May 3, 2022

Deployment links

🟠 Rinkeby Mainnet 🟣 Polygon 🟡 BSC Arbitrum 🟢 Gnosis Chain

Copy link
Member

@katspaugh katspaugh left a comment

Choose a reason for hiding this comment

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

Getting a deja vu :)

Have you tested it with different chains?

@iamacook
Copy link
Member Author

iamacook commented May 3, 2022

Have you tested it with different chains?

I have indeed.

@francovenica
Copy link
Contributor

Connected to WC and there is no eth_getBalance at all, not even at the beggining, but that's the way is already in dev, so I assume is a normal behavior
For trezor and ledger the eth_getBalance is only called when the addreses the devices are loaded and never again on future pollings.
Made a dummy tx with every device just making sure the pairing works fine

The only 2 networks with trez and ledger connection are Ethereum and rinkeby (the ones we have at least). It works the same in both of them.

Looks good to me

@iamacook iamacook merged commit 23eeb7a into dev May 4, 2022
@iamacook iamacook deleted the throttle-onboard branch May 4, 2022 07:12
@github-actions github-actions bot locked and limited conversation to collaborators May 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants