Add bitcoin-regtest-up package#8827
Conversation
ulissesferreira
temporarily deployed
to
default-branch
GitHub Actions
Inactive
|
Code sync follow-up:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| process.kill(process.pid, result.signal); | ||
| } | ||
|
|
||
| process.exit(result.status ?? 0); |
There was a problem hiding this comment.
Wrapper exits zero when child is killed by signal
Low Severity
When the child process is killed by a signal, spawnSync returns status: null and signal: '<name>'. The expression result.status ?? 0 evaluates to 0, so process.exit(0) runs, making signal-based termination appear as a clean successful exit. The preceding process.kill(process.pid, result.signal) is ineffective because Node.js delivers signals asynchronously via the event loop, and process.exit(0) runs synchronously before the signal handler fires. Parent processes and CI scripts checking the exit code would incorrectly see success.
Reviewed by Cursor Bugbot for commit ab33647. Configure here.
| } | ||
|
|
||
| process.exit(result.status ?? 0); | ||
| `, |
There was a problem hiding this comment.
Generated wrapper uses CJS in ESM-incompatible way
Medium Severity
The generated node_modules/.bin/bitcoind and bitcoin-cli wrapper scripts use require('node:child_process'), but these extensionless files are written without any module-type hint. In consuming projects whose package.json declares "type": "module", Node.js resolves these wrappers as ESM (since there's no .cjs extension and the nearest package.json governs), causing require to be undefined at runtime. The foundryup sibling package avoids this by symlinking directly to extracted binaries instead of generating script wrappers.
Reviewed by Cursor Bugbot for commit bbe4c1f. Configure here.
|
@metamaskbot publish-preview |
|
@metamaskbot publish-preview |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 82e0808. Configure here.
| } | ||
|
|
||
| return options; | ||
| } |
There was a problem hiding this comment.
Missing error handling crashes CLI without package.json
Medium Severity
readBitcoinRegtestInstallOptionsFromPackageJson calls readFileSync on package.json without any try-catch, so it throws an unhelpful ENOENT error if no package.json exists in the working directory. Since package config is optional (the function reads optional overrides), it should return empty options {} when the file is missing, matching how getBitcoinRegtestCacheDirectory gracefully handles a missing .yarnrc.yml. The CLI calls this function unconditionally on both the install and cache clean paths, so any invocation from a directory without package.json crashes.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 82e0808. Configure here.
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
1 participant
Description
Adds
@metamask/bitcoin-regtest-upto the core monorepo using the generated package structure, then replaces the scaffold with the existing Bitcoin Core regtest runtime installer implementation.Ownership follows the runtime tooling/platform pattern and includes Networks, matching the Tron Snap ownership direction.
Changes
packages/bitcoin-regtest-up.bitcoin-regtest-upCLI bin fromdist/bin/bitcoin-regtest-up.mjs.teams.jsonownership.30.2and updates the migrated README to match.Verification
yarn workspace @metamask/bitcoin-regtest-up run buildyarn workspace @metamask/bitcoin-regtest-up run testyarn eslint packages/bitcoin-regtest-upyarn constraintsyarn lint:misc --check packages/bitcoin-regtest-up/package.json packages/bitcoin-regtest-up/README.md packages/bitcoin-regtest-up/src/index.ts packages/bitcoin-regtest-up/src/install.ts packages/bitcoin-regtest-up/src/install.test.ts packages/bitcoin-regtest-up/src/bin/bitcoin-regtest-up.ts packages/bitcoin-regtest-up/jest.config.js .github/CODEOWNERS teams.json README.md tsconfig.json tsconfig.build.jsonyarn readme-content:checkyarn lint:teamsyarn workspace @metamask/bitcoin-regtest-up run changelog:validatenode packages/bitcoin-regtest-up/dist/bin/bitcoin-regtest-up.mjs --helpgit diff --checkNote
Medium Risk
Adds a new CLI that downloads and executes external Bitcoin Core binaries and writes to local/global caches, so failures or misuse could impact CI/dev environments despite being isolated to a new package.
Overview
Adds new
@metamask/bitcoin-regtest-uppackage that installs a pinned Bitcoin Core (30.2) release for regtest by downloading + checksum-verifying archives into the MetaMask cache and generatingnode_modules/.binwrappers forbitcoind/bitcoin-cli, plus acache cleancommand.Wires the package into the monorepo (workspace, TS project refs, README package list/graph) and sets ownership via
CODEOWNERS/teams.json, with Jest coverage config, docs config, licenses, changelog, and installer unit tests covering cache hits/misses and wrapper generation.Reviewed by Cursor Bugbot for commit 82e0808. Bugbot is set up for automated code reviews on this repo. Configure here.