Page MenuHomePhabricator
Differential D10758

[Cashtab] Advanced Tooling + Airdrop facility
ClosedPublic
Actions

Authored by emack on Jan 4 2022, 07:07.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Commits
rABC87bdcfb9d033: [Cashtab] Advanced Tooling + Airdrop facility
Summary

New 'Tools' screen proposed to house the following upcoming functions:

  • T1984
  • T1970
  • T1717
  • T2055
  • T1992
  • T2080
  • T2017

T1984 (Airdrops) is the first cab off the rank and is included as part of this diff.

Tested across web and mobile devices.

Explainer video: T2185

Test Plan
  • npm start
  • create a new eToken, send it to a bunch of addresses and note the Token ID
  • navigate back and forth between the Tools screen and other existing screens to ensure no state-related errors
  • navigate to Tools and expand the Airdrop section. Ensure the 'Calculate Airdrop', 'Copy to Send screen' and 'Copy to Clipboard' buttons are all disabled by default and the balance header is displayed.
  • fill in the ID and airdrop total, ensuring that the 'Calculate Airdrop' button is only enabled once both fields have valid input
  • fill in spaces/symbols/non-64 char string for the token ID and ensure a validation error is displayed and Airdrop button is disabled
  • attempt to input alphanumeric figures or symbols for the dividend total and ensure input is prevented by the frontend validation
  • Fill in with valid input for token ID and total airdrops, then click 'Calculate Pro-Rata Airdrop'.
  • ensure the airdrop for each address is proportional to their holding of the eToken.
  • if the airdrop for an address is too small resulting in sub-dust outputs, verify a soft error message is displayed notifying the user. This still allows the user to click on the now-enabled 'Copy to Clipboard' and Copy to Send screen' buttons. When clicking on the 'Copy to Send screen' button, ensure the Send screen validation picks up on the less than dust error as well.
  • if the airdrop is adequately sized where all outputs are above dust, verify 'Copy to Clipboard' correctly copies the multiple recipient outputs.
  • verify the 'Copy to Send screen' button routes the user to the Send screen with the sending mode defaulting to 'one to many' and the recipients field already populated.
  • click send and verify a successful one to many transaction in explorer and the app's Tx History.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
emack added a subscriber: Klakurka.Jan 30 2022, 06:35

Updated airdrop button label and progress modal message as per @Klakurka feedback from T2185.

Harbormaster completed remote builds in B18069: Diff 32070.Jan 30 2022, 06:39
bytesofman requested changes to this revision.Feb 1 2022, 19:46
  1. Run grep -r 'dividend' src/ and replace dividend with airdrop for all variable and function names
  2. Tools.js should have unit tests similar to Send.js or Wallet.js
web/cashtab/src/components/Common/CustomIcons.js
76 ↗(On Diff #32070)

Convention is for jsx components to be capitalized, not camel case

Please change customSpinner to CustomSpinner

web/cashtab/src/components/Send/Send.js
175 ↗(On Diff #32070)

Let's be consistent with airdrop through variable naming as well, otherwise will be confusing for future devs

location.state.dividendRecipients --> location.state.airdropRecipients

178 ↗(On Diff #32070)

airdropRecipients

web/cashtab/src/components/Tools/Tools.js
71 ↗(On Diff #32070)

handleTotalAirdropInput

77 ↗(On Diff #32070)

calculateXecAirdrop

This revision now requires changes to proceed.Feb 1 2022, 19:46
emack updated this revision to Diff 32117.Feb 2 2022, 01:15
emack marked 5 inline comments as done.
  • Added Tools.test.js and Tools snapshots
  • Full refactor of all dividend references in src/
  • Updated CustomSpinner jsx name
  • A separate follow up diff will be made to incorporate exclusion lists
Harbormaster completed remote builds in B18100: Diff 32117.Feb 2 2022, 01:18
bytesofman requested changes to this revision.Feb 3 2022, 03:28

If I click 'copy to send screen' with airdrop output that includes this warning, the validation error msg on send screen does not appear. It will appear if I adjust anything in the text area.

e.g. this happens when I use this input: 4bd147fc5d5ff26249a9299c46b80920c0b81f59a60e05428262160ebee0b0c3 for tokenId and 1000000 for airdrop amount

web/cashtab/src/components/Tools/Tools.js
187 ↗(On Diff #32117)

I think the spinner and no message is better. In practice, it will take longer for eTokens with more holders. We can add better handling for this in future diffs, not necessary for MVP.

Please remove   This normally takes a few seconds...

This revision now requires changes to proceed.Feb 3 2022, 03:28
emack updated this revision to Diff 32151.Feb 3 2022, 04:14
emack marked an inline comment as done.
  • enforced validation for outputs propagated to the send screen
  • removed querying modal message on response time
emack edited the test plan for this revision. (Show Details)Feb 3 2022, 04:15
Harbormaster completed remote builds in B18123: Diff 32151.Feb 3 2022, 04:18
bytesofman requested changes to this revision.Feb 3 2022, 22:58
bytesofman added inline comments.
web/cashtab/src/components/Tools/Tools.js
210 ↗(On Diff #32151)

The validatestatus and help fields go on the <Input> element; this way you can prevent a user from calculating an airdrop with an invalid tokenID

263 ↗(On Diff #32151)

also disable if the tokenId input is invalid

This revision now requires changes to proceed.Feb 3 2022, 22:58
emack edited the test plan for this revision. (Show Details)Feb 4 2022, 02:07
emack updated this revision to Diff 32183.Feb 4 2022, 02:34
emack marked an inline comment as done.
  • enforced 64 char eToken ID validation
  • updated isValidTokenId unit tests for 64 char scenarios
  • the Calculate Airdrop button remains disabled until both eToken ID and Total Aidrop inputs are validated
Harbormaster completed remote builds in B18145: Diff 32183.Feb 4 2022, 02:38
emack updated this revision to Diff 32405.Feb 16 2022, 12:17

Rebased to new UI

Harbormaster completed remote builds in B18286: Diff 32405.Feb 16 2022, 12:20
bytesofman requested changes to this revision.Mar 3 2022, 00:46
  1. Match width of other Collapse components

image.png (308×594 px, 21 KB)

vs Send Screen:

image.png (179×484 px, 6 KB)

  1. Match balance header to Send Screen
  1. Inline comments
web/cashtab/src/components/Tools/Tools.js
325 ↗(On Diff #32405)

What's the rationale for using this here vs the react-copy-to-clipboard module used for the QR code?

"Not using a module" is a good enough rationale. But, if this approach really is better, we should also create a task to deprecate react-copy-to-clipboard.

web/cashtab/src/utils/validation.js
287 ↗(On Diff #32405)

I think some structures would pass this while still being invalid, e.g. address, 5.5, 432, or anything else with extra entries. Not sure how or why we would get that, but we might as well validate for the specific structure we want since that's the only thing Electrum or Cashtab multi-send will take.

So -- validate addressStringArray[i].split(',') has length 2

Add unit tests + mocks for this case

This revision now requires changes to proceed.Mar 3 2022, 00:46
emack updated this revision to Diff 32590.Mar 4 2022, 12:23
emack marked 2 inline comments as done.
  • matched width with other Collapse components
  • matched balance header to Send screen
  • the approach to use the js ClipBoard API (navigator.clipboard.writeText) rather than importing a module was indeed a decision based on minimizing reliance on external libraries. Have tested this across Chrome, Firefox, Brave, Safari and in Extension/iOS/Android. The react-copy-to-clipboard module is also based on copy-to-clipboard, which uses the now obsolete Document.execCommand() approach. I'll create a new task to deprecate existing usage of copy to clipboard mobules.
  • isValidAirdropOutputsArray updated to mitigate array structures with additional values per line, along with unit tests and mocks.
  • adjusted color contrast in props.theme.buttons.styledLink and double checked it was only used in Tools.js
Harbormaster completed remote builds in B18401: Diff 32590.Mar 4 2022, 12:27
bytesofman requested changes to this revision.Mar 4 2022, 20:02
  1. CSS tweak on these buttons. Need a better hover effect here. I think best approach is match the css on the "Reply to msg" buttons seen in tx histor of op_return msgs

image.png (90×373 px, 6 KB)

image.png (37×142 px, 1 KB)

  1. Handle undefined output and throw an error, e.g. try sending airdrop of 1,000,000 to 4de69e374a8ed21cbddd47f2338cc0f479dc58daa2bbe11cd604ca488eca0ddf (SPICE token, one with many outputs, query times out SLPDB); right now app will lock up at 50% progress indefinitely.

image.png (148×755 px, 36 KB)

  1. In-line comment, !== not !=
web/cashtab/src/utils/validation.js
342 ↗(On Diff #32590)

!==

This revision now requires changes to proceed.Mar 4 2022, 20:02
emack updated this revision to Diff 32609.Mar 6 2022, 13:34
emack marked an inline comment as done.
  • Aligned airdrop action buttons to match CSS used with the Reply to Message button.
  • calculateXecAirdrop now catches undefined/timeout responses from SLPDB and triggering an Error Notification. Now mitigates the spice token example.
  • updated validation comparison.
Harbormaster completed remote builds in B18415: Diff 32609.Mar 6 2022, 13:38
emack updated this revision to Diff 32682.EditedMar 13 2022, 02:14

I reviewed the end to end airdrop UX and I feel the direct routing from the Token Details component to the Airdrop screen is a must have for the initial MVP diff. Otherwise the copy & paste ID approach is too primitive and unintuitive for production.

image.png (157×565 px, 17 KB)

  • added routing link to the SendToken screen which passes the eToken ID as the state param
  • updated Tools component to check for the presence of this state param, and if exists, open the the calculator dropdown by default and prepopulate the etoken ID field.
Harbormaster completed remote builds in B18470: Diff 32682.Mar 13 2022, 02:17
bytesofman requested changes to this revision.Mar 14 2022, 22:00

image.png (43×174 px, 2 KB)

image.png (38×168 px, 2 KB)

Please match the background + hover effects here to be the same as the repy to buttons in tx history, even though the background color of the holding div is different.

+ rebase to latest master for unit test fixes and run npm test

This revision now requires changes to proceed.Mar 14 2022, 22:00
emack updated this revision to Diff 32711.Mar 15 2022, 12:57

Rebased to latest master and aligned button styling to the Reply button.

Harbormaster completed remote builds in B18487: Diff 32711.Mar 15 2022, 13:00
bytesofman requested changes to this revision.Mar 15 2022, 16:27

OK, this is almost ready to go.

One thing -- I think with the unlimited potential for future features, we should account for adding a hamburger or pop-up to choose between multiple screens, and not a catch-all "tools" screen.

For now, this is a significant enough feature for a dedicated "Airdrop" screen. When we need to add another screen, we will replace 'Settings' with a expand menu that includes Settings and the next screen.

So,

  1. Please change the 'Tools' page to be 'Airdrop' page
  2. Please find an appropriate icon for the page (e.g. box with parachute)
This revision now requires changes to proceed.Mar 15 2022, 16:27
emack updated this revision to Diff 32823.Mar 18 2022, 07:42
  • refactored Tools component to Airdrop
  • updated App.js routing references across web and extension mode
  • updated unit tests and snapshots
  • updated Airdrop icon
  • retested across web, plugin, iOS and Android.
Harbormaster completed remote builds in B18555: Diff 32823.Mar 18 2022, 07:45
bytesofman requested changes to this revision.Mar 18 2022, 19:44

Looks like npm run extension was run on a node version without browserify

The extension.sh script should be improved to avoid this, I'll create a task.

In the meantime -- please back out the changes introduced in this diff to manifest.json, App.js, and App.css

This revision now requires changes to proceed.Mar 18 2022, 19:44
emack updated this revision to Diff 32861.Mar 19 2022, 00:55
  • Partial revert of previous diff due to the extension script issue
  • Fixed edge case where some etoken results were returning values with large decimal places. The resultString in Airdrop component now limits to 'currency.cashDecimals' for decimal places. (e.g. 5d0ce86026751642c454f0c209d787f3a9c13322092bb654d55778a6d2d18a41 and e4130a0dd36c6f58c208b5adf5901060843b59dd1be7f374584ed918179937a6)
Harbormaster completed remote builds in B18579: Diff 32861.Mar 19 2022, 00:59
bytesofman requested changes to this revision.Mar 21 2022, 17:33
bytesofman added inline comments.
web/cashtab/src/utils/cashMethods.js
932 ↗(On Diff #32861)

I don't think we get any benefit having this wrapped in its own function. The way this works currently, in the event of an error from getLatestBlockHeight we will be passing latestBlock as a bad var, which will presumably kick out of the try..catch loop where this function is called.

Instead of this separate function, use this try...catch logic to get the latest block inside the calculateXecAirdrop function. That way you can have a more specific and useful error message if the calculateXecAirdrop function is failing.

This revision now requires changes to proceed.Mar 21 2022, 17:33
emack updated this revision to Diff 32875.Mar 22 2022, 01:00

Moved blockheight retrieval logic from cashmethods into the Airdrop component.

Harbormaster completed remote builds in B18585: Diff 32875.Mar 22 2022, 01:04
bytesofman accepted this revision.Mar 22 2022, 13:10
This revision is now accepted and ready to land.Mar 22 2022, 13:10
aimal added a subscriber: aimal.Mar 22 2022, 15:27
Closed by commit rABC87bdcfb9d033: [Cashtab] Advanced Tooling + Airdrop facility (authored by emack). · Explain WhyMar 22 2022, 23:49
This revision was automatically updated to reflect the committed changes.
emack added a commit: rABC87bdcfb9d033: [Cashtab] Advanced Tooling + Airdrop facility.

Revision Contents
Changeset List

PathSize
web/
cashtab/
extension/
src/
components/
15 lines
170 lines
1 line
src/
assets/
34 lines
styles/
1 line
components/
Airdrop/
468 lines
__tests__/
115 lines
__snapshots__/
15 lines
Common/
10 lines
5 lines
1 line
Configure/
__tests__/
__snapshots__/
Home/
__tests__/
__snapshots__/
Receive/
__tests__/
__snapshots__/
Send/
16 lines
43 lines
__tests__/
__snapshots__/
Tokens/
__tests__/
__snapshots__/
utils/
__mocks__/
27 lines
__tests__/
56 lines
119 lines
34 lines
49 lines

Diff 32905

View Options

web/cashtab/extension/src/components/App.js

Loading...
View Options

web/cashtab/package-lock.json

Loading...
View Options

web/cashtab/package.json

Loading...
View Options

web/cashtab/src/assets/airdrop-icon.svg

Loading...
View Options

web/cashtab/src/assets/styles/theme.js

Loading...
View Options

web/cashtab/src/components/Airdrop/Airdrop.js

Loading...
View Options

web/cashtab/src/components/Airdrop/__tests__/Airdrop.test.js

Loading...
View Options

web/cashtab/src/components/Airdrop/__tests__/__snapshots__/Airdrop.test.js.snap

Loading...
View Options

web/cashtab/src/components/App.js

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

web/cashtab/src/components/Configure/__tests__/__snapshots__/Configure.test.js.snap

Loading...
View Options

web/cashtab/src/components/Home/__tests__/__snapshots__/Home.test.js.snap

Loading...
View Options

web/cashtab/src/components/Receive/__tests__/__snapshots__/Receive.test.js.snap

Loading...
View Options

web/cashtab/src/components/Send/Send.js

Loading...
View Options

web/cashtab/src/components/Send/SendToken.js

Loading...
View Options

web/cashtab/src/components/Send/__tests__/__snapshots__/Send.test.js.snap

Loading...
View Options

web/cashtab/src/components/Tokens/__tests__/__snapshots__/CreateTokenForm.test.js.snap

Loading...
View Options

web/cashtab/src/components/Tokens/__tests__/__snapshots__/Tokens.test.js.snap

Loading...
View Options

web/cashtab/src/utils/__mocks__/mockXecAirdropRecipients.js

Loading...
View Options

web/cashtab/src/utils/__tests__/cashMethods.test.js

Loading...
View Options

web/cashtab/src/utils/__tests__/validation.test.js

Loading...
View Options

web/cashtab/src/utils/cashMethods.js

Loading...
View Options

web/cashtab/src/utils/validation.js

Loading...