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

Alchemic Logs: Small improvements for eth_getLogs #3664

Merged
merged 2 commits into from
Nov 14, 2023
Merged

Conversation

Shadowfiend
Copy link
Contributor

@Shadowfiend Shadowfiend commented Nov 9, 2023

This PR is focused on two quality-of-life improvements:

  • Move eth_getLogs to Alchemy. This bypasses certain restrictions on logs that exist on other providers we use and makes certain dApps much better/more functional (e.g. the Threshold dashboard). If we see too high a spike in usage, we can revert this back and come up with a different solution.
  • Reworks a bunch of weird type duplication that existed in the PreferencesService. No user-facing changes here, it's all in type-land.

Testing

  • Add account 0xafeacce4ad8b3b863ef72a7b7aa9d0e84ca71dfb as read-only, connect to https://dashboard.threshold.network/staking . Prior to this branch, the page should remain blank. After this branch, the page should fill in a “Stake - Native” entry with 58,333,338.75 T total staked balance.

Latest build: extension-builds-3664 (as of Mon, 13 Nov 2023 22:08:36 GMT).

@Shadowfiend Shadowfiend requested a review from ioay November 9, 2023 04:23
@Shadowfiend Shadowfiend force-pushed the alchemic-logs branch 2 times, most recently from a243fc0 to 6cc2dfa Compare November 9, 2023 21:57
@Shadowfiend Shadowfiend changed the title Alchemic Logs: Big ball of wax PR with initial notifications work Alchemic Logs: Small improvements for eth_getLogs Nov 13, 2023
Other providers impose various limits on eth_getLogs calls that can
break lookups. Alchemy does not impose these, so use Alchemy for these
calls. If the cost is too high, we can adjust differently via more
complex fallbacks, but this solves the problem more neatly and will
hopefully not spike costs too much.
There was a duplication of types between preferences/types and
preferences/db, mostly due to a missed duplication way back when the
preferences service was still being booted up. This commit eliminates
the duplication, harmonizes the types, and re-exports appropriate
external-facing types.

No underlying functionality has changed.
@Shadowfiend Shadowfiend marked this pull request as ready for review November 13, 2023 22:14
@Shadowfiend Shadowfiend requested review from jagodarybacka and removed request for ioay November 13, 2023 22:14
Copy link
Contributor

@jagodarybacka jagodarybacka left a comment

Choose a reason for hiding this comment

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

All good 🚀

@jagodarybacka jagodarybacka merged commit e14964b into main Nov 14, 2023
5 of 6 checks passed
@jagodarybacka jagodarybacka deleted the alchemic-logs branch November 14, 2023 12:23
@jagodarybacka jagodarybacka mentioned this pull request Nov 22, 2023
Shadowfiend added a commit that referenced this pull request Nov 23, 2023
## What's Changed
* Check connecting to dapp on different network after disconnecting by
@michalinacienciala in #3654
* v0.51.0 by @Shadowfiend in
#3652
* Fix typos by @xiaolou86 in
#3668
* Alchemic Logs: Small improvements for eth_getLogs by @Shadowfiend in
#3664
* Fix getting currently connected dapp info by @jagodarybacka in
#3671
* Add static subcape link to the wallet by @jagodarybacka in
#3670
* Fix e2e tests by @michalinacienciala in
#3651
* Add Alchemy endpoint for Arbitrum Sepolia by @jagodarybacka in
#3665
* Do notify: Set up base NotificationService by @Shadowfiend in
#3666

## New Contributors
* @xiaolou86 made their first contribution in
#3668

**Full Changelog**:
v0.51.0...v0.52.0

Latest build:
[extension-builds-3678](https://github.com/tahowallet/extension/suites/18415208655/artifacts/1067210815)
(as of Wed, 22 Nov 2023 14:53:11 GMT).
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.

None yet

2 participants