Page MenuHomePhabricator

[lib] Batch ENS queries using ReverseRecords smart contract
ClosedPublic

Authored by ashoat on Oct 18 2023, 12:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Sep 17, 1:05 PM
Unknown Object (File)
Thu, Sep 12, 12:14 PM
Unknown Object (File)
Thu, Sep 12, 4:52 AM
Unknown Object (File)
Thu, Sep 12, 4:52 AM
Unknown Object (File)
Thu, Sep 12, 4:52 AM
Unknown Object (File)
Thu, Sep 12, 4:52 AM
Unknown Object (File)
Thu, Sep 12, 4:52 AM
Unknown Object (File)
Thu, Sep 12, 4:52 AM
Subscribers

Details

Summary

This resolves ENG-2593. For more info about this smart contract, see official ENS docs and GitHub repo.

When I start my iOS app, it needs to resolve 142 ENS names to build the in-memory search indices for @-mentioning and inbox search.

Before this diff, this meant 142 individual network requests to Alchemy. After this diff, it requires only 1.

Depends on D9524

Test Plan
  1. Included updated unit tests.
  2. I ran the iOS app and confirmed that the regression described here was resolved.
  3. I added a log statement to the code that issues a "single fetch" to confirm that it wasn't getting triggered anymore on iOS app start. (One "single fetch" was actually triggered, but it was for an ENS name that appeared immediately in my inbox... I think this is actually what we want, since we need that result more urgently.)
  4. I confirmed that ENS resolution still worked on the iOS app.
  5. I confirmed that ENS resolution still worked on the web app.
  6. I confirmed that the keyserver is still able to resolve ENS names in notifs.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fix behavior when there is no ENS name for a given address. Also add unit test for that case

lib/utils/ens-cache.js
61 ↗(On Diff #32155)

Prettier is forcing this line to be more than 80 chars

Minor (fix a comment and an invariant error message)

Distinguish null results from undefined results

ashoat added a reviewer: tomek.
lib/utils/ens-cache.js
77 ↗(On Diff #32158)

Typo: s/chaind/chain

I'm not an expert, but it seems reasonable and the test pan is thorough.

This revision is now accepted and ready to land.Oct 19 2023, 4:09 AM