- new isOnetoManyXECSend state added to Send.js to conditionally render address input field based on whether the user is in single recipient or multi-recipient mode.
- When in multi-recipient mode, new <TextArea> is rendered for users to input multiple recipients with formatting instructions shown in placeholder text.
- submit() function in Send.js updated to reflect whether the isOnetoManyXECSend modal is true or false. If true (multi recipient), it calls a separate one to many version of the original submit() logic which iterates through the <TextArea> inputs and converts it into an array and validates each line.
- new toLegacyArray() function added to Ticker.js which traverses the array of recipient addresses and converts them to bch cash addresses due to bch-api constraint
- sendBch() in useBCH.js now updated to sendXec() along with new parameters which can build both one to one and one to many multi recipient transactions.
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project - Commits
- rABC97eea03b145c: [Cashtab] Send multi-recipient XEC transaction
- npm start
- navigate to the Send screen and send a single XEC transction to ensure no regression
- click on 'Switch to multiple recipients' and enter in 3 valid recipient addresses and XEC send value separated by comma.
- click on 'Switch to multiple recipients' and enter in 3 valid recipient addresses and XEC send value separated by comma WITH an OP_RETURN message
- click on send and ensure the block explorer shows a transaction with the exact multi outputs and the total of those 3 values matches the cashtab Tx history entry
- click on 'Switch to multiple recipients' and enter in 3 valid recipient addresses and XEC send value, PLUS a 4th invalid address, and ensure this is picked up by validation
- click on 'Switch to multiple recipients' and enter in 3 valid recipient addresses and XEC send value, PLUS a 4th valid address but less than minimum transfer value (e.g. 3 XEC) and ensure a 5.5 XEC minimum error is displayed
- click on 'Switch to multiple recipients' and enter in 3 valid recipient addresses and XEC send value, PLUS a 4th valid eToken address and ensure an eToken error is displayed
- click on 'Switch to multiple recipients' and enter in no input or several spaces and ensure the 'Empty rows must be removed' field validation is displayed
- click on 'Switch to multiple recipients' and enter in multiple valid addresses and valid send values without clicking Send yet. Switch back to single recipient then switch back to multiple recipients. Ensure the inputs persisted between switches. Press Send and ensure successful transaction
- click on 'Switch to multiple recipients' and then switch back to single recipient and ensure a single recipient send XEC transaction is sent successfully
- npm run extension
- test the above in web extension mode
- verify cross browser compatibility in firefox
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- sendmanytx
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 17521 Build 34868: Build Diff cashtab-tests Build 34867: arc lint + arc unit
Event Timeline
web/cashtab/src/components/Common/EnhancedInputs.js | ||
---|---|---|
285 ↗ | (On Diff #31203) | Let's call this DestinationAddressMulti The original naming convention for the other form item is terrible, should also be changed. Created a task for that. |
web/cashtab/src/components/Common/Ticker.js | ||
224 ↗ | (On Diff #31203) | Split the array once and then assign the other variables. let addressValueArr = addressArray[i].split(','); let address = addressValueArr[0] let value = addressValueArr[1] |
web/cashtab/src/components/Send/Send.js | ||
191 ↗ | (On Diff #31203) | Sorry about this one....but.....now that we have sendXec as one function, we no longer need two separate functions for oneToManyXECSend() and oneToOneXECSend() Instead, a single combined send function should accept the isOneToManyXECSend state variable as a parameter -- then use this parameter for the isolated different workflows (calling sendXec with different parameters, logging separate event in Google Analytics) |
355 ↗ | (On Diff #31203) | Refactor so there is only one function here. |
- optimised array splitting in Ticker.js' toLegacyArray() function
- refactored the submit() and send() functions in Send.js
- FormItemWithoutQrCodeAddon in EnhancedInputs.js component changed to DestinationAddressMulti
web/cashtab/src/components/Send/Send.js | ||
---|---|---|
232 ↗ | (On Diff #31225) | This catch {} block looks identical for both oneToMany === true and oneToMany === false Please create a new function handleSendXecError(oneToManyFlag) that holds this code and is called in both catch {} blocks.. In the function, the event sent to errorNotification() should depend on oneToMany. If true, errorNotification(e, message, 'Sending XEC one to many'), if false, errorNotification(e, message, 'Sending XEC') |
355 ↗ | (On Diff #31225) | Delete this submit() {} function and just use the above send() function. Use isOneToManyXECSend as the boolean checked inside the send function. |
- Moved error handling for single and multi recipient XEC sends into a separate handleSendXecError() function.
- Removed submit() function and adjusted frontend to call send() directly.
web/cashtab/src/components/Send/Send.js | ||
---|---|---|
345 | Pass the parameter here as well. If intent is for param to show as not true because it is not passed and therefore undefined, it's okay to just use false -- but in this case might as well stay consistent with the flag. just a readability quibble |
May not need to fix in this diff, but there is an annoying UX issue. User story:
- Switch to multi-recipient mode and enter valid data
- Switch back to single. No validation error appears.
- Click "Send" --> validation error and send fails
- Switch back to multi-recipient mode --> input is saved and is now valid, but is shown as invalid with validation error, 'Send" button is disabled
Fix I think is to do a validation check every time the user toggles modes. Please correct or create a task.
web/cashtab/src/hooks/__tests__/useBCH.test.js | ||
---|---|---|
208 ↗ | (On Diff #31264) | Remove |
web/cashtab/src/hooks/useBCH.js | ||
987 ↗ | (On Diff #31264) | Use BigNumber here instead of Number to be consistent with rest of function --- we want to always use BigNumber when dealing with XEC or token amounts. Might also need it if user is sending exceptionally large amounts |
992 ↗ | (On Diff #31264) | Should already be a BigNumber here -- let me know if there was an important reason to do it the other way (keeping it as Number for all the addition, then converting at the end). |
Adjusted with BigNumber usage. A new task will be created to look at the edge case above.
web/cashtab/src/components/Common/Ticker.js | ||
---|---|---|
232 ↗ | (On Diff #31281) | This is just being converted back to a string on line 255, then back again to BigNumber in useBCH.js This should be a string until math operations are performed on it in useBCH.js, so no need to implement BigNumber in this function --- the conversion on L987 in useBCH.js is sufficient |