Allows building Bitcoin transactions, able to handle any crazy covenant; able to calculate leftover outputs precisely.
Details
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project - Commits
- rABCe78db2d47e63: [ecash-lib] Add `TxBuilder` to build Bitcoin transactions
npm run test && npm run build && npm run integration-tests
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (ecash-lib-tests) failed.
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 2Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (ecash-lib-tests) failed.
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 2Comment Actions
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 1Comment Actions
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
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.
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
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (ecash-lib-tests) failed.
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 2Comment Actions
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
Comment Actions
- 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? |
Comment Actions
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) |
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
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. |
Comment Actions
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.
|
Comment Actions
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
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (ecash-lib-tests) failed.
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 2Comment Actions
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>? |