Repository navigation
refactor: split each package's src/index.ts into modules - #10
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}
There was a problem hiding this comment.
Incomplete review: GitHub gave no textual diff for 2 file(s) not confirmed as non-text.
4 findings posted inline.
Findings without an inline location
1 nit
5. [low · nit] CONTRIBUTING still says the API docs come from TSDoc in each package's src/index.ts, now a barrel
CONTRIBUTING.md:101
CONTRIBUTING.md:101 says TypeDoc builds the API reference from the TSDoc comments in each package's src/index.ts. After this split, both index.ts files only re-export names, and the doc comments live in the topic modules (psbt.ts, descriptors.ts, esplora-client.ts and so on). The build still works (typedoc.json:12 follows the re-exports), but a contributor who follows the guide to fix a doc comment looks in the wrong file. This PR did not update CONTRIBUTING.md.
kiwidream
force-pushed
the
devex/module-split-pr
branch
from
October 30, 2026 15:30
7e44de4 to
26d4460
Compare
There was a problem hiding this comment.
Incomplete review: GitHub gave no textual diff for 2 file(s) not confirmed as non-text.
5 findings posted inline.
Still open from earlier reviews
- [medium · pre-existing · security] Response bodies are read with no size limit, so a hostile backend can exhaust client memory:
packages/api/src/transport.ts:404, first reported at7e44de4 - [medium · pre-existing] Descriptor object check takes exponential time on a tree whose subtrees are shared (a DAG):
packages/protocol/src/descriptors.ts:693, first reported at7e44de4 - [low · pre-existing] MempoolQbitClient.getAddressTxs compares the afterTxid cursor case-sensitively and rejects valid uppercase txids:
packages/api/src/mempool-qbit-client.ts:250, first reported at7e44de4 - [low · pre-existing] The verified address-history scan fetches pages through de-duplicated GETs that can predate its stats read:
packages/api/src/esplora-client.ts:488, first reported at7e44de4 - [low · nit] CONTRIBUTING still says the API docs come from TSDoc in each package's src/index.ts, now a barrel:
CONTRIBUTING.md:101, first reported at7e44de4
Move the 7,000-line packages/protocol/src/index.ts and the 3,800-line packages/api/src/index.ts into one module per topic. Code moves unchanged, doc comments included, as whole top-level declarations, and each module keeps them in their original relative order. Read it with `git show --color-moved=dimmed-zebra`. protocol, by the doc comments' @category tags: errors.ts (both error classes with their brand calls), networks.ts, constants.ts, consensus.ts, bytes.ts, bech32.ts, base58.ts, addresses.ts, scripts.ts, message.ts, transaction.ts (transaction types and their serialization), sighash.ts, psbt-fields.ts, psbt.ts, signing.ts, descriptors.ts and uri.ts. api: errors.ts (QbitApiError, its brand and the redaction helpers), raw-tx.ts (the raw-transaction decoder; it imports only @noble/hashes, no error class), validation.ts, adapters.ts, transport.ts, broadcast.ts, esplora-client.ts and mempool-qbit-client.ts. Each index.ts is a barrel that re-exports the same names by name (`export type` for types, as isolatedModules requires), so an export added to a module does not become public by accident. A module whose helpers another module uses lists them in an `export { ... }` at its end, so the declarations stay byte for byte the same; the barrels do not re-export them. The run-time imports form no cycle and no module imports index.ts at run time; the only cycles are type-only and compile away. Not shown as moved: - Import and export lines, and blank lines before or after declarations that used to follow one another directly. - api index.ts keeps its "Esplora response types" section, under a new comment saying why: testing.ts imports some of them from ./index, and in a module of their own tsup's DTS build would emit a shared lib/types-<hash>.d.ts chunk, which consumer:check rejects. - Comments that meant the single-file package: "this module" becomes "this package" in protocol descriptors.ts (isBip32ExtendedKeyExpression; decodeBase58Check now lives in base58.ts) and in api transport.ts (internalSignals and abortErrorFor). - The "P2MR signature hash" section banner, which headed the transaction types in index.ts, now opens sighash.ts, the port it describes, and transaction.ts gets a banner of its own. - `const MAX_RETRIES = 10` is moved, but too short for git to mark. - Two tests read source by path. The protocol bounds test now transpiles every .ts file under src/, keeping the tree, instead of index.ts alone. The api no-legacy-surface guard now reads every .ts file under src/, testing.ts included. Their assertions are unchanged, and each fails when a violation is planted in a moved module. The public contract is unchanged: for both packages, lib/index.d.ts and lib/index.d.mts export the same names (protocol 156, api 28) with the same kinds, types, members, declaration text and doc comments; lib/testing.* is byte for byte the same; and require() and import() of each build return the same keys, constant values, arities and class members. Each error class is still defined once per build, and none in lib/testing.*. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kiwidream
force-pushed
the
devex/module-split-pr
branch
from
October 30, 2026 17:11
26d4460 to
707e8e9
Compare
There was a problem hiding this comment.
Incomplete review: GitHub gave no textual diff for 2 file(s) not confirmed as non-text.
Findings without an inline location
Still open from earlier reviews
- [medium · pre-existing · security] Response bodies are read with no size limit, so a hostile backend can exhaust client memory:
packages/api/src/transport.ts:404, first reported at7e44de4 - [medium · pre-existing] Descriptor object check takes exponential time on a tree whose subtrees are shared (a DAG):
packages/protocol/src/descriptors.ts:693, first reported at7e44de4 - [low · pre-existing] The verified address-history scan fetches pages through de-duplicated GETs that can predate its stats read:
packages/api/src/esplora-client.ts:488, first reported at7e44de4 - [low · pre-existing] MempoolQbitClient's verified history also fetches pages via de-duplicated GETs that can predate its stats read:
packages/api/src/mempool-qbit-client.ts:159, first reported at26d4460 - [low · pre-existing] MempoolQbitClient.getAddressTxs compares the afterTxid cursor case-sensitively and rejects valid uppercase txids:
packages/api/src/mempool-qbit-client.ts:250, first reported at7e44de4 - [low · pre-existing] No check for duplicate input outpoints or empty outputs before signing, so key uses are spent on an invalid tx:
packages/protocol/src/signing.ts:575, first reported at26d4460 - [low · pre-existing] Multi-byte 0x01/0x08 input keys pass parsePsbt, and finalizeP2mrPsbt emits a second 0x08 record:
packages/protocol/src/psbt.ts:908, first reported at26d4460 - [low · nit] CONTRIBUTING still says the API docs come from TSDoc in each package's src/index.ts, now a barrel:
CONTRIBUTING.md:101, first reported at7e44de4 - [low · nit] After the split, many {@link} tags name symbols their module neither declares nor imports:
packages/protocol/src/errors.ts:7, first reported at26d4460
1 nit
10. [low · nit] Doc comments on the Esplora response types in the api barrel link to names the file only re-exports
packages/api/src/index.ts:45
packages/api/src/index.ts:45, 52, 105, 184-185 and 197 link to addressBalanceFromStats, EsploraClient.getAddressUtxos, feeTiersFromEstimates, FeeRateTier and MempoolQbitClient.getFeeEstimates. Lines 11-22 only re-export these names (export { X } from ...), which creates no local binding, so TypeScript scope (IDE hovers) does not resolve them. TypeDoc's fallback still does, so the docs check passes. This is the same problem as the errors.ts {@link} finding, but in a file that finding did not name.
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
Splits each package's single source file into one module per topic. This is a pure move with no behaviour or API change.
packages/protocol/src/index.ts(7,000 lines) becomes 17 modules, andpackages/api/src/index.ts(3,800 lines) becomes 8 modules plusraw-tx.ts. Eachindex.tsis now a barrel that re-exports the same names by name, so nothing becomes public by accident.errors,networks,constants,consensus,bytes,bech32,base58,addresses,scripts,message,transaction,sighash,psbt-fields,psbt,signing,descriptorsanduri.errors,raw-tx(the raw-transaction decoder, which imports only@noble/hashesand no error class),validation,adapters,transport,broadcast,esplora-clientandmempool-qbit-client.Read it with
git show --color-moved=dimmed-zebra. The commit message lists everything git doesn't mark as moved:index.ts;sighash.ts;Every declaration was moved from the current source, not from an earlier draft of this split, so no doc fix since then is reverted.
Testing
lib/index.d.tsandlib/index.d.mtsexport the same names (protocol 156, api 28), with the same kinds, types, members, declaration text and doc comments.require()andimport()return the same keys, constant values, arities and class members.lib/testing.*is byte for byte the same.lib/testing.*. The run-time imports form no cycle.pnpm verify(api 1,478 tests, protocol 1,460, examples 104),pnpm docs:api:check(113 links) andpnpm smoke:consumerpass.Stacked on #9 (the testing kit). Once #9 merges, this is rebased onto
release/0.1.1and retargeted.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.