Page MenuHomePhabricator
Differential D15553

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

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

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABCf958feda21dc: [Cashtab] Calculate max send amount in appropriate way using updated ecash…
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

bytesofman created this revision.Feb 26 2024, 18:14
Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 26 2024, 18:14
Harbormaster completed remote builds in B27433: Diff 45630.Feb 26 2024, 18:21
bytesofman updated this revision to Diff 45634.Feb 26 2024, 18:25

use rc from ecash-coinselect diff

Harbormaster completed remote builds in B27437: Diff 45634.Feb 26 2024, 18:30
bytesofman updated this revision to Diff 45650.Feb 27 2024, 00:24

rebase

bytesofman updated this revision to Diff 45651.Feb 27 2024, 00:27

back out plock changes

bytesofman updated this revision to Diff 45652.Feb 27 2024, 00:29

back out other unrelated changes

bytesofman updated this revision to Diff 45654.Feb 27 2024, 00:46

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

Harbormaster completed remote builds in B27455: Diff 45654.Feb 27 2024, 00:46
bytesofman updated this revision to Diff 45655.Feb 27 2024, 00:46

lint

bytesofman updated this revision to Diff 45657.Feb 27 2024, 00:56

rebase

bytesofman edited the summary of this revision. (Show Details)Feb 27 2024, 00:57
bytesofman added a parent revision: D15556: [Cashtab] Standardize and improve SendXec.test.js.
Harbormaster completed remote builds in B27451: Diff 45650.Feb 27 2024, 01:01
Harbormaster completed remote builds in B27452: Diff 45651.
teamcity added a comment.Feb 27 2024, 01:01
Build Bitcoin ABC Diffs / Diff Testing (ecash-coinselect) failed.

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
bytesofman added a comment.Feb 27 2024, 01:04
In D15553#350890, @teamcity wrote:
Build Bitcoin ABC Diffs / Diff Testing (ecash-coinselect) failed.

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

Harbormaster completed remote builds in B27453: Diff 45652.Feb 27 2024, 01:06
Harbormaster completed remote builds in B27458: Diff 45657.
Harbormaster completed remote builds in B27456: Diff 45655.Feb 27 2024, 01:08
bytesofman published this revision for review.Feb 27 2024, 01:08
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
bytesofman updated this revision to Diff 45662.Feb 27 2024, 12:08

fix test title, confirm msg of empty spaces is passed

Harbormaster completed remote builds in B27460: Diff 45662.Feb 27 2024, 12:08
bytesofman marked 2 inline comments as done.Feb 27 2024, 12:09
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 updated this revision to Diff 45663.Feb 27 2024, 12:09
bytesofman marked 2 inline comments as done.

remove focused test

bytesofman updated this revision to Diff 45665.Feb 27 2024, 12:12

rebase, bump version

bytesofman edited the summary of this revision. (Show Details)Feb 27 2024, 12:14
bytesofman removed a parent revision: D15556: [Cashtab] Standardize and improve SendXec.test.js.
Harbormaster completed remote builds in B27461: Diff 45663.Feb 27 2024, 12:21
Harbormaster completed remote builds in B27462: Diff 45665.Feb 27 2024, 12:25
emack accepted this revision.Feb 27 2024, 20:18
This revision is now accepted and ready to land.Feb 27 2024, 20:18
Closed by commit rABCf958feda21dc: [Cashtab] Calculate max send amount in appropriate way using updated ecash… (authored by bytesofman). · Explain WhyFeb 27 2024, 20:47
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABCf958feda21dc: [Cashtab] Calculate max send amount in appropriate way using updated ecash….

Revision Contents
Changeset List

PathSize
cashtab/
8 lines
2 lines
src/
components/
Send/
58 lines
__tests__/
986 lines
utils/
__tests__/
31 lines
40 lines

Diff 45655

View Options

cashtab/package-lock.json

Loading...
View Options

cashtab/package.json

Loading...
View Options

cashtab/src/components/Send/SendXec.js

Loading...
View Options

cashtab/src/components/Send/__tests__/SendXec.test.js

Loading...
View Options

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

Loading...
View Options

cashtab/src/utils/cashMethods.js

Loading...