Page MenuHomePhabricator

[ecash-lib] Add `TxBuilder` to build Bitcoin transactions
ClosedPublic

Authored by tobias_ruck on Apr 12 2024, 14:59.

Details

Summary

Allows building Bitcoin transactions, able to handle any crazy covenant; able to calculate leftover outputs precisely.

Test Plan

npm run test && npm run build && npm run integration-tests

Diff Detail

Repository
rABC Bitcoin ABC
Branch
ecash-lib-tx-builder
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 28550
Build 56641: Build Diffbuild-clang-tidy · build-clang · build-diff · build-without-wallet · ecash-lib-tests · build-debug
Build 56640: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

add integration tests + enable them

Tail of the build log:

   Compiling unicode-ident v1.0.12
   Compiling version_check v0.9.4
   Compiling typenum v1.17.0
   Compiling wasm-bindgen-shared v0.2.92
   Compiling log v0.4.21
   Compiling bumpalo v3.16.0
   Compiling cc v1.0.92
   Compiling once_cell v1.19.0
   Compiling thiserror v1.0.58
   Compiling wasm-bindgen v0.2.92
   Compiling cfg-if v1.0.0
   Compiling abc-rust-lint v0.1.0 (/work/chronik/abc-rust-lint)
   Compiling generic-array v0.14.7
   Compiling quote v1.0.36
   Compiling secp256k1-sys-abc v0.4.1 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling syn v2.0.58
   Compiling crypto-common v0.1.6
   Compiling block-buffer v0.10.4
   Compiling digest v0.10.7
   Compiling ripemd v0.1.3
   Compiling sha2 v0.10.8
   Compiling wasm-bindgen-backend v0.2.92
   Compiling secp256k1-abc v0.20.3 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling thiserror-impl v1.0.58
   Compiling wasm-bindgen-macro-support v0.2.92
   Compiling wasm-bindgen-macro v0.2.92
   Compiling ecash-lib-wasm v0.1.0 (/work/modules/ecash-lib-wasm)
    Finished release-wasm [optimized] target(s) in 5.41s
/work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/abc-ci-builds/ecash-lib-tests

added 362 packages, and audited 364 packages in 3s

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

found 0 vulnerabilities
CI configured to test build. Building...

> ecash-lib@0.1.0 build
> tsc && cp -r ./src/ffi ./dist

../chronik-client/proto/chronik.ts(2,18): error TS2307: Cannot find module 'long' or its corresponding type declarations.
../chronik-client/proto/chronik.ts(3,22): error TS2307: Cannot find module 'protobufjs/minimal' or its corresponding type declarations.
../chronik-client/proto/chronikNode.ts(2,18): error TS2307: Cannot find module 'long' or its corresponding type declarations.
../chronik-client/proto/chronikNode.ts(3,17): error TS2307: Cannot find module 'protobufjs/minimal' or its corresponding type declarations.
../chronik-client/src/ChronikClient.ts(5,23): error TS2307: Cannot find module 'isomorphic-ws' or its corresponding type declarations.
../chronik-client/src/ChronikClient.ts(6,21): error TS2307: Cannot find module 'ws' or its corresponding type declarations.
../chronik-client/src/ChronikClientNode.ts(5,22): error TS2307: Cannot find module 'ecashaddrjs' or its corresponding type declarations.
../chronik-client/src/ChronikClientNode.ts(6,23): error TS2307: Cannot find module 'isomorphic-ws' or its corresponding type declarations.
../chronik-client/src/ChronikClientNode.ts(7,21): error TS2307: Cannot find module 'ws' or its corresponding type declarations.
../chronik-client/src/failoverProxy.ts(5,38): error TS2307: Cannot find module 'axios' or its corresponding type declarations.
../chronik-client/src/failoverProxy.ts(6,23): error TS2307: Cannot find module 'isomorphic-ws' or its corresponding type declarations.
../chronik-client/src/failoverProxy.ts(7,21): error TS2307: Cannot find module 'ws' or its corresponding type declarations.
../chronik-client/src/failoverProxy.ts(179,35): error TS7006: Parameter 'x' implicitly has an 'any' type.
../chronik-client/src/failoverProxy.ts(249,32): error TS7006: Parameter 'e' implicitly has an 'any' type.
../chronik-client/src/failoverProxy.ts(255,30): error TS7006: Parameter 'e' implicitly has an 'any' type.
../chronik-client/src/failoverProxy.ts(275,43): error TS7006: Parameter 'msg' implicitly has an 'any' type.
../chronik-client/src/failoverProxy.ts(291,43): error TS7006: Parameter 'msg' implicitly has an 'any' type.
tests/txBuilder.test.ts(36,14): error TS2749: 'EccWasm' refers to a value, but is being used as a type here. Did you mean 'typeof EccWasm'?
Build ecash-lib-tests failed with exit code 2

Tail of the build log:

   Compiling proc-macro2 v1.0.79
   Compiling unicode-ident v1.0.12
   Compiling version_check v0.9.4
   Compiling typenum v1.17.0
   Compiling wasm-bindgen-shared v0.2.92
   Compiling cc v1.0.92
   Compiling log v0.4.21
   Compiling once_cell v1.19.0
   Compiling bumpalo v3.16.0
   Compiling thiserror v1.0.58
   Compiling wasm-bindgen v0.2.92
   Compiling cfg-if v1.0.0
   Compiling abc-rust-lint v0.1.0 (/work/chronik/abc-rust-lint)
   Compiling generic-array v0.14.7
   Compiling quote v1.0.36
   Compiling syn v2.0.58
   Compiling secp256k1-sys-abc v0.4.1 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling crypto-common v0.1.6
   Compiling block-buffer v0.10.4
   Compiling digest v0.10.7
   Compiling ripemd v0.1.3
   Compiling sha2 v0.10.8
   Compiling wasm-bindgen-backend v0.2.92
   Compiling secp256k1-abc v0.20.3 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling thiserror-impl v1.0.58
   Compiling wasm-bindgen-macro-support v0.2.92
   Compiling wasm-bindgen-macro v0.2.92
   Compiling ecash-lib-wasm v0.1.0 (/work/modules/ecash-lib-wasm)
    Finished release-wasm [optimized] target(s) in 5.39s
/work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/abc-ci-builds/ecash-lib-tests

added 362 packages, and audited 364 packages in 3s

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

found 0 vulnerabilities
CI configured to test build. Building...

> ecash-lib@0.1.0 build
> tsc && cp -r ./src/ffi ./dist

../chronik-client/proto/chronik.ts(2,18): error TS2307: Cannot find module 'long' or its corresponding type declarations.
../chronik-client/proto/chronik.ts(3,22): error TS2307: Cannot find module 'protobufjs/minimal' or its corresponding type declarations.
../chronik-client/proto/chronikNode.ts(2,18): error TS2307: Cannot find module 'long' or its corresponding type declarations.
../chronik-client/proto/chronikNode.ts(3,17): error TS2307: Cannot find module 'protobufjs/minimal' or its corresponding type declarations.
../chronik-client/src/ChronikClient.ts(5,23): error TS2307: Cannot find module 'isomorphic-ws' or its corresponding type declarations.
../chronik-client/src/ChronikClient.ts(6,21): error TS2307: Cannot find module 'ws' or its corresponding type declarations.
../chronik-client/src/ChronikClientNode.ts(5,22): error TS2307: Cannot find module 'ecashaddrjs' or its corresponding type declarations.
../chronik-client/src/ChronikClientNode.ts(6,23): error TS2307: Cannot find module 'isomorphic-ws' or its corresponding type declarations.
../chronik-client/src/ChronikClientNode.ts(7,21): error TS2307: Cannot find module 'ws' or its corresponding type declarations.
../chronik-client/src/failoverProxy.ts(5,38): error TS2307: Cannot find module 'axios' or its corresponding type declarations.
../chronik-client/src/failoverProxy.ts(6,23): error TS2307: Cannot find module 'isomorphic-ws' or its corresponding type declarations.
../chronik-client/src/failoverProxy.ts(7,21): error TS2307: Cannot find module 'ws' or its corresponding type declarations.
../chronik-client/src/failoverProxy.ts(179,35): error TS7006: Parameter 'x' implicitly has an 'any' type.
../chronik-client/src/failoverProxy.ts(249,32): error TS7006: Parameter 'e' implicitly has an 'any' type.
../chronik-client/src/failoverProxy.ts(255,30): error TS7006: Parameter 'e' implicitly has an 'any' type.
../chronik-client/src/failoverProxy.ts(275,43): error TS7006: Parameter 'msg' implicitly has an 'any' type.
../chronik-client/src/failoverProxy.ts(291,43): error TS7006: Parameter 'msg' implicitly has an 'any' type.
Build ecash-lib-tests failed with exit code 2

@bot ecash-lib-integration-tests

Tail of the build log:

    at Object.<anonymous> (/usr/lib/node_modules/nyc/node_modules/append-transform/index.js:64:4)
    at Module.load (node:internal/modules/cjs/loader:1206:32)
    at Module._load (node:internal/modules/cjs/loader:1022:12)
    at Module.require (node:internal/modules/cjs/loader:1231:19)
    at require (node:internal/modules/helpers:179:18)
    at exports.requireOrImport (/work/modules/ecash-lib/node_modules/mocha/lib/nodejs/esm-utils.js:53:16)
    at async exports.loadFilesAsync (/work/modules/ecash-lib/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async singleRun (/work/modules/ecash-lib/node_modules/mocha/lib/cli/run-helpers.js:125:3)
    at async exports.handler (/work/modules/ecash-lib/node_modules/mocha/lib/cli/run.js:370:5) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/work/modules/chronik-client/src/ChronikClient.ts',
    '/work/modules/chronik-client/index.ts',
    '/work/modules/ecash-lib/tests/txBuilder.test.ts',
    '/work/modules/ecash-lib/node_modules/mocha/lib/nodejs/esm-utils.js',
    '/work/modules/ecash-lib/node_modules/mocha/lib/mocha.js',
    '/work/modules/ecash-lib/node_modules/mocha/lib/cli/one-and-dones.js',
    '/work/modules/ecash-lib/node_modules/mocha/lib/cli/options.js',
    '/work/modules/ecash-lib/node_modules/mocha/lib/cli/cli.js'
  ]
}
-------------------|---------|----------|---------|---------|-------------------
File               | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-------------------|---------|----------|---------|---------|-------------------
All files          |       0 |        0 |       0 |       0 |                   
 ecash-lib         |       0 |        0 |       0 |       0 |                   
  eslint.config.js |       0 |        0 |       0 |       0 |                   
 ecash-lib/src     |       0 |        0 |       0 |       0 |                   
  ecc.ts           |       0 |      100 |       0 |       0 | 23-38             
  hash.ts          |       0 |      100 |     100 |       0 | 7-9               
  index.ts         |       0 |        0 |       0 |       0 |                   
  op.ts            |       0 |        0 |       0 |       0 | 33-124            
  opcode.ts        |       0 |      100 |     100 |       0 | 10-154            
  script.ts        |       0 |        0 |       0 |       0 | 26-158            
  sighashtype.ts   |       0 |        0 |       0 |       0 | 19-166            
  tx.ts            |       0 |        0 |       0 |       0 | 73-153            
  txBuilder.ts     |       0 |        0 |       0 |       0 | 70-224            
  unsignedTx.ts    |       0 |        0 |       0 |       0 | 31-222            
 ecash-lib/src/io  |       0 |        0 |       0 |       0 |                   
  bytes.ts         |       0 |        0 |       0 |       0 | 13-64             
  hex.ts           |       0 |        0 |       0 |       0 | 5-68              
  int.ts           |       0 |        0 |       0 |       0 |                   
  varsize.ts       |       0 |        0 |       0 |       0 | 14-47             
  writer.ts        |       0 |        0 |       0 |       0 |                   
  writerbytes.ts   |       0 |        0 |       0 |       0 | 21-79             
  writerlength.ts  |       0 |      100 |       0 |       0 | 16-41             
-------------------|---------|----------|---------|---------|-------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='0']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='528']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='0']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='198']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='0']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='78']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='0']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='519']
##teamcity[blockClosed name='Code Coverage Summary']
mv: cannot stat 'test_results/ecash-lib-integration-tests-junit.xml': No such file or directory
Build ecash-lib-integration-tests failed with exit code 1

run dependent builds in ecash-lib-integration-tests

@bot ecash-lib-integration-tests

Tail of the build log:

1 moderate severity vulnerability

To address all issues, run:
  npm audit fix

Run `npm audit` for details.
/work/modules/ecash-lib /work/modules/chronik-client /work/modules/ecash-lib-wasm /work/abc-ci-builds/ecash-lib-integration-tests

added 362 packages, and audited 364 packages in 2s

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

found 0 vulnerabilities

> ecash-lib@0.1.0 integration-tests
> mocha --import=tsx ./tests/*.test.ts --reporter mocha-junit-reporter --reporter-options mochaFile=test_results/ecash-lib-integration-tests-junit.xml --reporter-options testsuitesTitle=Ecash Lib Integration Tests --reporter-options rootSuiteTitle=Ecash Lib

Test runner error, aborting: Error: spawn python ENOENT
--------------------|---------|----------|---------|---------|-------------------
File                | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
--------------------|---------|----------|---------|---------|-------------------
All files           |       0 |        0 |       0 |       0 |                   
 ecash-lib          |       0 |        0 |       0 |       0 |                   
  eslint.config.js  |       0 |        0 |       0 |       0 |                   
 ecash-lib/src      |       0 |        0 |       0 |       0 |                   
  ecc.ts            |       0 |      100 |       0 |       0 | 23-38             
  hash.ts           |       0 |      100 |     100 |       0 | 7-9               
  index.ts          |       0 |        0 |       0 |       0 |                   
  op.ts             |       0 |        0 |       0 |       0 | 33-124            
  opcode.ts         |       0 |      100 |     100 |       0 | 10-154            
  script.ts         |       0 |        0 |       0 |       0 | 26-158            
  sighashtype.ts    |       0 |        0 |       0 |       0 | 19-166            
  tx.ts             |       0 |        0 |       0 |       0 | 73-153            
  txBuilder.ts      |       0 |        0 |       0 |       0 | 70-224            
  unsignedTx.ts     |       0 |        0 |       0 |       0 | 31-222            
 ecash-lib/src/ffi  |       0 |        0 |       0 |       0 |                   
  ecash_lib_wasm.js |       0 |        0 |       0 |       0 | 3-336             
 ecash-lib/src/io   |       0 |        0 |       0 |       0 |                   
  bytes.ts          |       0 |        0 |       0 |       0 | 13-64             
  hex.ts            |       0 |        0 |       0 |       0 | 5-68              
  int.ts            |       0 |        0 |       0 |       0 |                   
  varsize.ts        |       0 |        0 |       0 |       0 | 14-47             
  writer.ts         |       0 |        0 |       0 |       0 |                   
  writerbytes.ts    |       0 |        0 |       0 |       0 | 21-79             
  writerlength.ts   |       0 |      100 |       0 |       0 | 16-41             
--------------------|---------|----------|---------|---------|-------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='0']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='703']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='0']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='247']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='0']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='108']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='0']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='688']
##teamcity[blockClosed name='Code Coverage Summary']
mv: cannot stat 'test_results/ecash-lib-integration-tests-junit.xml': No such file or directory
Build ecash-lib-integration-tests failed with exit code 1

python -> python3 in functional test

@bot ecash-lib-integration-tests

Failed tests logs:

====== Bitcoin ABC functional tests: abc_p2p_avalanche_quorum.py ======

------- Stdout: -------
2024-04-13T16:31:48.840000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20240413_163048/abc_p2p_avalanche_quorum_9
2024-04-13T16:32:08.527000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 147, in main
    self._run_test_internal()
  File "/work/test/functional/test_framework/test_framework.py", line 137, in _run_test_internal
    self.run_test()
  File "/work/test/functional/abc_p2p_avalanche_quorum.py", line 257, in run_test
    poll_and_assert_response(node, AvalancheVoteError.UNKNOWN)
  File "/work/test/functional/abc_p2p_avalanche_quorum.py", line 105, in poll_and_assert_response
    assert_equal(actual, expected)
  File "/work/test/functional/test_framework/util.py", line 61, in assert_equal
    raise AssertionError(
AssertionError: not(AvalancheVote(error=0, hash=549d16794885afafc52abf299717b4c0446d50e7bcbdc74fa9b856c4a25132e9) == AvalancheVote(error=-1, hash=549d16794885afafc52abf299717b4c0446d50e7bcbdc74fa9b856c4a25132e9))
2024-04-13T16:32:08.528000Z TestFramework (INFO): Stopping nodes
2024-04-13T16:32:08.787000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20240413_163048/abc_p2p_avalanche_quorum_9
2024-04-13T16:32:08.787000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20240413_163048/abc_p2p_avalanche_quorum_9/test_framework.log
2024-04-13T16:32:08.787000Z TestFramework (ERROR): 
2024-04-13T16:32:08.787000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20240413_163048/abc_p2p_avalanche_quorum_9' to consolidate all logs
2024-04-13T16:32:08.787000Z TestFramework (ERROR): 
2024-04-13T16:32:08.787000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-04-13T16:32:08.788000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2024-04-13T16:32:08.788000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_p2p_avalanche_quorum.py

@bot ecash-lib-integration-tests

Tail of the build log:

   Compiling cc v1.0.94
   Compiling once_cell v1.19.0
   Compiling log v0.4.21
   Compiling wasm-bindgen v0.2.92
   Compiling thiserror v1.0.58
   Compiling cfg-if v1.0.0
   Compiling abc-rust-lint v0.1.0 (/work/chronik/abc-rust-lint)
   Compiling generic-array v0.14.7
   Compiling quote v1.0.36
   Compiling syn v2.0.58
   Compiling secp256k1-sys-abc v0.4.1 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling block-buffer v0.10.4
   Compiling crypto-common v0.1.6
   Compiling digest v0.10.7
   Compiling ripemd v0.1.3
   Compiling sha2 v0.10.8
   Compiling wasm-bindgen-backend v0.2.92
   Compiling secp256k1-abc v0.20.3 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling thiserror-impl v1.0.58
   Compiling wasm-bindgen-macro-support v0.2.92
   Compiling wasm-bindgen-macro v0.2.92
   Compiling ecash-lib-wasm v0.1.0 (/work/modules/ecash-lib-wasm)
    Finished release-wasm [optimized] target(s) in 5.65s
Test depends on chronik-client. Building TypeScript...
/work/modules/chronik-client /work/modules/ecash-lib-wasm /work/abc-ci-builds/ecash-lib-tests

> chronik-client@0.26.2 prepublish
> npm run build


> chronik-client@0.26.2 build
> tsc


added 270 packages, and audited 271 packages in 7s

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

1 moderate severity vulnerability

To address all issues, run:
  npm audit fix

Run `npm audit` for details.
/work/modules/ecash-lib /work/modules/chronik-client /work/modules/ecash-lib-wasm /work/abc-ci-builds/ecash-lib-tests

added 362 packages, and audited 364 packages in 2s

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

found 0 vulnerabilities
CI configured to test build. Building...

> ecash-lib@0.1.0 build
> tsc && cp -r ./src/ffi ./dist

tests/txBuilder.test.ts(157,9): error TS2322: Type 'string' is not assignable to type 'Uint8Array'.
Build ecash-lib-tests failed with exit code 2

Remaining items to test:

  • (moved to separate diff) OP_CHECKMULTISIG test (Schnorr, ECDSA)
  • P2SH
  • OP_CODESEPARATOR handling
  • Fee/Leftover/Dust calculation, feePerKb
  • Insufficient sats handling

complete tests, reorder imports

@bot ecash-lib-integration-tests

  • Would be useful to include some tests for "normal" txs, even if they are not bounced off the node (could just test against expected raw hex, though now that the node is here, I suppose might as well use it...but can see it being complicated to give an address a normal balance in that kind of test script). For example,
  • generate a private key, generate a p2pkh address from this key, fund it, send a tx to a p2pkh adddress with change
  • send a tx to a p2pkh address without change (exact amount)

...p2psh? other txs?

I'm not clear on how I would use this txBuilder in, for example, Cashtab, based on the current test suite.

  • so leftover is change? is there a reason for this word choice / does it have a slightly different definition?

current way Cashtab builds a tx -- we use the ecash-coinselect lib to determine if we have change > dust, then add it there. So, to keep using that, I would need to update ecash-coinselect with schnorr sizes. I can see it being useful to just have this logic inside ecash-lib, there isn't really a use case for ecash-coinselect without tx building. Similar to the point above, though, I don't understand the expected behavior.

modules/ecash-lib/package.json
36 ↗(On Diff #47160)

nice. I haven't done this in other modules bc it didn't work. But, now I know why --- need to build the deps for those modules in CI first, which this does.

Will need to get around to updating this in Cashtab and other libs/apps in the monorepo.

modules/ecash-lib/src/txBuilder.ts
68 ↗(On Diff #47160)
92 ↗(On Diff #47160)

How do we get here? What does this error msg mean?

196–197 ↗(On Diff #47160)

style convention for doxygen comments in the monorepo. should have this handled with a linter but I am not really sure how.

modules/ecash-lib/tests/txBuilder.test.ts
230 ↗(On Diff #47160)

this would throw an error if the broadcast were rejected by the node?

558 ↗(On Diff #47160)

in the existing tx builder, the inputs are just utxos

how would these inputs be prepared in an app dev context? i.e., are we taking a Utxo_InNode and adding sequence, signData, and signatory every time?

This revision now requires changes to proceed.Apr 15 2024, 16:52
  • Would be useful to include some tests for "normal" txs, even if they are not bounced off the node (could just test against expected raw hex, though now that the node is here, I suppose might as well use it...but can see it being complicated to give an address a normal balance in that kind of test script). For example,
  • generate a private key, generate a p2pkh address from this key, fund it, send a tx to a p2pkh adddress with change
  • send a tx to a p2pkh address without change (exact amount)

I mean there are already a few tests like this, for example the it('TxBuilder P2PKH'), but if you want I can add some more.

I'm not clear on how I would use this txBuilder in, for example, Cashtab, based on the current test suite.

Well you'd add inputs and outputs and then sign, like in the tests... but I can see that we might need to simplify things

  • so leftover is change? is there a reason for this word choice / does it have a slightly different definition?

Yes, I've been using leftover instead of change because change has a lot of overlapping meanings (change also as in "modify" etc.), but we can change it to change if you think it's too confusing.

current way Cashtab builds a tx -- we use the ecash-coinselect lib to determine if we have change > dust, then add it there. So, to keep using that, I would need to update ecash-coinselect with schnorr sizes. I can see it being useful to just have this logic inside ecash-lib, there isn't really a use case for ecash-coinselect without tx building. Similar to the point above, though, I don't understand the expected behavior.

Currently indeed you'll have to select enough UTXOs already, and then ecash-lib will add the right change output, but it might be a good idea to add a function that given a list of UTXOs and outputs will build a tx "optimally" selecting some UTXOs.

modules/ecash-lib/src/txBuilder.ts
92 ↗(On Diff #47160)

The user specified two change/leftover outputs, but they can only specify one at most

196–197 ↗(On Diff #47160)

👍🏻 Did this manually everywhere else in this module before already I think

modules/ecash-lib/tests/txBuilder.test.ts
230 ↗(On Diff #47160)

Exactly, so this tests if the tx was successfully broadcast

558 ↗(On Diff #47160)

in the existing tx builder, the inputs are just utxos

I don’t think it’s as simple as that, you also have to provide the secret key, the sighash type, etc.

This current design is more “tx centric” (ie doesn’t hide the Bitcoiny details) and less “wallet centric”, but we can easily add a wallet abstraction above it

how would these inputs be prepared in an app dev context? i.e., are we taking a Utxo_InNode and adding sequence, signData, and signatory every time?

Yes, but this doesn’t generalize. To spend a P2SH output you have to specify the redeemScript, which is (generally) unknowable from a UTXO alone.

sequence is “mandatory” simply because there’s no obvious default.
SignData is everything required to build the sighash preimage that’s not part of the tx itself.
Signatory is everything required to build a scriptSig given a sighash pre image

I mean there are already a few tests like this, for example the it('TxBuilder P2PKH'), but if you want I can add some more.

I'm not sure how to go from Utxo_InNode to the inputs offered to TxBuilder in the tests

Yes, I've been using leftover instead of change because change has a lot of overlapping meanings (change also as in "modify" etc.), but we can change it to change if you think it's too confusing.

I personally like change for linguistic reasons. I imagine "kids these days" do not even know what change is, since everything is by credit card or some other "innovation" that is used instead of cash (with the main featureset being...missing features of cash). It is ambiguous in english since it does also have some other meanings. Is there a german word that just always means "money back" change? Either way, I don't think it's that important. Just a comment "this is change" is fine.

"leftover" is also ambiguous in english since this usually means "yesterday's dinner, now in a plastic storage container in the fridge." Of course, compared with "change," this has much less overlap with common software terminology.

it might be a good idea to add a function that given a list of UTXOs and outputs will build a tx "optimally" selecting some UTXOs.

This is currently what ecash-coinselect does, though it only supports an accumlative algorithm atm. it would be nice to just get rid of that lib because it's all handled here. The one thing it does that is not involved in tx building is calculate the max amount of xec a wallet can send.

modules/ecash-lib/tests/txBuilder.test.ts
212–223 ↗(On Diff #47160)

my main confusion here is where this input data comes from

this doesn't look like a Utxo_InNode -- so, for an app dev, is there some intermediate step to go from a utxo to the inputs required by this lib?

modules/ecash-lib/tests/txBuilder.test.ts
212–223 ↗(On Diff #47160)

Yes.

  • prevOut and signData.value would come from a Utxo
  • signData.outputScript could also come from a Utxo, except for P2SH outputs
  • script: will be empty and filled in by the signatory, except if the signatory is empty. We should probably make this more optional
  • sequence will usually be 0xffffffff or 0xfffffffe.
  • signatory for P2PKH will be P2PKHSignatory, which provides the secret key, public key and the sighash type.

Ok so remaining punchlist here

  • At least a comment about how "leftover" is "change"
  • imo we might as well put a default for sequence, though it should still be available as a param.

re this:

script: will be empty and filled in by the signatory, except if the signatory is empty. We should probably make this more optional

action:

  • I don't really get what this is or what "more optional" means

re this:

signatory for P2PKH will be P2PKHSignatory, which provides the secret key, public key and the sighash type.

action:

  • signatory -- this is something else we get from this library? where does it come from?

And -- just in telegram or in comments here is fine,

  • short user story about how I would use this lib to make a normal ecash tx (p2pkh wallet) in Cashtab

Tail of the build log:

   Compiling once_cell v1.19.0
   Compiling cc v1.0.94
   Compiling log v0.4.21
   Compiling thiserror v1.0.58
   Compiling cfg-if v1.0.0
   Compiling wasm-bindgen v0.2.92
   Compiling abc-rust-lint v0.1.0 (/work/chronik/abc-rust-lint)
   Compiling generic-array v0.14.7
   Compiling quote v1.0.36
   Compiling syn v2.0.60
   Compiling secp256k1-sys-abc v0.4.1 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling crypto-common v0.1.6
   Compiling block-buffer v0.10.4
   Compiling digest v0.10.7
   Compiling ripemd v0.1.3
   Compiling sha2 v0.10.8
   Compiling wasm-bindgen-backend v0.2.92
   Compiling secp256k1-abc v0.20.3 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling thiserror-impl v1.0.58
   Compiling wasm-bindgen-macro-support v0.2.92
   Compiling wasm-bindgen-macro v0.2.92
   Compiling ecash-lib-wasm v0.1.0 (/work/modules/ecash-lib-wasm)
    Finished release-wasm [optimized] target(s) in 6.10s
Test depends on chronik-client. Building TypeScript...
/work/modules/chronik-client /work/modules/ecash-lib-wasm /work/abc-ci-builds/ecash-lib-tests

> chronik-client@0.26.2 prepublish
> npm run build


> chronik-client@0.26.2 build
> tsc


added 270 packages, and audited 271 packages in 6s

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

1 moderate severity vulnerability

To address all issues, run:
  npm audit fix

Run `npm audit` for details.
/work/modules/ecash-lib /work/modules/chronik-client /work/modules/ecash-lib-wasm /work/abc-ci-builds/ecash-lib-tests

added 362 packages, and audited 364 packages in 2s

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

found 0 vulnerabilities
CI configured to test build. Building...

> ecash-lib@0.1.0 build
> tsc && cp -r ./src/ffi ./dist

src/txBuilder.ts(10,29): error TS2307: Cannot find module './sighashtype.js' or its corresponding type declarations.
Build ecash-lib-tests failed with exit code 2

make some input fields optional, add comments

  • signatory -- this is something else we get from this library? where does it come from?

A signatory is a function that signs a type of script (i.e. creates a scriptSig that makes the scriptPubKey verify to True), so we have P2PKHSignatory for P2PKH outputs and P2PKSignatory for P2PK outputs.

The Agora script also has a signatory, to accept a trade, see AgoraSignatory in agora.ts for an example.

modules/ecash-lib/src/tx.ts
12 ↗(On Diff #47349)

could use a short explanation of why this is the default in the comments. most app devs will probably not know what this means.

modules/ecash-lib/tests/txBuilder.test.ts
199 ↗(On Diff #47351)

🎯 nice exactly what i was looking for here

235 ↗(On Diff #47351)

will this always be the format for outputs, requiring a script?

This is the kind of place where you think input like address would be best handled by some kind of wrapper, if necessary? Otherwise the way it works now, we need script: <outputScript>?

modules/ecash-lib/src/tx.ts
12 ↗(On Diff #47349)

sure

modules/ecash-lib/tests/txBuilder.test.ts
235 ↗(On Diff #47351)

Yes, you always need to specify the Script, but I think it's better to have that explicit here, and then later we can add a addressScript function or so

add doc comment to DEFAULT_SEQUENCE

This revision is now accepted and ready to land.Apr 21 2024, 20:33