Conversation
…dlock fixes
2.4.4 contains TWO self-deadlocks that stop proof fetching entirely. Both are the
same mistake: a StorageKnex method drops the caller's transaction token, so its
query asks the pool for a second connection while the caller's transaction holds
the first. knex forces {min:1,max:1} on sqlite, so it can never be granted and the
task dies after acquireConnectionTimeout with
KnexTimeoutError: Timeout acquiring a connection. The pool is probably full.
Are you missing a .transacting(trx) call?
1. getProvenOrRawTx(txid, trx) dropped trx on its findProvenTxs call.
Fixed upstream by ef710c3 (bsv-blockchain/ts-stack#426), published in 2.6.0.
2. readSettings() took no trx at all, so verifyReadyForDatabaseAccess deadlocked
on the same pool. Fixed by 4958767a5 (#444), published in 2.7.0.
TaskCheckForProofs therefore fails before processing any request, which is why an
affected wallet's proven_tx_reqs backlog sits at zero attempts -- the rows are
never reached rather than tried and failed.
Downstream this makes BEEF payment ancestry never terminate at a proven parent, so
x-bsv-payment headers grow without bound until a Cloudflare-fronted origin refuses
the request at its 32KB header cap -- after the wallet has already broadcast and
paid.
Observed on a real wallet running the released build: 2,532 monitor error events,
340 proof requests outstanding at 0 attempts, 433 unproven transactions. Applying
the equivalent fix moved proven_tx_reqs completed 18 -> 119, outstanding 139 -> 38,
and the payment header 33,058 -> 13,666 bytes, with a wallet Cloudflare had been
refusing paying again.
Verified by unpacking the published tarballs rather than trusting release notes:
version getProvenOrRawTx readSettings
2.4.4 drops trx no trx param
2.6.2 trx forwarded no trx param
2.10.1 trx forwarded trx forwarded
Landing 2.6.x would therefore have shipped only half the fix.
@bsv/sdk moves ^2.2.0 -> ^2.4.0 because it has to: wallet-toolbox declares
peerDependencies @bsv/sdk ^2.4.0 from 2.7.0 onwards, which is the same version that
carries the readSettings fix, so the two cannot be separated. Checked that every
other @bsv/* dependency accepts it -- amountinator, btms, btms-permission-module and
message-box-client all ask for ^2.1.6.
2.10.1 also carries 96e888612, which raises minimumDesiredUTXOValue off the historic
32 satoshis. That mismatch against the 96-satoshi dust floor is the other half of the
header-growth problem, so it lands here for free.
@bsv/wallet-toolbox-client is deliberately left at 2.4.4. Its newer releases are
bundled packages whose exports map blocks the deep '/out/src/...' paths this app
imports across six files. That is a separate migration and unrelated to this defect,
which lives in StorageKnex -- Electron main only.
NOTE FOR DEVELOPERS: this raises the effective minimum Node for a local install to
22. @bsv/sdk 2.4.0, @bsv/wallet-toolbox 2.10.1 and better-sqlite3 all declare
engines node >=22, and on Node 20 npm install now dies in the postinstall step
(electron-builder install-app-deps -> @electron/rebuild, itself already requiring
>=22.12.0 on master). Verified on Node 24.13.0.
npm run build passes (renderer tsc + vite, electron tsc). Tests: 15/15 default,
58/58 test:tokens, 32/32 test:stas:db including the four migration tests that
exercise StorageKnex over knex+sqlite.
There was a problem hiding this comment.
Pull request overview
This PR updates bsv-desktop’s core BSV stack dependencies to align the Electron wallet’s bundled toolchain with the official Sigma “official-wallet” version matrix, primarily by pinning @bsv/wallet-toolbox and its peer @bsv/sdk to specific versions and refreshing the lockfile.
Changes:
- Pin
@bsv/sdkto2.4.2and@bsv/wallet-toolboxto2.10.4inpackage.json. - Refresh
package-lock.jsonto reflect the pinned versions and updated transitive dependency graph.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| package.json | Pins the primary BSV SDK/toolbox dependencies to the specified versions. |
| package-lock.json | Updates the resolved dependency graph to match the new pinned versions (including updated transitive packages). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "@bsv/sdk": "2.4.2", | ||
| "@bsv/uhrp-react": "^1.0.6", | ||
| "@bsv/wallet-toolbox": "^2.4.4", | ||
| "@bsv/wallet-toolbox": "2.10.4", | ||
| "@bsv/wallet-toolbox-client": "^2.4.4", |
There was a problem hiding this comment.
Fixed in a25eb50: the legacy client is pinned exactly to 2.4.4 and the lockfile is refreshed.
|
@sirdeggen This is ready for upstream review. The independent audit's dependency blocker is fixed at |
| "engines": { | ||
| "node": ">=22" | ||
| }, |
There was a problem hiding this comment.
Fixed in eea1012: pull-request CI now runs only Node 22, matching the declared runtime floor and upgraded dependencies.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/push.yaml:15
- The workflow update to a Node 22-only matrix is a good time to also bump the GitHub Actions used here:
actions/checkout@v2and especiallyactions/setup-node@v1are very old and have been subject to deprecation/runtime transitions on GitHub-hosted runners. Updating them reduces the risk of CI suddenly breaking even though the repo is now pinned to a modern Node runtime.
node-version: [22.x]
steps:
- uses: actions/checkout@v2
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v1
|
Follow-up at |
a838ac3 to
6795328
Compare
|
Packaged-bundle smoke is now complete at |
Summary
Refreshes the dependency upgrade proposed in #78 on current v2.8.4 master and aligns the packaged candidate with the exact versions used by the merged Sigma official-wallet matrix.
@bsv/wallet-toolboxexactly to 2.10.4@bsv/sdkexactly to 2.4.2@bsv/wallet-toolbox-clientexactly to 2.4.4 because Desktop still uses its deep legacy exportstest:stas:dbpreserve a Vitest failure after restoring the Electron native buildVerification
npm test: 24/24 passednpm run test:tokens: 58/58 passednpm run test:stas:db: 32/32 passed; native Electron rebuild completednpm run build: passedgit diff --check: passedRemaining gates and known risks
npm install --package-lock-only --ignore-scriptsreports 39 existing dependency advisories (5 low, 8 moderate, 20 high, 6 critical). Reachability and remediation are outside this narrowly scoped version-alignment PR.vm-browserifyeval warnings remain unchanged.Supersedes #78.