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: save user announcements to local storage and last fetched block #685

Merged
merged 38 commits into from
Jul 11, 2024

Conversation

marcomariscal
Copy link
Contributor

@marcomariscal marcomariscal commented Jun 18, 2024

Description

Updates to fetch and scan only the latest blocks. The latest blocks are those after the initial scan, where each subsequent scan saves the latest fetched block to local storage, allowing for appropriate updating of the scan range:

  1. Initial scan --> fetch/scan blocks 1 to x
  2. Next scan --> fetch/scan blocks x + 1 to latest block (y)
  3. Subsequent scan --> fetch/scan blocks y + 1 to latest block
    etc.

Methodology:

  • User announcements are updated in local storage on each fetch/scan
  • The latest fetched block is updated in local storage on each fetch/scan
  • The most recent announcement block data is updated in local storage on each fetch/scan to accurately show if there are no announcements fetched in the current scan
  • The startBlock for the scan is appropriately updated to the latest fetched block

Other Updates:

  1. Show fetching/scanning latest verbiage above the top right of the AccountReceiveTable, intentionally omitting the large scan indicator at the bottom of the table
  2. Implemented a clear local storage button with auto app refresh

Comments:

  • There were instances where querying the subgraph potentially hung (using goldsky, so outdated); while this may not be directly affected by this PR, it's worth noting for testing purposes

To Test:

  1. Perform the initial scan using account A
  2. Navigate to a different route/page using account A
  3. Validate: Navigate back to the receive page to confirm:
    a) The initial user announcements are present
    b) The fetching/scanning sequence is reflected appropriately (it's fetching/scanning only the latest blocks)
  4. Perform a send transaction from a different account to account A, then navigate to the receive view connected with account A; validate as in step 3
  5. Test the clear local storage functionality and ensure the app refreshes correctly

Copy link

netlify bot commented Jun 18, 2024

Deploy Preview for jolly-shaw-20fe62 ready!

Name Link
🔨 Latest commit b627889
🔍 Latest deploy log https://app.netlify.com/sites/jolly-shaw-20fe62/deploys/669020c0f4be990008541698
😎 Deploy Preview https://deploy-preview-685--jolly-shaw-20fe62.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@marcomariscal marcomariscal marked this pull request as ready for review June 25, 2024 17:59
@marcomariscal marcomariscal changed the title feat: cache user announcements and last fetched block feat: save user announcements to local storage and last fetched block Jun 25, 2024
@marcomariscal marcomariscal requested review from garyghayrat and jferas and removed request for jferas June 25, 2024 18:33
Copy link
Contributor

@jferas jferas left a comment

Choose a reason for hiding this comment

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

Ran into an interesting UX loose end.. connected my wallet and did scan for received transactions.. had to sign to get them.. did so and got the list displayed.. went back to home and hit the receive tab again.. got the list immediate. Very nice...

Then closed that browser tab.. went back to the app via the deploy preview URL.. went to scan for receives, and got the list I had previously as well as a request to sign. Something might need to be cleared?
Screenshot 2024-06-25 at 3 37 56 PM

@marcomariscal
Copy link
Contributor Author

Ran into an interesting UX loose end.. connected my wallet and did scan for received transactions.. had to sign to get them.. did so and got the list displayed.. went back to home and hit the receive tab again.. got the list immediate. Very nice...

Then closed that browser tab.. went back to the app via the deploy preview URL.. went to scan for receives, and got the list I had previously as well as a request to sign. Something might need to be cleared? Screenshot 2024-06-25 at 3 37 56 PM

Yee, that's intentional. When the user refreshes the page, they need to sign again to scan because the keys are no longer available in memory. Sensitive data won't be cached to ensure security and privacy.

We've updated the sign-to-scan verbiage slightly for this scenario but welcome any feedback on clearer or less confusing language, and the ux in general.

marcomariscal and others added 5 commits July 9, 2024 11:49
* fix: explicitly sort the tokens by addr

* fix: use vm.computeCreateAddress

* fix: mirror test sender params

* fix: use actual owner

* fix: add back gnosis

* Remove all reference to INFURA_ID (#687)

---------

Co-authored-by: John Feras <[email protected]>
Copy link

Coverage after merging feat/last-fetched-block-usage into master will be

81.12%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
contracts-periphery/src
   UmbraBatchSend.sol100%100%100%100%
   UniswapWithdrawHook.sol81.82%100%66.67%83.33%14
umbra-js/src
   ethers.ts76.27%100%61.11%100%
   types.ts50%100%0%100%
umbra-js/src/classes
   KeyPair.ts98.65%96.49%100%100%31–32
   RandomNumber.ts100%100%100%100%
   StealthKeyRegistry.ts100%100%100%100%
   TxHistoryProvider.ts77.55%76.47%66.67%79.31%11, 11, 19–20, 31, 38, 44, 53, 57, 8
   Umbra.ts75.41%63.69%82.93%82.09%105, 135–136, 157, 229–231, 315–318, 380, 394, 401–403, 405–406, 409–410, 428, 430, 432, 432, 432, 432–433, 433, 433–435, 450, 450, 450–451, 455, 458, 478, 496–498, 585, 602, 615–616, 626–627, 640, 643–644, 674–675, 751, 755, 755, 755–758, 758, 758, 758, 758, 758, 758–759, 759, 759, 759, 759, 759, 759, 759, 759, 759, 759, 761, 761, 761–762, 765, 765, 765, 765, 765–766, 769, 778, 778, 787, 790, 790, 790–792, 805, 809, 812–813, 819–821, 828, 828, 828–831, 862, 867–868, 868–869
umbra-js/src/typechain
   index.ts70.59%100%37.50%100%
umbra-js/src/typechain/factories
   index.ts100%100%100%100%
umbra-js/src/typechain/factories/@OpenZeppelin
   index.ts100%100%100%100%
umbra-js/src/typechain/factories/@openzeppelin/contracts
   index.ts100%100%100%100%
umbra-js/src/typechain/factories/@openzeppelin/contracts/access
   Ownable__factory.ts50%100%0%66.67%70, 76
   index.ts50%100%0%100%
umbra-js/src/typechain/factories/@openzeppelin/contracts/token
   index.ts100%100%100%100%
umbra-js/src/typechain/factories/@openzeppelin/contracts/token/ERC20
   ERC20__factory.ts29.03%0%12.50%47.06%310, 314, 314, 314–315, 317, 326, 326, 326, 333, 333, 333, 336, 339, 345
   IERC20__factory.ts50%100%0%66.67%201, 204
   index.ts50%100%0%100%
umbra-js/src/typechain/factories/contracts
   IUmbraHookReceiver__factory.ts50%100%0%66.67%61, 67
   MockHook__factory.ts22.58%0%0%41.18%107, 111, 111, 111–112, 114, 121, 121, 121, 126, 126, 126, 129, 132, 138, 144
   StealthKeyRegistry__factory.ts22.58%0%0%41.18%202, 206, 206, 206–207, 209, 216, 216, 216, 221, 221, 221, 224, 227, 233, 239
   TestToken__factory.ts64.52%66.67%50%70.59%329–330, 348, 351, 354, 360, 366
   Umbra__factory.ts70.97%66.67%62.50%76.47%495–496, 525, 529, 532, 538
   index.ts50%100%0%100%
umbra-js/src/utils
   cns.ts50%33.33%50%55%33–36, 40, 40, 40, 40, 40–41, 44–46
   constants.ts100%100%100%100%
   ens.ts36.11%25%33.33%40%25–27, 35, 56–57, 57, 57–58, 63–64, 66, 70, 70, 70, 70, 70–71, 75–77
   utils.ts76.67%68.66%76.92%82%109–111, 113–115, 121, 139–140, 169, 190, 246, 253–260, 260, 260, 260, 260, 260, 260–261, 263, 294, 296–297, 306, 306, 306–307, 309, 312, 321–322, 363, 380, 380, 380, 400, 429, 431, 436, 448, 448–449, 454, 456–457, 468–472, 478, 480, 522–523, 523, 523–525, 525, 525, 536, 59–60, 779–780, 792–793, 85, 93, 93
umbra-js/test
   testPrivateKeys.ts100%100%100%100%
   utils.ts100%100%100%100%

@garyghayrat
Copy link
Member

Thank you @marcomariscal !

@garyghayrat garyghayrat merged commit a13be3a into master Jul 11, 2024
9 checks passed
OKEAMAH pushed a commit to OKEAMAH/umbra-protocol that referenced this pull request Jul 11, 2024
…ScopeLift#685)

* feat: set the last fetched block as the start block

* feat: handle caching user announcements and latest fetched block

* feat: show user announcements if there are any

* fix: handle watching/loading announcements

* fix: parse out lastFetchedBlock and fix user announcement loading logic

* chore: log

* feat: handle block data caching

* feat: show most recent block data if exists

* fix: type check

* feat: handle user announcements already present and sign language

* feat: only show fetching when no user announcements

* feat: fetching latest from last fetched block component

* feat: fetching latest translation for cn

* feat: clear local storage button and functionality

* fix: start block handling logic

* feat: dedupe user announcements

* fix: logic

* fix: minimize debugging logs on userAnnouncement changes

* feat: handle scanning latest announcements from last fetched block

* feat: sort by timestamp explicitly

* feat: no loading sequence when there are announcements

* fix: need sig lately verbiage

* fix: add need sig lately to cn

* fix: little more mb

* fix: no withdraw verbiage on need-sig-lately

* feat: handle need sig

* Update frontend/src/i18n/locales/en-US.json

Co-authored-by: Gary Ghayrat <[email protected]>

* feat: handle sign button instead of needs sig

* Update frontend/src/i18n/locales/zh-CN.json

Co-authored-by: Gary Ghayrat <[email protected]>

* fix: move local storage clear button above lang

* fix: spacing more uniform

* fix: use computed ref as param, and set setIsInWithdrawFlow to false on mount

* feat: sign and withdraw

* fix: contract periphery tests (ScopeLift#688)

* fix: explicitly sort the tokens by addr

* fix: use vm.computeCreateAddress

* fix: mirror test sender params

* fix: use actual owner

* fix: add back gnosis

* Remove all reference to INFURA_ID (ScopeLift#687)

---------

Co-authored-by: John Feras <[email protected]>

* fix: use balanceIndex to ensure that the correct balance is fetched from the stealthBalances array

* fix: dedupe by tx hash and receiver instead of just tx hash

* fix: include receiver to derive isWithdrawn

* fix: img

---------

Co-authored-by: Gary Ghayrat <[email protected]>
Co-authored-by: John Feras <[email protected]>
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.

3 participants