Page MenuHomePhabricator
Differential D17773

[ecash-wallet] Init ecash-wallet in monorepo
AcceptedPublic
Actions

Authored by bytesofman on Mon, Mar 10, 18:31.

Details

Reviewers
tobias_ruck
Group Reviewers
Restricted Project
Summary

Now that we have type stability with chronik-client and sophisticated tx construction with ecash-lib, we can (and should) create an abstracted typed wallet class to make tx construction easier and more consistent for builders

Some requirements

  • Support XEC sends
  • Support token methods
  • Support agora
  • Support "chained" txs, i.e. token sends to more than max outputs, or XEC sends to outputs that would bump tx size above the broadcast limit
  • regtest integration
  • Should be able to drop this into Cashtab and pull out all of Cashtab's custom tx methods
  • HD wallet support
  • Construct from WIF

This diff just inits the repo and CI. Next diff will add send XEC method and get that tested with regtest integration. Then will add other methods.

Test Plan

npm test

I am building this lib incrementally following a personal-use class I have been testing / dogfooding for the last couple of months.

So, any comments on structure, layout, questions about why some things are being initialized this way --- also important.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
init-wallet-lib
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 32645
Build 64778: Build Diff
Build 64777: arc lint + arc unit

Event Timeline

bytesofman created this revision.Mon, Mar 10, 18:31
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Mar 10, 18:31
Harbormaster completed remote builds in B32640: Diff 52995.Mon, Mar 10, 18:31
bytesofman updated this revision to Diff 52996.Mon, Mar 10, 18:32

lint

Harbormaster completed remote builds in B32641: Diff 52996.Mon, Mar 10, 18:32
bytesofman published this revision for review.Mon, Mar 10, 18:33
tobias_ruck requested changes to this revision.Mon, Mar 10, 18:43
tobias_ruck added a subscriber: tobias_ruck.
tobias_ruck added inline comments.
modules/ecash-wallet/index.ts
8 ↗(On Diff #52996)

is there a reason for this interface? the class below as a type should be fine—for example we don't need ChronikClientInterface

38 ↗(On Diff #52996)

why not just Wallet?

  • More based
  • Shorter
  • Can be used on other chains (doge, ...)

if people already have "Wallet" in their name space, they can rename it at import

43 ↗(On Diff #52996)

I usually call this pkh (as in p2pkh) and it's less ambiguous

47 ↗(On Diff #52996)

spacex

48 ↗(On Diff #52996)

I think again this constructor should be private, and then we add a bunch of helper methods (fromSecKey/fromSk, fromMnemonic, fromEntropy, etc.). But either works.

I think we should plan for this to eventually properly support HD nodes (i.e. used addresses are advanced, like in Electrum ABC).

There's this: https://github.com/Amatack/ecash-wallet-cli

although I never used it

This revision now requires changes to proceed.Mon, Mar 10, 18:43
bytesofman marked 5 inline comments as done.Mon, Mar 10, 21:12
bytesofman added inline comments.
modules/ecash-wallet/index.ts
8 ↗(On Diff #52996)

nice, yeah can just lose it

I ended up having these in some ts files due to looking up how to fix the lint errors, and this (heavily over-specified) way is popular

38 ↗(On Diff #52996)

yup, this is better, updated

48 ↗(On Diff #52996)

yup might as well just get that going now. makes it more extendable.

bytesofman updated this revision to Diff 52998.Mon, Mar 10, 21:13
bytesofman marked 3 inline comments as done.

remove redundant interface, change name to Wallet, private constructor with fromSk static method helper constructor

Harbormaster completed remote builds in B32643: Diff 52998.Mon, Mar 10, 21:13
tobias_ruck requested changes to this revision.Mon, Mar 10, 21:45
tobias_ruck added inline comments.
modules/ecash-wallet/README.md
11–16 ↗(On Diff #52998)
modules/ecash-wallet/index.ts
19–23 ↗(On Diff #52998)
57 ↗(On Diff #52998)

do I always have to do your homework smh where's the xecellency

This revision now requires changes to proceed.Mon, Mar 10, 21:45
bytesofman updated this revision to Diff 53000.Mon, Mar 10, 21:51
bytesofman marked 2 inline comments as done.

more fixes from the rename and constructor change, better property comments

Harbormaster completed remote builds in B32644: Diff 53000.Mon, Mar 10, 21:51
bytesofman updated this revision to Diff 53001.Mon, Mar 10, 21:51

remove another XecWallet mention

Harbormaster completed remote builds in B32645: Diff 53001.Mon, Mar 10, 21:51
tobias_ruck accepted this revision.Mon, Mar 10, 22:28
tobias_ruck added inline comments.
modules/ecash-wallet/index.ts
65

btw any reason to use this syntax instead of this? My VSCode shows this as a field, not a method, but it's clearly a method

also this is shorter. not sure where that convention came from

80

same here

This revision is now accepted and ready to land.Mon, Mar 10, 22:28

Revision Contents
Changeset List

PathSize
contrib/
teamcity/
23 lines
modules/
ecash-wallet/
2 lines
23 lines
122 lines
85 lines
3819 lines
49 lines
4 lines
12 lines

Diff 53001

View Options

contrib/teamcity/build-configurations.yml

Loading...
View Options

modules/ecash-wallet/.gitignore

Loading...
View Options

modules/ecash-wallet/README.md

Loading...
View Options

modules/ecash-wallet/index.test.ts

Loading...
View Options

modules/ecash-wallet/index.ts

Loading...
View Options

modules/ecash-wallet/package-lock.json

Loading...
View Options

modules/ecash-wallet/package.json

Loading...
View Options

modules/ecash-wallet/tsconfig.build.json

Loading...
View Options

modules/ecash-wallet/tsconfig.json

Loading...