[ecash-wallet] Add fluent API for building and broadcasting txs
DraftPublic
Actions

Authored by bytesofman on Wed, Mar 19, 04:27.

Details

Reviewers

None

Group Reviewers

Restricted Project

Summary

We want a powerful and user-friendly API that exposes commonly used steps in tx processing. For example, getting utxos to send a tx (/checking if we have the utxos we need to send a tx), getting a tx fee, and checking a tx size are all things that are commonly done without broadcasting a tx. We keep methods that do not need to be async as, well, not async. Only `broadcast() is (of necessity) async.

Note we will need to have a websocket that keeps the utxo set in sync, as a dev it is annoying to always have to remember to call await wallet.sync() before sending a tx. However, I think we need to get the tx API worked out first so we know what exactly the websocket should do and manage.

Test Plan

npm test, build node in build and then BUILD_DIR="${PWD}/../../build" npm run integration-tests

Diff Detail

Repository

rABC Bitcoin ABC

Branch

wallet-txs-regtest

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 32768
Build 65024: Build Diff	ecash-wallet-tests
Build 65023: arc lint + arc unit

Event Timeline

bytesofman created this revision.Wed, Mar 19, 04:27

Herald added a reviewer: Restricted Project. · View Herald TranscriptWed, Mar 19, 04:27

Harbormaster completed remote builds in B32760: Diff 53165.Wed, Mar 19, 04:30

Improve comments, add tests for fee() and size() methods

Harbormaster completed remote builds in B32768: Diff 53181.Wed, Mar 19, 14:57

tobias_ruck added a subscriber: tobias_ruck.Wed, Mar 19, 18:54

tobias_ruck added inline comments.

modules/ecash-wallet/helpers.ts
42	Careful, this will be incorrect as the signatory is missing, which will add bytes (eg the signature)
modules/ecash-wallet/package.json
7	There’s something wrong with this. The unit tests should be in one folder (usually src) and the integration tests in another (usually tests), like it’s done for ecash-lib. Then we can use the wildcard. Otherwise we’ll have to add new tests here every time we add a new file.
modules/ecash-wallet/wallet.ts
42	IMO you should change this to Recipient (or Output), and add sats and tokenId and isMintBaton, just like Output in my BIP70 diff. And then we use that as unified interface for where to send tokens and the wallet just handles it. Remember ALP allows multiple token IDs per send, I think we should enable this by default, that’s why it should be part of the output. Also, keep in mind that we also need to enable attaching data in the OP_RETURN. For ALP/eMPP, this will be extra eMPP pushdata, and for non-eMPP, this could be OP_RETURN. Either way it would be an UInt8Array[] (which must be empty for SLP btw)
91	Event is really not a good name IMO. TransactionsConfig (because you want this to be multiple ones). Recipients might work to. Or just SendConfig. And I think there should be one unified config for sats only, tokens, and OP_RETURN data
93	I think you just want one interface which handles all three—remember they can overlap in ALP, eg SENDs and MINTs and BURNs can be in 1 tx . So you want the outputs as I describe above (with the mint baton Booleans), and the burn amount, and the OP_RETURN data. outputs/recipients (sats and/or tokens, or mint baton) burns (list of tokenId + burn amount) opreturn data And then the function figures out how to build the tx(s) based on that.
121	Probably want to use Partial here instead of duplicating types