Current multi send input validation does not pick up instances where the total multi send values is greater than the wallet balance. This diff patches isValidMultiSendUserInput with this additional check.
Details
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project
- npm test
- npm start
- test with a multi-send input with a single value that is greater than the wallet's balance
- test with a multi-send input with an aggregate value that is greater than the wallet's balance
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- multiSendAmtValidation
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 26984 Build 53534: Build Diff cashtab-tests Build 53533: arc lint + arc unit
Event Timeline
Comment Actions
does not pick up instances where one of the multi send values is greater than the wallet balance.
what we really want to check though is the total amount being sent does not exceed the wallet balance. This check does cover a certain edge case. but it doesn't solve the problem.
Comment Actions
Added check for aggregate send value exceeding wallet balance and corresponding unit tests
| cashtab/src/validation/__tests__/index.test.js | ||
|---|---|---|
| 751 ↗ | (On Diff #44930) | refactor these unit tests to use the vectors approach |
| cashtab/src/validation/index.js | ||
| 524 ↗ | (On Diff #44930) | add xecBalance as a param |
| 536 ↗ | (On Diff #44930) | we don't need a bignumber here. the function should take the balance as a number. you can do this conversion in sendXec when you pass it through. separately i'm working to replace the cumbersome balances object in wallet.state with balanceSats (number) |
Comment Actions
- Refactored isValidMultiSendUserInput unit tests to use the vectors approach
- Updated comments
- Replaced BN usage with Number and converting balances.totalBalance in SendXec.js to number when passing in to this function
| cashtab/src/validation/__tests__/index.test.js | ||
|---|---|---|
| 861 ↗ | (On Diff #44991) | input1 and input2 are unintelligible call these by their param names, userMultisendInput and xecBalance |
| cashtab/src/validation/index.js | ||
| 537 ↗ | (On Diff #44991) | |
| 563 ↗ | (On Diff #44991) | we don't need numValue as a separate var |
| 564 ↗ | (On Diff #44991) | we don't need this check now that we are checking against the total balance |
| 578 ↗ | (On Diff #44991) | |
| 582 ↗ | (On Diff #44991) | checking against xecBalance is a good improvement, but we actually want to check against the max amount of xec we can send. This is currently calculated by the 'onMax' routine in SendXec. Can pass this in as a param instead of the balance. |
Comment Actions
- Updated onMax() to return the value upon completion, which is then passed as a param to isValidMultiSendUserInput from SendXec.js
- Removed redundant vector and related unit test on a single amount being greater than wallet balance
| cashtab/src/components/Send/SendXec.js | ||
|---|---|---|
| 622 | the onMax() function is doing a lot of other things that we do not want to be doing in real time for a validation function. need to remove the part that only calculates the balance and make that it's own function. then call that in the current onMax() and also here. that should be a separate diff. | |
| cashtab/src/validation/index.js | ||
| 580 | this is cutoff either need a diff that fixes the problem of only one line rendering for these msgs or need to save some space  | |