Repository navigation
examples: add runnable, CI-checked examples and embed the README quick starts - #6
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![qbit-reviewer[bot]](https://avatars.githubusercontent.com/in/5446813?s=60&v=4){kind=small}
kiwidream
force-pushed
the
devex/examples
branch
from
October 29, 2026 18:49
5207afa to
82a7f0c
Compare
There was a problem hiding this comment.
5 findings posted inline.
Worth a look
6. [low] init() (WASM signer load) is not bounded by deadlineMs or the abort signal
examples/src/send-payment.ts:262
Promise.all([init(), scan]) waits on @qbit-oss/pqc's init(), which takes no signal or deadline. If the scan resolves and the WASM load stalls, sendPayment can outlive its documented single deadline (line 111) and ignore an abort until signing begins (lines 304-305). Unproven: init is in the external @qbit-oss/pqc package. A human should check whether its load can block indefinitely (for example a network fetch in browsers) or always finishes quickly from bundled bytes.
…k starts A private @qbit-oss/examples workspace, run by pnpm test and typecheck, with four examples that sign and verify with the real @qbit-oss/pqc 0.1.0: check-balance, payment-uri, message-proof and send-payment. send-payment is written to be copied into a wallet: - It validates the fee rate (between qbit Core's minimum relay fee and a wallet ceiling), the amount and the UTXO rows before signing. - It selects coins largest first, skips outputs worth no more than their fee, caps inputs below Core's standard transaction size, and folds change below Core's dust threshold into the fee. - It refuses a new payment while one is pending, checked again just before saving, and records each signature with its txid, input and leaf. - It stops on cancellation or deadline between steps, and after an unknown broadcast outcome follows broadcasting.md: look the transaction up with getTx, resubmit the same hex, and never rebuild with different inputs. - abandonPendingPayment releases a stuck payment only when the indexer does not know it and every input is still unspent; the only safe follow-up is a fee bump that spends the same inputs. The package READMEs embed their quick starts from standalone files that run as pasted under CommonJS and ESM, and readme-snippets.mjs --check fails the tests when a README block drifts from its source or a marker goes missing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kiwidream
force-pushed
the
devex/examples
branch
from
October 29, 2026 19:06
82a7f0c to
77fc91a
Compare
There was a problem hiding this comment.
Still open from earlier reviews
- [medium] Coin selection spends immature coinbase outputs, so payments from a mined wallet keep failing:
examples/src/send-payment.ts:264, first reported at82a7f0c - [low] reconcileBroadcast reads the clock twice and can throw invalid_options after the transaction was stored and sent:
examples/src/send-payment.ts:388, first reported at82a7f0c - [low · nit] The last-round lookup can use the whole remaining deadline, which skips the promised final resubmission:
examples/src/send-payment.ts:396, first reported at82a7f0c - [low · nit] init() (the WASM signer load) is not bounded by deadlineMs or the abort signal:
examples/src/send-payment.ts:262, first reported at82a7f0c - [low · nit] readmeBlock splits on LF only, so the quick-start tests fail on a CRLF checkout:
examples/test/quick-start.test.ts:62, first reported at82a7f0c
…adline Coin selection in send-payment.ts spent coinbase outputs younger than QBIT_COINBASE_MATURITY, so a payment from a freshly mined regtest wallet picked them, qbit Core rejected the transaction, and every retry made the same choice. sendPayment now reads the tip height with the UTXO scan and, for each candidate younger than 1,000 blocks (or without a usable block height), looks its funding transaction up with getTx, verified by txid, and skips it when the first input is a coinbase. Lookups run one at a time, once per transaction, at most 106 per payment, inside the payment deadline. A new immature_funds error says when only those outputs could pay; insufficient_funds and too_many_inputs are still decided first, without a lookup. A tip height that is not a safe, non-negative integer is refused. reconcileBroadcast reads the clock once when it checks the time left before end, so an end that passes between two readings ends as unknown_outcome, never invalid_options. Its last-round lookup gets at most half of the time left, so a stalled indexer cannot take the time the final resubmission needs. The quick-start test's readmeBlock splits on CRLF as well as LF, and readme-snippets.mjs skips, with a note, a tracked Markdown file missing from the working tree instead of crashing with ENOENT. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ck past maturity reconcileBroadcast capped only its last round's lookup. A lookup in an earlier round, or the resubmission after three not_found replies, still got all of the time left, so one stalled request could use up the deadline, and the last round, which looks up and resubmits once, never ran. Every request before the last round now has to end a poll interval before end. msLeft throws before the request once that point has passed, so a round with no time left sends nothing, and the next round is the last. The stalled-reconciliation test changes with it: a backend that never answers now gets a second lookup and the one last-round resubmission, which stall too through a new `stall` broadcast reply in the stub. Before, the first lookup took the whole deadline and nothing was resubmitted, which is the bug. Coin selection spends a coinbase output only at QBIT_COINBASE_MATURITY plus one confirmations, 1,001, the rule qbit Core's own wallet applies (GetTxBlocksToMaturity). The mempool takes a spend at 1,000, but the tip height comes from the indexer, which may be a block ahead of the node, and a reorg that leaves the tip a block lower evicts such a spend. The boundary test moves to 1,000 confirmations (skipped) and 1,001 (spent). selectInputs runs both passes through one walk, so the accumulate, cover and input-cap logic exists once. insufficient_funds and too_many_inputs are still decided before any lookup, and lookups still happen only for the outputs the walk reaches. The walk now stops at MAX_INPUTS before looking up another output; before, it could look one more up after reaching the cap. MAX_FUNDING_LOOKUPS, an alias of MAX_INPUTS, is gone, and the lookup bound uses MAX_INPUTS directly. readme-snippets.mjs splits the READMEs and the example sources on CRLF as well as LF, and --write rewrites a README only when one of its blocks changed, with the line ending most of its lines use. A README with mixed endings, such as a CRLF file holding an LF block, or an LF file with one CRLF line, failed both modes, since the script split it on whichever ending it found first. A uniform CRLF checkout already passed, because dedent trims each region line's trailing \r; splitting the sources on CRLF makes that explicit instead of incidental. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
4 findings posted inline.
Still open from earlier reviews
- [low] init() (the WASM signer load) is not bounded by deadlineMs or the abort signal:
examples/src/send-payment.ts:293, first reported at82a7f0c
Worth a look
6. [low] A wallet with many UTXOs may be unable to pay because the single address UTXO request can be refused by electrs
examples/src/send-payment.ts:291
Unproven. sendPayment fetches every UTXO with one getAddressUtxos call (line 291, GET /address/:addr/utxo). It has no fallback, and nothing in the repository mentions a UTXO limit.
Blockstream-derived electrs refuses that endpoint above its --utxos-limit (default 500) with an HTTP error. A regtest mining wallet, which the coinbase-maturity logic targets, has over 1,000 coinbase outputs, so every payment would fail before coin selection.
What to check: whether the qbit indexer enforces such a limit and with what default. If it does, document the limit or add a fallback.
This was referenced Oct 30, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a private
@qbit-oss/examplesworkspace, never published, thatpnpm testandtypecheckrun. It has four examples that sign and verify with the real@qbit-oss/pqc0.1.0:check-balance,payment-uri,message-proofandsend-payment.send-paymentis written for wallets to copy, so it errs on the side of not losing funds:broadcasting.md: look the transaction up withgetTx, resubmit the same hex, and never rebuild with different inputs. Cancellation and the deadline are checked between steps.abandonPendingPaymentreleases a stuck payment only when the indexer doesn't know it and every input is still unspent. The only safe follow-up is a fee bump spending the same inputs.The package READMEs embed their quick starts from standalone files. Those run as pasted under both CommonJS and ESM, and
readme-snippets.mjs --checkfails the tests on drift or a missing marker.Testing
pnpm verifypasses, including 77 example tests and the snippet check. So doespnpm smoke:consumer. Neither published tarball contains example files.Stacked on #5.
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.