Page MenuHomePhabricator

[Cashtab] Calculate max send amount in appropriate way using updated ecash-coinselect
ClosedPublic

Authored by bytesofman on Feb 26 2024, 18:14.

Details

Summary

We are using a legacy function to calculate the fee here that is underestimating the actual max send amount.

Using an updated helper function from ecash-coinselect, calculate the max send amount appropriately (including consideration for a cashtab msg, if present).

Test Plan

npm test

Diff Detail

Repository
rABC Bitcoin ABC
Branch
no-calcfee
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 27456
Build 54475: Build Diffcashtab-tests
Build 54474: arc lint + arc unit

Event Timeline

use rc from ecash-coinselect diff

back out other unrelated changes

add updated sendxec tests (will rebase), add integration test

Tail of the build log:

/work/modules/ecash-coinselect /work/abc-ci-builds/ecash-coinselect
npm ERR! code EUSAGE
npm ERR! 
npm ERR! `npm ci` can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.
npm ERR! 
npm ERR! Missing: ecash-coinselect@2.2.0 from lock file
npm ERR! 
npm ERR! Clean install a project
npm ERR! 
npm ERR! Usage:
npm ERR! npm ci
npm ERR! 
npm ERR! Options:
npm ERR! [--install-strategy <hoisted|nested|shallow|linked>] [--legacy-bundling]
npm ERR! [--global-style] [--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]]
npm ERR! [--include <prod|dev|optional|peer> [--include <prod|dev|optional|peer> ...]]
npm ERR! [--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] [--no-audit]
npm ERR! [--no-bin-links] [--no-fund] [--dry-run]
npm ERR! [-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
npm ERR! [-ws|--workspaces] [--include-workspace-root] [--install-links]
npm ERR! 
npm ERR! aliases: clean-install, ic, install-clean, isntall-clean
npm ERR! 
npm ERR! Run "npm help ci" for more info

npm ERR! A complete log of this run can be found in: /root/.npm/_logs/2024-02-27T01_01_42_041Z-debug-0.log
Build ecash-coinselect failed with exit code 1

Tail of the build log:

/work/modules/ecash-coinselect /work/abc-ci-builds/ecash-coinselect
npm ERR! code EUSAGE
npm ERR! 
npm ERR! `npm ci` can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.
npm ERR! 
npm ERR! Missing: ecash-coinselect@2.2.0 from lock file
npm ERR! 
npm ERR! Clean install a project
npm ERR! 
npm ERR! Usage:
npm ERR! npm ci
npm ERR! 
npm ERR! Options:
npm ERR! [--install-strategy <hoisted|nested|shallow|linked>] [--legacy-bundling]
npm ERR! [--global-style] [--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]]
npm ERR! [--include <prod|dev|optional|peer> [--include <prod|dev|optional|peer> ...]]
npm ERR! [--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] [--no-audit]
npm ERR! [--no-bin-links] [--no-fund] [--dry-run]
npm ERR! [-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
npm ERR! [-ws|--workspaces] [--include-workspace-root] [--install-links]
npm ERR! 
npm ERR! aliases: clean-install, ic, install-clean, isntall-clean
npm ERR! 
npm ERR! Run "npm help ci" for more info

npm ERR! A complete log of this run can be found in: /root/.npm/_logs/2024-02-27T01_01_42_041Z-debug-0.log
Build ecash-coinselect failed with exit code 1

failure related to pulling ecash-coinselect changes out of this diff, handled in already landed separate diff

emack requested changes to this revision.Feb 27 2024, 07:30
emack added a subscriber: emack.
emack added inline comments.
cashtab/src/components/Send/SendXec.js
681 ↗(On Diff #45657)

need to use opReturnMsg.trim() !== '' here otherwise a message full of spaces will bypass this

cashtab/src/components/Send/__tests__/SendXec.test.js
1088 ↗(On Diff #45657)

you are adding a cashtab msg though?

This revision now requires changes to proceed.Feb 27 2024, 07:30

fix test title, confirm msg of empty spaces is passed

bytesofman added inline comments.
cashtab/src/components/Send/SendXec.js
681 ↗(On Diff #45657)

not seeing this behavior in the current app, which uses this same check to decide whether or not to push a target output for a cashtab msg

else if (opReturnMsg !== false && opReturnMsg !== '') {
            // If not an airdrop msg, add opReturnMsg as a Cashtab Msg, if it exists
            targetOutputs.push(getCashtabMsgTargetOutput(opReturnMsg));
        }

perhaps it could be improved in both spots, but that should be its own diff.

here's a msg i made by just adding spaces to the message output: https://explorer.e.cash/tx/bb1d0bd6e875278fed70e3c23f4da769282866b2c27a6fed7295ff6d9213448e

updated the added test here to confirm this

cashtab/src/components/Send/__tests__/SendXec.test.js
1088 ↗(On Diff #45657)

lol right but I wasn't when I first wrote this title

good catch, updated

bytesofman marked 2 inline comments as done.

remove focused test

This revision is now accepted and ready to land.Feb 27 2024, 20:18