Details

Reviewers

bytesofman
Fabien

Group Reviewers

Restricted Project

Commits

rABCe618271a66b9: [Cashtab][Alias] Update Send.js to use api endpoint in place of…

Summary

T3216

Update the alias lookup in send.js component's address input field to use the alias api endpoint

Test Plan

npm test
change the alias flag in Ticker.js to true
npm start
navigate to the Send page
input a pre-registered alias in the address input field and ensure it parses to the right address
ensure the hyperlink underneath links correctly to the explorer page for this address
send an XEC amount to this alias and ensure successful broadcast, and sanity check tx details in explorer
input a valid eCash address and ensure it is accepted
input a valid prefix-less eCash address and ensure it is accepted
input an invalid eCash address and invalid alias and ensure it triggers validation error
input an unregistered alias and ensure the eCash Alias does not exist or yet to receive 1 confirmation validation error is displayed

Diff Detail

Repository

rABC Bitcoin ABC

Branch

updateSendEndpoint

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 24443
Build 48486: Build Diff	cashtab-tests
Build 48485: arc lint + arc unit

Event Timeline

emack created this revision.Jul 5 2023, 08:38

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 5 2023, 08:38

emack requested review of this revision.Jul 5 2023, 08:38

Harbormaster completed remote builds in B24358: Diff 41261.Jul 5 2023, 08:38

bytesofman requested changes to this revision.Jul 5 2023, 22:17

bytesofman added inline comments.

cashtab/src/components/Common/Ticker.js
13 ↗	(On Diff #41261)	remove this param in a separate diff if it's no longer being used by Cashtab
cashtab/src/components/Send/Send.js
498 ↗	(On Diff #41261)	`getAliasDetails` should return the JSON directly

This revision now requires changes to proceed.Jul 5 2023, 22:17

emack mentioned this in D14219: [Cashtab][Alias] Update alias-server wrappers to return JSON.Jul 6 2023, 07:28

emack marked 2 inline comments as done.Jul 6 2023, 07:33

emack added inline comments.

cashtab/src/components/Common/Ticker.js
13 ↗	(On Diff #41261)	this will always be in use by Cashtab as it needs to know where to send the registration transaction to
cashtab/src/components/Send/Send.js
498 ↗	(On Diff #41261)	coming up via D14219, will rebase thereafter.

emack mentioned this in rABC5695d76dcf90: [Cashtab][Alias] Update alias-server wrappers to return JSON.Jul 8 2023, 06:53

rebased to use the new queryAliasServer wrapper

Harbormaster completed remote builds in B24419: Diff 41336.Jul 9 2023, 10:38

bytesofman requested changes to this revision.Jul 10 2023, 17:40

bytesofman added inline comments.

cashtab/src/components/Common/Ticker.js
13 ↗	(On Diff #41336)	this change still doesn't seem related to this diff -- in this case, diff is modifying behavior of send transactions that take an alias input. why are the test vectors for alias registration txs changing?
cashtab/src/utils/__tests__/transactions.test.js
122 ↗	(On Diff #41336)	why are these changing?

This revision now requires changes to proceed.Jul 10 2023, 17:40

emack marked 2 inline comments as done.Jul 11 2023, 07:05

emack added inline comments.

cashtab/src/utils/__tests__/transactions.test.js
122 ↗	(On Diff #41336)	these were changed due to the alias payment address changing but since that's been backed out, these will too.

Removed change to aliasPaymentAddress

Harbormaster completed remote builds in B24442: Diff 41370.Jul 11 2023, 07:08

Fabien added a subscriber: Fabien.Jul 11 2023, 07:16

Fabien added inline comments.

cashtab/src/components/Send/Send.js
500 ↗	(On Diff #41370)	You don't need the else. Unrelated: is the isRegistered flag really useful ? I suppose it was there because of the pending alias feature but this is likely no longer needed.

emack marked an inline comment as done.Jul 11 2023, 08:04

emack added inline comments.

cashtab/src/components/Send/Send.js
500 ↗	(On Diff #41370)	yea isRegistered is kind of redundant now, since I can just use the `!aliasDetails.address` check and if there is no address attached to an alias it's not registered. There is no scenario where a confirmed alias has no address attached.

Updated if statement and registration status check

Harbormaster completed remote builds in B24443: Diff 41372.Jul 11 2023, 08:07

bytesofman requested changes to this revision.Jul 11 2023, 17:38

bytesofman added inline comments.

cashtab/src/components/Send/Send.js
502	Check for `aliasDetails.error` -- the alias server validates input, so you can use its returned validation msg e.g. https://alias.etokens.cash/alias/invalid_alias {"error":"Error fetching /alias/invalid_alias: alias param cannot contain non-alphanumeric characters"}