Page MenuHomePhabricator
Differential D17459

[Cashtab] [bugfix] Patch genesis supply validation for ALP tokens in create token form and ensure large number input fields are string
ClosedPublic
Actions

Authored by bytesofman on Thu, Jan 2, 20:27.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABCf3221e53549b: [Cashtab] [bugfix] Patch genesis supply validation for ALP tokens in create…
Summary

The necessary functions to do this were written as part of a broader refactor for better supporting multiple token types. But they were not implemented on the create token screen.

Note: The issue of large number input field was discovered in this diff. I considered splitting it out. However I think this diff is a good demonstration of the issue, and the test here confirms the implementation.

So I think it is worth landing this as a single diff. If we revert, the prod app already has bad alp validation, and we need the precision fix to confirm that with a (good) test.

Test Plan

npm test

Diff Detail

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

Event Timeline

bytesofman created this revision.Thu, Jan 2, 20:27
Herald added a reviewer: Restricted Project. · View Herald TranscriptThu, Jan 2, 20:27
Harbormaster completed remote builds in B31897: Diff 51934.Thu, Jan 2, 20:32
bytesofman updated this revision to Diff 51935.Thu, Jan 2, 20:32

improve doc

bytesofman published this revision for review.Thu, Jan 2, 20:32
Harbormaster completed remote builds in B31898: Diff 51935.Thu, Jan 2, 20:38
bytesofman retitled this revision from [Cashtab] Patch genesis supply validation for ALP tokens in create token form to [Cashtab] [bugfix] genesis supply validation for ALP tokens in create token form.Thu, Jan 2, 21:08
emack requested changes to this revision.Thu, Jan 2, 22:34
emack added a subscriber: emack.
emack added inline comments.
cashtab/src/components/Etokens/__tests__/CreateTokenForm.test.js
526 ↗(On Diff #51935)

SLP max here 18446744073709551615 does not match the SLP max inputted above 18446744073709552000. How is this expect() passing?

This revision now requires changes to proceed.Thu, Jan 2, 22:34
bytesofman marked an inline comment as done.Thu, Jan 2, 23:30
bytesofman added inline comments.
cashtab/src/components/Etokens/__tests__/CreateTokenForm.test.js
526 ↗(On Diff #51935)

good catch.

this is coming from JS failing to render the precise number.

html input fields must be a number or a string. This field is a number.

really tho..it should be a string, because it is expected to occasionally have numbers that are too big for JS.

Updated and updated this test (and others) as appropriate.

I thought about splitting out that change from this diff (i.e., make sure all of our token qty input fields are string instead of number, then correct the bug here). imo it makes sense to combine them, since the issues are related.

bytesofman updated this revision to Diff 51937.Thu, Jan 2, 23:31
bytesofman marked an inline comment as done.

make sure input fields that take amounts larger than JS max safe number are actually string fields

bytesofman retitled this revision from [Cashtab] [bugfix] genesis supply validation for ALP tokens in create token form to [Cashtab] [bugfix] Patch genesis supply validation for ALP tokens in create token form and ensure large number input fields are string.Thu, Jan 2, 23:34
bytesofman edited the summary of this revision. (Show Details)
Harbormaster completed remote builds in B31900: Diff 51937.Thu, Jan 2, 23:36
teamcity added a comment.Thu, Jan 2, 23:36
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

Run `npm audit` for details.

> ecash-lib@1.2.0 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

60 packages are looking for funding
  run `npm fund` for details

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.2.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3310 packages in 25s

320 packages are looking for funding
  run `npm fund` for details

8 vulnerabilities (6 moderate, 2 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.9.1 build
> node scripts/build.js

Creating an optimized production build...
Failed to compile.

TS2322: Type '{ placeholder: string; name: string; value: string; decimals: SlpDecimals; handleInput: (e: ChangeEvent<HTMLInputElement>) => void; error: string | false; handleOnMax: () => void; }' is not assignable to type 'IntrinsicAttributes & SendTokenInputProps'.
  Property 'decimals' does not exist on type 'IntrinsicAttributes & SendTokenInputProps'.
    1009 |                             name="genesisQty"
    1010 |                             value={formData.genesisQty}
  > 1011 |                             decimals={
         |                             ^^^^^^^^
    1012 |                                 parseInt(formData.decimals) as SlpDecimals
    1013 |                             }
    1014 |                             handleInput={handleInput}


Build cashtab-tests failed with exit code 1
bytesofman updated this revision to Diff 51939.Fri, Jan 3, 02:11

tslint patch

Harbormaster completed remote builds in B31901: Diff 51939.Fri, Jan 3, 02:17
teamcity added a comment.Fri, Jan 3, 02:17
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

  npm audit fix

Run `npm audit` for details.

> ecash-lib@1.2.0 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 2s

60 packages are looking for funding
  run `npm fund` for details

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.2.0 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1489 packages, and audited 3310 packages in 24s

320 packages are looking for funding
  run `npm fund` for details

8 vulnerabilities (6 moderate, 2 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

> cashtab@3.9.1 build
> node scripts/build.js

Creating an optimized production build...

Treating warnings as errors because process.env.CI = true.
Most CI servers set it automatically.

Failed to compile.

[eslint] 
src/components/Common/Inputs.tsx
  Line 10:10:  'SlpDecimals' is defined but never used  @typescript-eslint/no-unused-vars


Build cashtab-tests failed with exit code 1
emack requested changes to this revision.Fri, Jan 3, 03:00

Remove SlpDecimals import in Inputs.tsx to get the build working

This revision now requires changes to proceed.Fri, Jan 3, 03:00
bytesofman updated this revision to Diff 51940.Fri, Jan 3, 04:34

tslint again

Harbormaster completed remote builds in B31902: Diff 51940.Fri, Jan 3, 04:39
emack accepted this revision.Fri, Jan 3, 05:57
This revision is now accepted and ready to land.Fri, Jan 3, 05:57
Closed by commit rABCf3221e53549b: [Cashtab] [bugfix] Patch genesis supply validation for ALP tokens in create… (authored by bytesofman). · Explain WhyFri, Jan 3, 06:01
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABCf3221e53549b: [Cashtab] [bugfix] Patch genesis supply validation for ALP tokens in create….

Revision Contents
Changeset List

PathSize
cashtab/
4 lines
2 lines
src/
components/
Common/
5 lines
Etokens/
CreateTokenForm/
43 lines
Token/
9 lines
__tests__/
97 lines
4 lines

Diff 51941

View Options

cashtab/package-lock.json

Loading...
View Options

cashtab/package.json

Loading...
View Options

cashtab/src/components/Common/Inputs.tsx

Loading...
View Options

cashtab/src/components/Etokens/CreateTokenForm/index.tsx

Loading...
View Options

cashtab/src/components/Etokens/Token/index.tsx

Loading...
View Options

cashtab/src/components/Etokens/__tests__/CreateTokenForm.test.js

Loading...
View Options

cashtab/src/components/Etokens/__tests__/TokenActions.test.js

Loading...