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

feat: lazy Tx history / balance charts hasta la vista #9078

Draft
wants to merge 34 commits into
base: develop
Choose a base branch
from

Conversation

gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Mar 18, 2025

Description

Makes the Tx history lazy, one page per account on initial load, then the rest is all user-fetched.

TODO:

- [x] Base work happy
- [x] Ensure incoming Txs still happy
- [x] Any other features relying on historical Tx history and which may need to fetch it on-demand? 
  - [x] RFOX
  - [x] Arb Bridge
- [x] Find a way to make Arb bridge claims work
- [x] Ensure errored accounts still handled gracefully
  - [x] Can we still keep the errored accounts warning/tooltip ?
- [x] Bigly cleanup 
- [x] Desktop full regression/testing
- [x] Mobile full regression/testing
- [x] Tx history migration right before opening me

Issue (if applicable)

Risk

Extremely high theoretically, Tx history is a huge one.
Worst case scenario, Tx history is entirely rekt.
Realistically, you still get your first page of Tx and your incoming Tx history, the real risk is on:

  1. Reconciliation or user-action-based Tx history fetch being wrong
  2. Features depending on it (RFOX, Arb Bridge) being borked

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

Testing

TESTING NOTES ⚠️: When comparing Txs history after loading more, you may notice that:

  1. It seemingly does not "load more": it does, but loading more loads more pages as it should, which may not append a/some Txs but add some somewhere in the middle of your set. See below.
  2. The sets on prod and this diff may look different if you have large wallets with various activity times across accounts, unless you were to fully load all your Tx history in "Activity"
  3. The above two points also applies to Arb Bridge Claims and RFOX (which use a different logic of "fetch until a Tx is found, but say you have a native wallet with multiple accounts, this may not necessarily append a new Tx because of said time range shenanigans)

Those are both expected. Loading more does precisely that, loads more pages. However, not all accounts may have pages for the same time range, e.g

  • page 2 of my ETH account 0 may not have much timerange and encapsulate one day only (degen account)
  • while say my BTC account 1 could be a cold wallet account and have very few transactions ranging across 2 years
  • but my account 0 for BTC could have more usage with THOR swaps and one page encapsulates a few months.

Actual testing:

  • Accounts Tx history still loads (initial page of max 10 per account, or less if no Tx history)
  • Incoming Txs are still happy
  • Accounts autodiscovery is still happy
  • Clicking "Load More" on the Activity tab loads more for all accounts (eng. to confirm with XHRs)
  • Clicking "Load more" on an asset page loads more for all accounts of that chain only (eng. to confirm with XHRs)
  • Clicking "Load more" on an account asset page loads more for that account *only (eng. to confirm with XHRs)
  • RFOX activity tab is still happy and load more does scan until it finds more
  • Arb Bridge claims list is still happy and load more does scan until it finds more
  • Txs made while app is closed are still fetched when reopening the app

Engineering

  • ☝🏽

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)
  • ☝🏽

Screenshots (if applicable)

  • Smol EVM wallet

https://jam.dev/c/4cf74bff-cabe-4604-8034-1bef8a598914

  • Large Tx history EVM wallet

https://jam.dev/c/e24aa7f0-552b-4aab-abbf-bc340e21aea6

  • Native

https://jam.dev/c/614a8898-3d8d-4ce4-9cf7-079b2ebf76cf

Comment on lines +49 to +50
// Only fetch if we're connected, not currently loading (this one is weird as we may be going back and forth between loading and idle)
// *and* as a last resort *if* we're not fetching Tx history, which is a bit of a hack, but it's a last resort to avoid blocking the main thread
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not strictly part of this PR but might as well given our current stance on opportunities, will help later refactor. This was spewing a lot of XHRs on app load without a wallet, which was turbo annoying when trying to improve XHRs perfs.

@gomesalexandre gomesalexandre force-pushed the feat_tx_history_things branch from ac6e3be to e94ea02 Compare March 18, 2025 19:29
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The same as previous <DashboardChart />, without the chart. Think we should be safe to remove it as part of this PR?

@gomesalexandre gomesalexandre changed the title [skip ci] wip: lazy Tx history / balance charts hasta la vista feat: lazy Tx history / balance charts hasta la vista Mar 19, 2025
@gomesalexandre gomesalexandre marked this pull request as ready for review March 19, 2025 10:07
@gomesalexandre gomesalexandre requested a review from a team as a code owner March 19, 2025 10:07
@gomesalexandre gomesalexandre marked this pull request as draft March 19, 2025 17:16
@gomesalexandre
Copy link
Contributor Author

Actually ready but moving to draft, pending discussions with @twblack88. We may want to change something major here which will cause rewrite/refactor so let's not waste review time for now.

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.

Auto fetching Balance chart
1 participant