Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABC419b85ab298a: [ecash-wallet, cashtab-faucet] Support arbitrary amount SLP burns with…

Summary

A number of desired eCash network actions require more than one transaction to fulfill. The job of ecash-wallet is to translate human-facing dev desires ("actions") into explicit network action. To do this, we will need to support some types of chained tx.

This support should be introduced incrementally as some chained txs will be more complex than others and most will require special dedicated logic.

In this diff we introduce chained txs for one of the easiest use cases: intentionally burning SLP tokens. To do this, we must prepare a utxo of the exact atoms we want to burn, and then burn that utxo.

Cashtab currently burns SLP tokens with "unintentional burns" i.e. a SEND tx where the outputs are less than the inputs. This is a cleaner way of doing it.

ALP allows intentional burns with change, so we do not add this type of chained TX support for ALP burns.

Breaking change as we no longer simply forward chronik's response from chronik-client broadcast() method. We need some more info to be useful for chained txs.

Test Plan

npm test, CI tests

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bytesofman created this revision.Sep 15 2025, 18:02

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 15 2025, 18:02

Harbormaster completed remote builds in B34449: Diff 55669.Sep 15 2025, 18:12

lose BuiltTx, cleaner

Harbormaster completed remote builds in B34450: Diff 55670.Sep 15 2025, 18:22

bytesofman published this revision for review.Sep 15 2025, 18:32

bytesofman mentioned this in D18587: [ecash-wallet] Support intentional burns of arbitrary SLP amounts with chained txs.Sep 15 2025, 22:07

Fabien added a subscriber: Fabien.Sep 16 2025, 08:08

Fabien added inline comments.

modules/ecash-wallet/src/wallet.ts
342 ↗	(On Diff #55670)	can't this be generalized ? The wallet should know whether the utxo belong to it or not and update itself by marking all the inputs as spent and adding this utxo. Maybe this "self sync" can be made optional so it can support weird edge cases
600 ↗	(On Diff #55670)	I don't like the name, it made me bug. What about txids ?
605 ↗	(On Diff #55670)	maybe it's worth keeping it a single value (always use the first) with a deprecation notice, or rename so it's clear it's plural
608 ↗	(On Diff #55670)	dito

bytesofman planned changes to this revision.Sep 16 2025, 13:33

bytesofman added inline comments.

modules/ecash-wallet/src/wallet.ts
342 ↗	(On Diff #55670)	yes, this needs some more thought the aim here for chained txs is for `selectUtxos` to get all necessary utxos from the initial Action. In this way, we know we have enough utxos for the whole Action up front, and there is no risk of utxo sync issues in between tx broadcasts. this implementation though is calling selectUtxos again in processing the sub actions.
600 ↗	(On Diff #55670)	yes, should be updated. This is a copypasta artifact of converting BuiltTx
605 ↗	(On Diff #55670)	ditto. will updated to use plural. it's a breaking change anyway, so I think keeping a `size` would cause more confusion than it resolves
608 ↗	(On Diff #55670)	will update

automatically update utxo set on tx builds

Harbormaster completed remote builds in B34483: Diff 55718.Sep 16 2025, 19:07

Build Bitcoin ABC Diffs / Diff Testing (cashtab-faucet-tests) failed.

Failed tests logs:

====== routes.js: /claimxec/:address sends an airdrop if called with an address with no tx history.routes.js /claimxec/:address sends an airdrop if called with an address with no tx history ======
Error: expected {   address: 'ecash:qrfkcnzdm0dvkrc20dhcf7qv23vt736ynuujzxnzs6',   msg: 'Success',   txid: 'd19c496e82bd160c841968ec0d2b61bf64cb884b002835649594cd973967d33b' } response body, got {   address: 'ecash:qrfkcnzdm0dvkrc20dhcf7qv23vt736ynuujzxnzs6',   msg: 'Success' }
Error: expected {
  address: 'ecash:qrfkcnzdm0dvkrc20dhcf7qv23vt736ynuujzxnzs6',
  msg: 'Success',
  txid: 'd19c496e82bd160c841968ec0d2b61bf64cb884b002835649594cd973967d33b'
} response body, got {
  address: 'ecash:qrfkcnzdm0dvkrc20dhcf7qv23vt736ynuujzxnzs6',
  msg: 'Success'
}
    at Context.<anonymous> (src/routes.test.ts:503:14)
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
----
    at error (node_modules/supertest/lib/test.js:335:15)
    at Test._assertBody (node_modules/supertest/lib/test.js:193:16)
    at /work/apps/cashtab-faucet/node_modules/supertest/lib/test.js:308:13
    at Test._assertFunction (node_modules/supertest/lib/test.js:285:13)
    at Test.assert (node_modules/supertest/lib/test.js:164:23)
    at localAssert (node_modules/supertest/lib/test.js:120:14)
    at fn (node_modules/supertest/lib/test.js:125:7)
    at Test.callback (node_modules/superagent/src/node/index.js:913:12)
    at fn (node_modules/superagent/src/node/index.js:1166:18)
    at IncomingMessage.<anonymous> (node_modules/superagent/src/node/parsers/json.js:19:7)
    at IncomingMessage.emit (node:events:531:35)
    at IncomingMessage.emit (node:domain:489:12)
    at endReadableNT (node:internal/streams/readable:1698:12)
    at processTicksAndRejections (node:internal/process/task_queues:90:21)

      + expected - actual

       {
         "address": "ecash:qrfkcnzdm0dvkrc20dhcf7qv23vt736ynuujzxnzs6"
         "msg": "Success"
      +  "txid": "d19c496e82bd160c841968ec0d2b61bf64cb884b002835649594cd973967d33b"
       }

Each failure log is accessible here:
routes.js: /claimxec/:address sends an airdrop if called with an address with no tx history.routes.js /claimxec/:address sends an airdrop if called with an address with no tx history

bytesofman marked an inline comment as done.Sep 16 2025, 19:10

bytesofman added inline comments.

modules/ecash-wallet/src/wallet.ts
342 ↗	(On Diff #55670)	ok -- for now I have generalized to auto-determine the utxo set on every `build()` call Initially I was going to do it on every `broadcast()` -- but for chained txs, we need it done between `build()`s, as we do not want to `broadcast()` any until we know we can build them all Some complications from doing it on `build()` that are handled and tested here We may not want to update the utxo set ... e.g. if we are testing the `build()` routine, or mb some other reason. imo this is the less common, power-user preference...so this is not default, and requires specified params. We need to know SLP change outputs that ecash-wallet generates in order to add these to the utxo set. These are calculated in `finalizeOutputs`, but were just an intermediate artifact as we used to convert these straight to tx builder outputs. We now return these from `finalizeOutputs` so they can be used. The utxo updating does not preserve the same utxo ordering as before, which can impact txs created with the current accumulative utxo selection algorithm. imo this is not a big deal, but I did have to change the order of some burn vs chained burn tests vs previous versions of this diff to account for this all in all ... I think an important feature. The wallet "knows" exactly what the impact to its utxo set will be for every tx it creates, so there is no reason it should not update when this happens. This will allow for crazy-fast rapid-fire send txs. Later on, we can also add a websocket to update on received txs. the aim here for chained txs is for selectUtxos to get all necessary utxos from the initial Action. I've abandoned this for now. We could have edge cases where we have enough sats to fuel and SEND but not a BURN as part of a chained SLP burn. However in this case, we will be unable to `build()` the second tx in the chain and an error will be thrown before anything in the chain is broadcast.

CI investigation

mock wallet utxos in between tests

bytesofman added inline comments.Sep 16 2025, 20:33

apps/cashtab-faucet/src/routes.test.ts
121 ↗	(On Diff #55719)	now that `.build().broadcast()` updates the wallet's utxo set -- running all the tests would cause later tests to use different utxos and thus create a different txid confirms we are seeing the right behavior from the utxo updating, will be useful in apps that are testing subsequent txs without updating utxos with regtest

Harbormaster completed remote builds in B34484: Diff 55719.Sep 16 2025, 20:41

some not so great nits, but tbh this is a bit difficult to review. I'll need to spend more time on this. Might also allocate some time at some point to tidy everything in here up a bit

modules/ecash-wallet/src/wallet.ts
256 ↗	(On Diff #55719)	i don't know how to write comments like that, i don't know what words to use
276 ↗	(On Diff #55719)	shouldn't the doc comment be before the function definition?
288 ↗	(On Diff #55719)
355 ↗	(On Diff #55719)
357 ↗	(On Diff #55719)	is it possible to instead of having this boolean to instead either deduce it automatically (I'm not sure why this flag is needed?) it seems like users of this function shouldn't need to worry about updating the UTXOs, seems like a very internal thing. if it's there to handle some internal stuff, it should be kept internal
518 ↗	(On Diff #55719)	also here
531 ↗	(On Diff #55719)	assign `finalizedOutputs[i]` to a const up top, then you should be able to skip the !
538 ↗	(On Diff #55719)	consider using type narrowing: https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates

This revision now requires changes to proceed.Sep 16 2025, 21:02

bytesofman marked 8 inline comments as done.Sep 16 2025, 21:35

bytesofman added inline comments.

modules/ecash-wallet/src/wallet.ts
256 ↗	(On Diff #55719)	this is actually an artifact from an earlier approach taken in this diff. Ended up implementing in a different way. But, good catch, removed this change.
276 ↗	(On Diff #55719)	Yes, another WIP notes issue. Fixed.
357 ↗	(On Diff #55719)	The default behavior (if you just call `build()` is to use ALL_BIP143 and automatically update the utxos The reason for the flag -- the user may wish to `build()` without changing the utxo set of the wallet. The specific use case for this is myself in developing this library -- e.g. we call `build()` in various tests, often to show an error is thrown, and in this case we do not want to mutate the utxo set. We could have some other method / wrapper method to make them more distinct? But even with this... would probably still need to have the params. Could do something like `_build()` with the params, public `build()` and `buildDev()` that only take a sighash param. Seems a bit more complicated though, esp as the default params are good in `build()`
531 ↗	(On Diff #55719)	nice, TIL
538 ↗	(On Diff #55719)	just having the const def fixed this too

remove unused param, better function doc, better type checking

add tests for getUtxoFromOutput

Harbormaster completed remote builds in B34488: Diff 55723.Sep 16 2025, 21:46

tbh this is a bit difficult to review.

I could split out the utxo determination stuff? It is though extensively tested and well vetted by ts, both by the unit tests for getUtxoFromOutput and the demonstration that removing all of the .sync() calls from transactions.test.ts after the initial sync() does not cause any test failures.

Harbormaster completed remote builds in B34489: Diff 55724.Sep 16 2025, 21:49

tobias_ruck requested changes to this revision.Sep 22 2025, 19:19

tobias_ruck added inline comments.

modules/ecash-wallet/src/wallet.ts
283 ↗	(On Diff #55724)	seems like a better solution is to add a `clone` function to the wallet, and then a user can get the same behavior without this boolean
525 ↗	(On Diff #55724)	must be undefined? you mean must not?
1402 ↗	(On Diff #55724)	this seems clearer in this context
357 ↗	(On Diff #55719)	it seems better to add a "clone" function, then you run the test on that clone and immediately discard it, then you don't need this flag

This revision now requires changes to proceed.Sep 22 2025, 19:19

remove cumbersome utxo update flag, fix comment error, more clear key for utxo selection result determining chained req

Harbormaster completed remote builds in B34532: Diff 55787.Sep 22 2025, 20:20

bytesofman added inline comments.Sep 22 2025, 20:20

modules/ecash-wallet/src/wallet.ts
283 ↗	(On Diff #55724)	yeah this is much cleaner, dont know what i was thinking
525 ↗	(On Diff #55724)	d'oh, yeah tho we should never get here (I usually prefer to keep these kinds of `throw` checks in vs cast a type) updated

tobias_ruck requested changes to this revision.Sep 22 2025, 20:20

tobias_ruck added inline comments.

modules/ecash-wallet/src/wallet.ts
724–734 ↗	(On Diff #55787)	this design seems a bit bizarre to me tbh. It seems much better to retain BuiltTx, and then have BuiltAction be a composite of those. Then `txid`, `size` and `fee` is a method on BuiltTx, and they can be accessed via e.g. `action.txs[i].size()`. Makes it easier to iterate over. Also make sure to annotate types (txid is `string`).
725 ↗	(On Diff #55787)	probably a good idea to hash this when constructing the `BuiltTx`

This revision now requires changes to proceed.Sep 22 2025, 20:20

bytesofman marked 2 inline comments as done.Sep 22 2025, 20:44

preserve the BuiltTx class, use this for txid, size, fee

Harbormaster completed remote builds in B34533: Diff 55788.Sep 22 2025, 20:56

tobias_ruck accepted this revision.Sep 23 2025, 14:11

This revision is now accepted and ready to land.Sep 23 2025, 14:11

Closed by commit rABC419b85ab298a: [ecash-wallet, cashtab-faucet] Support arbitrary amount SLP burns with… (authored by bytesofman). · Explain WhySep 23 2025, 14:19

This revision was automatically updated to reflect the committed changes.

bytesofman added a commit: rABC419b85ab298a: [ecash-wallet, cashtab-faucet] Support arbitrary amount SLP burns with….

[ecash-wallet, cashtab-faucet] Support arbitrary amount SLP burns with automated chained txs
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 55807

apps/cashtab-faucet/src/routes.test.ts

apps/cashtab-faucet/src/routes.ts

apps/cashtab-faucet/src/transactions.test.ts

apps/cashtab-faucet/src/transactions.ts

apps/cashtab-faucet/test/vectors.ts

modules/ecash-wallet/README.md

modules/ecash-wallet/package-lock.json

modules/ecash-wallet/package.json

modules/ecash-wallet/src/wallet.test.ts

modules/ecash-wallet/src/wallet.ts

modules/ecash-wallet/test/postage.test.ts

modules/ecash-wallet/test/transactions.test.ts

[ecash-wallet, cashtab-faucet] Support arbitrary amount SLP burns with automated chained txsClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 55807

apps/cashtab-faucet/src/routes.test.ts

apps/cashtab-faucet/src/routes.ts

apps/cashtab-faucet/src/transactions.test.ts

apps/cashtab-faucet/src/transactions.ts

apps/cashtab-faucet/test/vectors.ts

modules/ecash-wallet/README.md

modules/ecash-wallet/package-lock.json

modules/ecash-wallet/package.json

modules/ecash-wallet/src/wallet.test.ts

modules/ecash-wallet/src/wallet.ts

modules/ecash-wallet/test/postage.test.ts

modules/ecash-wallet/test/transactions.test.ts

[ecash-wallet, cashtab-faucet] Support arbitrary amount SLP burns with automated chained txs
ClosedPublic
Actions

Revision Contents
Changeset List