⚙ D20520 [alp-dex] Batch inventory maintain and misc sweeps

bytesofman created this revision.Aug 31 2026, 10:39

bytesofman held this revision as a draft.

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) I'm approving this in spirit, but not in Git.

────────────────────────────────────────────────────────────────────────

minor [Functional Correctness]
→ ]8;;vscode://file//work/apps/alp-dex/scripts/summarizeWallets.ts:72apps/alp-dex/scripts/summarizeWallets.ts:72-75]8;;

Do not report a failed fee query as a zero balance.

When this request fails, feeUtxos stays empty and the script later
prints utxos=0 sats=0. main() then exits successfully. Automation can
treat an unavailable Chronik response as a valid empty fee wallet.

Mark the fee snapshot unavailable and exit nonzero, or rethrow after any
desired partial output.

────────────────────────────────────────────────────────────────────────

major [Stability & Availability]
→ ]8;;vscode://file//work/apps/alp-dex/src/inventory/maintain.ts:195apps/alp-dex/src/inventory/maintain.ts:195]8;;

Bound inventory funding work per maintenance pass.

This loop has no total batch limit. For example, a caller can provide
utxoAtoms = 1n and a large valid atom balance. The loop then performs
one broadcast per 28 units. This can run for billions of iterations, block
maintenance, and spend substantial fees before returning.

Set a maximum number of funding batches per pass and defer remaining units
to a later maintenance run.

────────────────────────────────────────
Review complete
2 findings ✔

Major 1
Minor 1

8 files reviewed:

apps/alp-dex/README.md
apps/alp-dex/index.ts
apps/alp-dex/package.json
apps/alp-dex/scripts/summarizeWallets.ts
apps/alp-dex/src/inventory/maintain.ts
apps/alp-dex/src/inventory/plan.ts
apps/alp-dex/test/inventory.test.ts
modules/ecash-wallet/src/wallet.ts

────────────────────────────────────────

Print all AI prompts: coderabbit review --show-prompts

Harbormaster completed remote builds in B37952: Diff 61037.Aug 31 2026, 10:52

Drop per-build debug logs. Keep ALP/misc batching, former-inventory hold, summarize-wallets, and the findIndex/-1 requiredUtxo fix.

bytesofman retitled this revision from [alp-dex] Batch inventory mint and diagnose chained sats crashes to [alp-dex] Batch inventory maintain and fix requiredUtxo lookup.Aug 31 2026, 11:07

bytesofman edited the summary of this revision. (Show Details)

bytesofman edited the test plan for this revision. (Show Details)

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) Put abstractions in code, details in metadata. Program for the general case, and put the specifics outside the compiled code base.

────────────────────────────────────────────────────────────────────────

minor [Functional Correctness]
→ ]8;;vscode://file//work/apps/alp-dex/src/inventory/classify.ts:194apps/alp-dex/src/inventory/classify.ts:194]8;;

Add atoms as a deterministic sort tie-breaker.

Two piles can have the same utxoCount and tokenId but different
atoms. They compare equal here, so input order changes the key built in
index.ts and sends duplicate Telegram alerts for unchanged inventory.
Sort by atoms after tokenId.

────────────────────────────────────────────────────────────────────────

minor [Functional Correctness]
→ ]8;;vscode://file//work/apps/alp-dex/index.ts:107apps/alp-dex/index.ts:107]8;;

Clear the former-inventory key when no pile remains.

If a pile disappears and then returns with the same contents, this
condition skips notification because lastFormerInventoryKey still has
the old key. Reset the key on an empty result before this condition.

────────────────────────────────────────
Review complete
2 findings ✔

Minor 2

12 files reviewed:

apps/alp-dex/README.md
apps/alp-dex/index.ts
apps/alp-dex/package.json
apps/alp-dex/scripts/summarizeWallets.ts
apps/alp-dex/src/inventory/classify.ts
apps/alp-dex/src/inventory/maintain.ts
apps/alp-dex/src/inventory/plan.ts
apps/alp-dex/src/ops/telegramMessages.ts
apps/alp-dex/test/inventory.test.ts
apps/alp-dex/test/telegramMessages.test.ts

... and 2 more files
────────────────────────────────────────

Print all AI prompts: coderabbit review --show-prompts

Harbormaster completed remote builds in B37953: Diff 61038.Aug 31 2026, 11:18

Address CodeRabbit: cap fund batches per pass, fail fee snapshot instead of printing zeros, sort former-inventory by atoms, reset notify key when piles are gone.

bytesofman edited the summary of this revision. (Show Details)Aug 31 2026, 11:33

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) We don't ship debt; we refinance it into tests.

────────────────────────────────────────────────────────────────────────

minor [Stability & Availability]
→ ]8;;vscode://file//work/apps/alp-dex/index.ts:130apps/alp-dex/index.ts:130]8;;

Retry failed former-inventory notifications.

Line 130 records the key even when telegramOps.send() later rejects. The
same piles then do not notify again until their key changes. Update the
key only after a successful send, or restore the prior key in the
rejection handler.

────────────────────────────────────────────────────────────────────────

major [Functional Correctness]
→ ]8;;vscode://file//work/modules/ecash-wallet/src/wallet.ts:2300modules/ecash-wallet/src/wallet.ts:2300-2306]8;;

Keep default dust values in chained output accounting.

this.action.outputs permits omitted sats; paymentOutputsToTxOutputs
converts those outputs to dustSats. requireSats() now throws for the
same valid output in _buildSizeLimitExceededChained. A large chained XEC
action with default-dust recipients therefore fails to build. Use
output.sats ?? dustSats here, while retaining an error for a missing
array element.

────────────────────────────────────────
Review complete
2 findings ✔

Major 1
Minor 1

12 files reviewed:

apps/alp-dex/README.md
apps/alp-dex/index.ts
apps/alp-dex/package.json
apps/alp-dex/scripts/summarizeWallets.ts
apps/alp-dex/src/inventory/classify.ts
apps/alp-dex/src/inventory/maintain.ts
apps/alp-dex/src/inventory/plan.ts
apps/alp-dex/src/ops/telegramMessages.ts
apps/alp-dex/test/inventory.test.ts
apps/alp-dex/test/telegramMessages.test.ts

... and 2 more files
────────────────────────────────────────

Print all AI prompts: coderabbit review --show-prompts

Harbormaster completed remote builds in B37956: Diff 61041.Aug 31 2026, 11:44

Silent no-op maintain logs. Retry former-inventory Telegram if send fails (only advance the dedup key after success). requireSats/dust fallback in chained XEC builds is out of scope for this diff.

bytesofman edited the summary of this revision. (Show Details)Aug 31 2026, 13:04

bytesofman edited the test plan for this revision. (Show Details)

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) Simplicity does not precede complexity, but follows it. - Alan Perlis

────────────────────────────────────────────────────────────────────────

minor [Functional Correctness]
→ ]8;;vscode://file//work/apps/alp-dex/index.ts:121apps/alp-dex/index.ts:121-138]8;;

Prevent duplicate notices while a send is pending.

This detached send() call leaves lastFormerInventoryKey unchanged
until the promise resolves. A later maintenance pass with the same piles
then queues another Telegram message. Track an in-flight key before
calling send(), and clear that key only when the send fails.

────────────────────────────────────────────────────────────────────────

minor [Maintainability & Code Quality]
→ ]8;;vscode://file//work/apps/alp-dex/index.ts:28apps/alp-dex/index.ts:28-30]8;;

Do not suppress former-inventory warnings.

maintainHadActivity() returns false when a pass only finds
formerInventory. This return bypasses the warning loop at Lines 40-46.
Move the return after that loop, or also require
inventory.formerInventory.length === 0.

────────────────────────────────────────────────────────────────────────

major [Functional Correctness]
→ ]8;;vscode://file//work/modules/ecash-wallet/src/wallet.ts:2300modules/ecash-wallet/src/wallet.ts:2300-2306]8;;

Preserve default dust values for chained outputs.

PaymentOutput.sats can be omitted. Existing output processing defaults
it to dustSats. For an oversized non-token action, this path now throws
when a valid output omits sats, instead of applying that default. Use
output.sats ?? dustSats here, with the action or default dust value.

────────────────────────────────────────
Review complete
3 findings ✔

Major 1
Minor 2

12 files reviewed:

apps/alp-dex/README.md
apps/alp-dex/index.ts
apps/alp-dex/package.json
apps/alp-dex/scripts/summarizeWallets.ts
apps/alp-dex/src/inventory/classify.ts
apps/alp-dex/src/inventory/maintain.ts
apps/alp-dex/src/inventory/plan.ts
apps/alp-dex/src/ops/telegramMessages.ts
apps/alp-dex/test/inventory.test.ts
apps/alp-dex/test/telegramMessages.test.ts

... and 2 more files
────────────────────────────────────────

Print all AI prompts: coderabbit review --show-prompts

bytesofman edited the summary of this revision. (Show Details)Aug 31 2026, 13:11

Harbormaster completed remote builds in B37957: Diff 61042.Aug 31 2026, 13:15

Drop former-inventory hold/notify and silent no-op maintain logs (now D20524 / D20525). Rebase onto master after D20521; keep ALP/misc batching and the findIndex requiredUtxo fix.

bytesofman edited the summary of this revision. (Show Details)Sep 1 2026, 07:38

bytesofman edited the test plan for this revision. (Show Details)

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) Because, no one wants to be a human linter.

────────────────────────────────────────
Review complete
No findings ✔

8 files reviewed:

apps/alp-dex/README.md
apps/alp-dex/package.json
apps/alp-dex/scripts/summarizeWallets.ts
apps/alp-dex/src/inventory/maintain.ts
apps/alp-dex/src/inventory/plan.ts
apps/alp-dex/test/inventory.test.ts
modules/ecash-wallet/src/wallet.test.ts
modules/ecash-wallet/src/wallet.ts

────────────────────────────────────────

Harbormaster completed remote builds in B37971: Diff 61058.Sep 1 2026, 07:46

Move summarize-wallets to D20527. This diff is now ALP/misc batching plus the findIndex requiredUtxo fix.

bytesofman published this revision for review.Sep 1 2026, 08:59

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) undefined is not a business requirement.

────────────────────────────────────────
Review complete
No findings ✔

5 files reviewed:

apps/alp-dex/src/inventory/maintain.ts
apps/alp-dex/src/inventory/plan.ts
apps/alp-dex/test/inventory.test.ts
modules/ecash-wallet/src/wallet.test.ts
modules/ecash-wallet/src/wallet.ts

────────────────────────────────────────

Harbormaster completed remote builds in B37973: Diff 61060.Sep 1 2026, 09:06

The ecash-wallet lib needs a version bump also

apps/alp-dex/src/inventory/maintain.ts
108 ↗	(On Diff #61060)	Don't you have an assertPositiveInteger already implemented ? Then you can just use that && >= 1
apps/alp-dex/src/inventory/plan.ts
64 ↗	(On Diff #61060)	Same I think there is a facility that exists already

This revision now requires changes to proceed.Sep 1 2026, 09:34

Drop ecash-wallet (now D20528). Rebase onto master after D20525. Reuse assertPositiveCountOrNone for fund-batch counts.

bytesofman retitled this revision from [alp-dex] Batch inventory maintain and fix requiredUtxo lookup to [alp-dex] Batch inventory maintain and misc sweeps.Sep 1 2026, 10:00

bytesofman edited the summary of this revision. (Show Details)

bytesofman edited the test plan for this revision. (Show Details)

Harbormaster completed remote builds in B37975: Diff 61063.Sep 1 2026, 10:08

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) Find bugs once. Once a human tester finds a bug, it should be the last time a human tester finds that bug. Automatic tests should check for it from then on.

────────────────────────────────────────
Review complete
No findings ✔

4 files reviewed:

apps/alp-dex/src/inventory/actions.ts
apps/alp-dex/src/inventory/maintain.ts
apps/alp-dex/src/inventory/plan.ts
apps/alp-dex/test/inventory.test.ts

────────────────────────────────────────

Fabien requested changes to this revision.Sep 2 2026, 08:22

Fabien added inline comments.

apps/alp-dex/src/inventory/plan.ts
81 ↗	(On Diff #61063)	There is a change in behavior here as remainingUnits can't be 0 anymore. An option would be to pass a parameter to indicate if it's a strict positive (>0) or not (>=0)

This revision now requires changes to proceed.Sep 2 2026, 08:22

assertPositiveCountOrNone takes min so remainingUnits may be 0 and maxFundBatchesPerToken must be >= 1. Rebase onto master after D20524; former-inventory hold still applies, misc sweeps are batched.

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) Blink twice if you're a bug.

────────────────────────────────────────
Review complete
No findings ✔

4 files reviewed:

apps/alp-dex/src/inventory/actions.ts
apps/alp-dex/src/inventory/maintain.ts
apps/alp-dex/src/inventory/plan.ts
apps/alp-dex/test/inventory.test.ts

────────────────────────────────────────

Harbormaster completed remote builds in B37990: Diff 61088.Sep 2 2026, 09:56

Beware that the suggestion changes the behavior of the function so it might impact other callsites

apps/alp-dex/src/inventory/plan.ts
40 ↗	(On Diff #61088)	With your implementation the lower bound is exclusive not inclusive (which imo is counter intuitive, see suggestion below).
46 ↗	(On Diff #61088)	if you want no change in behavior min should default to 1 (with the suggestion below)
48–58 ↗	(On Diff #61088)

This revision now requires changes to proceed.Sep 2 2026, 10:10

assertPositiveCountOrNone: min is inclusive (return count >= min), default min=1 so action zero-counts stay no-ops. remainingUnits still passes min=0.

Harbormaster completed remote builds in B37994: Diff 61093.Sep 2 2026, 10:37

Build Bitcoin ABC Diffs / Diff Testing (ai-review) passed.
CodeRabbit Review

Diff : committed changes only
Compare : HEAD → master
Directory : work
────────────────────────────────────────

(\(\
(• .•) Bazinga! You missed a semicolon.

────────────────────────────────────────
Review complete
No findings ✔

4 files reviewed:

apps/alp-dex/src/inventory/actions.ts
apps/alp-dex/src/inventory/maintain.ts
apps/alp-dex/src/inventory/plan.ts
apps/alp-dex/test/inventory.test.ts

────────────────────────────────────────

Fabien accepted this revision.Sep 2 2026, 13:49

Fabien added inline comments.

apps/alp-dex/src/inventory/plan.ts
40–42 ↗	(On Diff #61093)	Default `1` so a zero count is a no-op rather than an error. This makes no sense, count is never modified so the function is always a no-op, it's an assert

This revision is now accepted and ready to land.Sep 2 2026, 13:49

Closed by commit rABCad677fe54124: [alp-dex] Batch inventory maintain and misc sweeps. · Explain WhySep 2 2026, 14:06

This revision was automatically updated to reflect the committed changes.

bytesofman added a commit: rABCad677fe54124: [alp-dex] Batch inventory maintain and misc sweeps.

[alp-dex] Batch inventory maintain and misc sweeps
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 61106

apps/alp-dex/src/inventory/actions.ts

apps/alp-dex/src/inventory/maintain.ts

apps/alp-dex/src/inventory/plan.ts

apps/alp-dex/test/inventory.test.ts

[alp-dex] Batch inventory maintain and misc sweepsClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 61106

apps/alp-dex/src/inventory/actions.ts

apps/alp-dex/src/inventory/maintain.ts

apps/alp-dex/src/inventory/plan.ts

apps/alp-dex/test/inventory.test.ts

[alp-dex] Batch inventory maintain and misc sweeps
ClosedPublic
Actions

Revision Contents
Changeset List