Page MenuHomePhabricator

[Cashtab] Wallet methods to ts
ClosedPublic

Authored by bytesofman on Tue, Nov 5, 18:31.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABC51a757254596: [Cashtab] Wallet methods to ts
Summary

Needed for ts component to manage NFT collections. Convert in limited diff here as part of broader effort to bring Cashtab to ts.

Now that we have stable types from ChronikClient, opportunity for more sophisticated and tested Wallet as a class, maybe a library for app devs. But this would be a separate effort.

Test Plan

npm test

Diff Detail

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

Event Timeline

Tail of the build log:

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

/work/cashtab /work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests
npm warn deprecated inflight@1.0.6: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm warn deprecated @babel/plugin-proposal-private-methods@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-private-methods instead.
npm warn deprecated @babel/plugin-proposal-numeric-separator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-numeric-separator instead.
npm warn deprecated @babel/plugin-proposal-nullish-coalescing-operator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-nullish-coalescing-operator instead.
npm warn deprecated @babel/plugin-proposal-class-properties@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-class-properties instead.
npm warn deprecated rimraf@3.0.2: Rimraf versions prior to v4 are no longer supported
npm warn deprecated @babel/plugin-proposal-optional-chaining@7.21.0: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-optional-chaining instead.
npm warn deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated stable@0.1.8: Modern JS already guarantees Array#sort() is a stable sort, so this library is deprecated. See the compatibility table on MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#browser_compatibility
npm warn deprecated sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec instead
npm warn deprecated workbox-cacheable-response@6.6.0: workbox-background-sync@6.6.0
npm warn deprecated rollup-plugin-terser@7.0.2: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-terser
npm warn deprecated workbox-google-analytics@6.6.0: It is not compatible with newer versions of GA starting with v4, as long as you are using GAv3 it should be ok, but the package is not longer being maintained
npm warn deprecated domexception@4.0.0: Use your platform's native DOMException instead
npm warn deprecated abab@2.0.6: Use your platform's native atob() and btoa() methods instead
npm warn deprecated @humanwhocodes/config-array@0.13.0: Use @eslint/config-array instead
npm warn deprecated @babel/plugin-proposal-private-property-in-object@7.21.11: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-private-property-in-object instead.
npm warn deprecated eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1645 packages, and audited 1651 packages in 18s

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

1 low severity vulnerability

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

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

TS7016: Could not find a declaration file for module 'randombytes'. '/work/cashtab/node_modules/randombytes/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/randombytes` if it exists or add a new declaration (.d.ts) file containing `declare module 'randombytes';`
     5 | import { BN } from 'slp-mdm';
     6 | import * as bip39 from 'bip39';
  >  7 | import randomBytes from 'randombytes';
       |                         ^^^^^^^^^^^^^
     8 | import * as utxolib from '@bitgo/utxo-lib';
     9 | import cashaddr from 'ecashaddrjs';
    10 | import appConfig from 'config/app';


Build cashtab-tests failed with exit code 1
bytesofman published this revision for review.Tue, Nov 5, 18:50
bytesofman added inline comments.
cashtab/src/wallet/fixtures/vectors.js
15 ↗(On Diff #50675)

these used to be strings in ChronikClientNode

Now that we have stable types from ChronikClient, use the right types.

35 ↗(On Diff #50675)

now that we have stable types and know what shape to expect, there's no point in testing for "what happens if we get the wrong type"

emack requested changes to this revision.Wed, Nov 6, 07:37
emack added a subscriber: emack.
emack added inline comments.
cashtab/src/wallet/index.ts
331 ↗(On Diff #50676)

how come the @returns is specified here but higher up it's removed? e.g. line 122

432 ↗(On Diff #50676)

as above

437 ↗(On Diff #50676)

what's the reason for splitting this out?

This revision now requires changes to proceed.Wed, Nov 6, 07:37
bytesofman marked an inline comment as done.

update returns and param types, spacing for consistency

bytesofman added inline comments.
cashtab/src/wallet/index.ts
331 ↗(On Diff #50676)

I went through and cleaned these up a bit for consistency.

Now that we have typescript, the whole @param and @returns stuff is mostly superfluous. The exact types are specified in the fn definition.

So, I applied this heuristic:

  • Remove specified types from params desc
  • Keep an explanation if it seems helpful
  • returns may or may not be specified...I kept it if there was a helpful comment. Otherwise this is handled by the ts spec.

It's not perfect, but imo the typescript implementation is the key part.

432 ↗(On Diff #50676)

see remarks above

437 ↗(On Diff #50676)

typescript linting

exponent is a const here, while mantissa is not

makes it look pretty weird ... but the linter is right.

This revision is now accepted and ready to land.Thu, Nov 7, 10:01
This revision was automatically updated to reflect the committed changes.
bytesofman marked 2 inline comments as done.