Lib Fixes: Copy and Paste Bugs - #4222
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are at least two correctness issues in the modified code paths (LTC isValidSignature not passing the public key to getScriptCode, and bitcore-lib-cash calling DecodeBitfield without binding this in strict mode), plus a definite lint hazard from leading whitespace in bitcore-lib’s interpreter file.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR applies a set of bug fixes and lint/cleanup changes across the Bitcore family of libs, with a primary focus on preventing bitcore-lib-cash from incorrectly using SegWit-related script classification and tightening consistency in shared crypto/script code.
Changes:
- bitcore-lib-cash: Removes SegWit witness script-type handling from transaction input classification and performs interpreter cleanups/refactors.
- bitcore-lib (+ ltc/doge/cash variants): Normalizes hashing modules (
crypto→nodeCrypto) and addresses assorted lint issues (imports, semicolons, indentation, unreachable code). - bitcore-lib: Fixes Taproot output branch tracking / validity handling in
transaction/output.js.
File summaries
| File | Description |
|---|---|
| packages/bitcore-lib/lib/transaction/output.js | Fixes Taproot internal branch tracking and validity logic; simplifies node propagation. |
| packages/bitcore-lib/lib/transaction/input/publickeyhash.js | Import ordering and const/let cleanup in scriptCode/signature verification paths. |
| packages/bitcore-lib/lib/script/interpreter.js | Lint-related edits including flag constant punctuation; adds/keeps utility usage. |
| packages/bitcore-lib/lib/crypto/hash.js | Renames crypto require binding to nodeCrypto for clarity/shadowing avoidance. |
| packages/bitcore-lib-ltc/lib/transaction/input/publickeyhash.js | Const/let cleanup and import reordering in P2PKH input implementation. |
| packages/bitcore-lib-ltc/lib/script/interpreter.js | Lint-driven refactors (const/let, formatting, unreachable break removal). |
| packages/bitcore-lib-ltc/lib/networks.js | Removes unreachable bech32prefix assignment block and modernizes loops/const. |
| packages/bitcore-lib-ltc/lib/crypto/hash.js | Renames crypto binding and fixes missing semicolon; keeps scrypt dependency. |
| packages/bitcore-lib-doge/lib/transaction/input/publickeyhash.js | Import ordering and const/let cleanup. |
| packages/bitcore-lib-doge/lib/script/interpreter.js | Lint-driven refactors similar to LTC interpreter. |
| packages/bitcore-lib-doge/lib/crypto/hash.js | Renames crypto binding and fixes missing semicolon; keeps scrypt dependency. |
| packages/bitcore-lib-doge/lib/block/blockheader.js | Lint cleanup and clarifies PoW comment for Dogecoin; minor const/let usage. |
| packages/bitcore-lib-cash/lib/transaction/transaction.js | Removes SegWit witness script-type checks from input type selection (BCH correctness). |
| packages/bitcore-lib-cash/lib/script/interpreter.js | Extensive formatting/refactor/lint cleanup across interpreter paths, incl. introspection blocks. |
| packages/bitcore-lib-cash/lib/crypto/hash.js | Renames crypto require binding to nodeCrypto. |
| packages/bitcore-lib-cash/index.js | Fixes versionGuard error message to correctly reference bitcore-lib-cash. |
| packages/bitcore-build/wdio.conf.js | Increases mocha timeout and adds targeted eslint suppressions for build/test config. |
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const dummy = stacktop(-idxDummy); | ||
|
|
||
| let bitfieldObj = DecodeBitfield(dummy, nKeysCount); | ||
| const bitfieldObj = DecodeBitfield(dummy, nKeysCount); | ||
|
|
There was a problem hiding this comment.
You can fix by making
function DecodeBitField(dummy, size) {into
Interpreter.prototype._decodeBitfield = function(dummy, size) {and then...
const bitfieldObj = DecodeBitfield(dummy, nKeysCount);becomes
const bitfieldObj = this._decodeBitfield(dummy, nKeysCount);Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Description 📝
Bug fixes and linting for libs. The main bug was bitcore-lib-cash calling segwit functions causing errors. The others errors are sparse and documented in the Changelog.
Motivation
For #4194 I need to add utxos to a bitcore-lib-cash transaction using
associateInputs, but doing so throws an error because the inputs are checked for being a witness when BCH does not have SegWit. Ths PR removes those checks. I used Opus 5 to find similar errors copy and paste.Changelog 🗒️
isWitnessScriptHashOut/isWitnessPublicKeyHashOut) from transaction input classificationbitcore-libinstead ofbitcore-lib-cashSEQUENCE_LOCKTIME_DISABLE_FLAGreference (missingInterpreter.qualifier) in interpreterSCRIPT_VERIFY_CLEANSTACKflag definitionoutput.jsinternal branch tracking to use_branch/_isValidconsistently and fix validity checkbech32prefixassignment innetworks.jscryptovariable tonodeCryptoinhash.jsto stop shadowing the Node built-in globalbreakstatements,varredeclarations, andthis-aliasing in script interpretersTesting Notes 🥼
No significant functional changes so tests should catch everything
Checklist ✔️