Page MenuHomePhabricator
Differential D13463

[Cashtab] [Alias-server] - pt 2 - Incorporate pending aliases endpoint
ClosedPublic
Actions

Authored by emack on Mar 25 2023, 10:00.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Commits
rABC373e539248fd: [Cashtab] [Alias-server] - pt 2 - Incorporate pending aliases endpoint
Summary

T3003

Depends on D13367

This diff implements the pending endpoint via the new getPendingAliases and appendWithPendingAliases functions. The Registered Aliases list will now be based on the following logic:

  • user registers a new alias
  • upon successful tx broadcast, cashtab retrieves pending list from endpoint
  • adds '(pending)' to each pending alias retrieved
  • filters out ones not related to this wallet
  • updates the state variable in Alias.js (activeWalletAliases) that tracks the aliases to be displayed under Registered Aliases, simultaneously removing duplicates
  • if user is subsequently navigating to Alias.js again, the useEffect loop will call appendWithPendingAliases again to ensure it reflects the latest pending state

Note: refer to tg chat regarding current issue with the pending endpoint. Otherwise the diff should be displaying the correct pending aliases.

Test Plan
  • enable alias in ticker.js
  • npm start
  • register a new alias and ensure it gets listed under Registered Aliases on Alias.js with (pending) appended to name
  • navigate to home then navigate to Alias.js to ensure the useEffect() logic does not lose the newly added pending aliases
  • ensure the pending aliases match what is on https://aliasdev.etokens.cash/pending
  • switch to another wallet, register a new alias, note it's appearance on the pending endpoint
  • switch back to original wallet, navigate to Alias.js and ensure the alias registered with the other wallet is not displayed as pending
  • wait for a new block to be found and ensure the previously pending alias no longer has the (pending) text and can is now active

Diff Detail

Repository
rABC Bitcoin ABC
Branch
aliasServerIntegration
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 22693
Build 45006: Build Diffcashtab-tests
Build 45005: arc lint + arc unit

Event Timeline

emack created this revision.Mar 25 2023, 10:00
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 25 2023, 10:00
emack requested review of this revision.Mar 25 2023, 10:00
Harbormaster completed remote builds in B22693: Diff 38878.Mar 25 2023, 10:03
bytesofman requested changes to this revision.Mar 25 2023, 13:48
bytesofman added inline comments.
web/cashtab/src/components/Alias/Alias.js
181

This is sloppy but for now probably the best way to do it. Both Cashtab and the server are using the same websocket to trigger updates, and the server will take processing time after the websocket notification. We need to implement the same thing on the "on block" alias update checks.

An option for cleaning this up: I can add a websocket to the alias server. Then Cashtab just listens for new aliases.

Even then, we need to keep the 'on block' methods bc websockets are never 100% reliable.

web/cashtab/src/components/Common/Ticker.js
28

Since we will have multiple endpoints, we should only store the base Url in Ticker.js

Then we will make calls to {config.aliasServerBaseUrl}/pending etc

web/cashtab/src/utils/aliasUtils.js
41

If you don't get an API response, return false so that Alias.js knows you don't know the pending API info. Then handle that appropriately in Alias.js, i.e. just assume you have no pending aliases.

This revision now requires changes to proceed.Mar 25 2023, 13:48
emack updated this revision to Diff 38902.Mar 26 2023, 01:00
emack marked 2 inline comments as done.
  • changed all endpoint references to aliasServerBaseUrl
  • updated getPendingAliases to return false upon no api response
Harbormaster completed remote builds in B22708: Diff 38902.Mar 26 2023, 01:02
bytesofman requested changes to this revision.Mar 26 2023, 03:43
  • navigate to alias page with a wallet with no aliases
  • register pending99
  • it appears instantly as pending
  • try to register it again
  • it's allowed, then it appears two more times

image.png (719×582 px, 81 KB)

Looks like the existence of 'pending99' didn't stop a second tx from broadcasting. Also, when the app checked the api a second time, it saw "pending99" there twice (as the second registration was allowed) -- then added both of them

API design is to allow duplicate alias names at the pending endpoint bc it is meant to show all unconfirmed alias txs that have valid fees. This shouldn't be something cashtab needs to handle -- just need to make sure we prevent an alias from being re-registered. Esp as there is no confirmation modal to register an alias, someone 'accidentally' registering the same (expensive) alias twice in a row is likely to happen.

So -- this diff works for instantly showing a registered alias as pending. However, it does not work for preventing the registration of a pending alias.

If this is planned for a later diff, this is good to go. Otherwise the logic should be added here.

This revision now requires changes to proceed.Mar 26 2023, 03:43
emack updated this revision to Diff 38903.EditedMar 26 2023, 04:16

isAliasAvailable updated to check both registered and pending aliases in order to determine whether the user's chosen alias should be registered

Harbormaster completed remote builds in B22709: Diff 38903.Mar 26 2023, 04:19
bytesofman requested changes to this revision.Mar 26 2023, 04:46

wait for a new block to be found and ensure the previously pending alias no longer has the (pending) text and can is now active

image.png (666×1 px, 245 KB)

Looks like the aliases did update on block found, but the pending tag was not dropped

This revision now requires changes to proceed.Mar 26 2023, 04:46
emack updated this revision to Diff 38904.Mar 26 2023, 07:13

I realized getLatestAliases in useWallet.js was already updating aliasCache within the cashtabCache context object, but it wasn't being picked up by Alias.js' useEffect loop. Upon further digging I fixed this by updating the loop trigger from cashtabCache to cashtabCache.aliasCache.aliases. Upon finding a new block, Alias.js' pending aliases now drops their '(pending)' tag in real time.

Unit test mocks also had to be updated to include the aliasCache structure to avoid null pointers thrown by Jest.

Harbormaster completed remote builds in B22710: Diff 38904.Mar 26 2023, 07:15
emack retitled this revision from [Cashtab] [Alias] - pt 2 - Incorporate pending aliases endpoint to [Cashtab] [Alias-server] - pt 2 - Incorporate pending aliases endpoint.Mar 26 2023, 07:20
bytesofman accepted this revision.Mar 26 2023, 13:47
This revision is now accepted and ready to land.Mar 26 2023, 13:47
Closed by commit rABC373e539248fd: [Cashtab] [Alias-server] - pt 2 - Incorporate pending aliases endpoint (authored by emack). · Explain WhyMar 27 2023, 00:59
This revision was automatically updated to reflect the committed changes.
emack added a commit: rABC373e539248fd: [Cashtab] [Alias-server] - pt 2 - Incorporate pending aliases endpoint.

Revision Contents
Changeset List

PathSize
web/
cashtab/
src/
components/
Alias/
41 lines
Common/
1 line
utils/
27 lines

Diff 38878

View Options

web/cashtab/src/components/Alias/Alias.js

Loading...
View Options

web/cashtab/src/components/Common/Ticker.js

Loading...
View Options

web/cashtab/src/utils/aliasUtils.js

Loading...