fix(bitcoindservice.ts and bitcoindservice.spec.ts): setting up defau… by TomLucasakaTGeek · Pull Request #1360 · jamaljsr/polar

TomLucasakaTGeek · 2026-02-08T09:57:45Z

Description

The Problem

By default, when multiple wallets are loaded in a Bitcoin Core node, RPC calls require an explicit wallet context. Polar was previously making calls to the base RPC endpoint. When a user connected an external tool (like Sparrow Wallet) that created additional wallets, Bitcoin Core would return an error:

The Fix
I updated the BitcoindService to ensure all RPC interactions are explicitly directed to the default Polar wallet, even if other wallets are present on the node.

Explicit Wallet Targeting: Modified the RPC client configuration to append wallet/ to the base URL path.

Default Wallet Fallback: Ensured that commands are executed against the default wallet ("" or the primary loaded wallet) by utilizing the -rpcwallet equivalent in the service layer.

Technical Changes
src/lib/bitcoin/bitcoindService.ts: Updated the RPC request logic to include the wallet name in the request path.

Verification Results

<--
Manual Testing: Verified that Polar remains connected and functional even after creating multiple wallets via bitcoin-cli and Sparrow Wallet on a Regtest node.
-->

Linting: yarn lint:all passes successfully.

Tests: All unit tests pass.

Steps to Test

Download Sparrow Wallet
Open Sparrow Wallet in regtest mode by opening it using "Sparrow.exe -n regtest" command
Generate keys and connect wallet to Polar by selecting "Bitcoin Core" as Polar acts as a Core Network
Provide values of credentials from the Polar Network and connect it.
The issue is resolved.

Screenshots

…lt wallet value configuration The issue in Polar was that it created a new wallet everytime an external wallet (eg Sparrow Wallet) was connected and to fix the problem, we setup a default value , for it to pick to resolve comflicts fix jamaljsr#1096

TomLucasakaTGeek · 2026-02-08T09:58:26Z

@jamaljsr I have raised a new PR.

greptile-apps · 2026-02-08T10:01:00Z

Greptile Overview

Greptile Summary

This PR updates BitcoindService to always construct the bitcoin-core client with wallet: '', and adds a unit test asserting that option is passed.

The intent is to avoid Bitcoin Core multi-wallet errors by forcing wallet-scoped RPC calls to use a specific wallet context. However, hard-coding wallet: '' targets the empty-name wallet endpoint (/wallet/) and will fail on nodes where the empty-name wallet is not loaded/created (a common case when other wallets exist). This also doesn’t align with createDefaultWallet(), which only creates the empty wallet when no wallets are loaded.

Confidence Score: 2/5

This PR is not safe to merge as-is because it can break wallet-scoped RPC calls on nodes without an empty-name wallet loaded.
The core change hard-codes wallet: '', which forces all RPCs onto the empty-name wallet endpoint and will error when that wallet doesn’t exist (especially when other wallets are present). The added test only asserts the option is set and doesn’t validate the intended multi-wallet behavior.
src/lib/bitcoin/bitcoind/bitcoindService.ts, src/lib/bitcoin/bitcoind/bitcoindService.spec.ts

Important Files Changed

Filename	Overview
src/lib/bitcoin/bitcoind/bitcoindService.ts	Sets `BitcoinCore` client `wallet: ''` for all RPC calls; this can break wallet-scoped RPCs when the empty-name wallet is not loaded/created, and it doesn’t actually select an existing default wallet when multiple wallets are present.
src/lib/bitcoin/bitcoind/bitcoindService.spec.ts	Adds a unit test asserting the client is constructed with `wallet: ''`; however this assertion doesn’t validate the multi-wallet fix and codifies behavior that can fail if the empty-name wallet doesn’t exist.

Sequence Diagram

sequenceDiagram
  participant App as Polar
  participant Svc as BitcoindService
  participant RPC as BitcoinCore RPC client
  participant Node as bitcoind

  App->>Svc: createDefaultWallet(node)
  Svc->>RPC: new BitcoinCore({ host, user, pass, wallet: '' })
  RPC->>Node: POST /wallet/ (listwallets)
  Node-->>RPC: [wallet names]
  alt no wallets loaded
    Svc->>RPC: createWallet('')
    RPC->>Node: POST /wallet/ (createwallet "")
  end

  App->>Svc: getWalletInfo/sendFunds/etc
  Svc->>RPC: new BitcoinCore({ ..., wallet: '' })
  RPC->>Node: POST /wallet/ (wallet-scoped RPC)
  alt empty-name wallet exists
    Node-->>RPC: success
  else empty-name wallet missing
    Node-->>RPC: error "Wallet file not found"/404
  end

greptile-apps

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-08T10:01:07Z

  });

+  // Adding Test case for selecting default wallet and not create a new one
+  it('should initialize the client with an explicit empty wallet path', () => {


Test asserts wrong behavior

This test only verifies that BitcoinCore was constructed with wallet: '', but it doesn’t prove the reported multi-wallet issue is fixed (and it bakes in behavior that can be incorrect if the empty-name wallet doesn’t exist). As written, it will still pass even if wallet-scoped RPCs fail at runtime due to missing "" wallet. A more reliable test would mock listWallets() to return non-empty wallets and assert the service targets an actually-existing wallet, or assert that createDefaultWallet() is invoked/ensures the wallet exists before calling wallet-scoped RPCs.

greptile-apps · 2026-02-08T10:01:09Z

Additional Comments (1)

src/lib/bitcoin/bitcoind/bitcoindService.ts
Incorrect wallet selection

Setting wallet: '' does not target the “default”/currently loaded wallet when multiple wallets exist; it explicitly targets the empty-name wallet endpoint (i.e. .../wallet/). If a user’s node doesn’t have an empty-name wallet loaded/created, wallet-scoped RPCs (getwalletinfo, getbalances, sendtoaddress, etc.) will fail with “wallet not found”/404. This change also contradicts createDefaultWallet() which only creates the empty wallet when listWallets() returns [] (not when other wallets exist). Consider selecting an existing wallet (e.g. first from listWallets()) or ensuring the empty wallet exists regardless of other loaded wallets.

codecov · 2026-02-08T13:14:15Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (711755f) to head (1dae2fe).

Additional details and impacted files

@@            Coverage Diff            @@
##            master     #1360   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          211       211           
  Lines         7014      7014           
  Branches      1398      1348   -50     
=========================================
  Hits          7014      7014

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jamaljsr · 2026-02-08T14:05:06Z

This PR doesn't resolve the issue for me. I still get the multiple wallet error after opening Sparrow.

Here's what I did

Start with Sparrow closed
In Polar, create a new network with just a bitcoind node
Start the network and mine a block to confirm Polar can talk to bitcoind
Ran bitcoin-cli listwallets on the node to confirm only one wallet is present
Open sparrow wallet and connect to the Polar bitcoind node
Ran bitcoin-cli listwallets on the node to confirm there are two wallets present
Try to mine a block in Polar. The error is shown

TomLucasakaTGeek · 2026-02-08T19:52:37Z

oh no, lemme check

greptile-apps Bot reviewed Feb 8, 2026

View reviewed changes

jamaljsr mentioned this pull request Feb 8, 2026

Default wallet rpc polar patch1 #1356

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(bitcoindservice.ts and bitcoindservice.spec.ts): setting up defau…#1360

fix(bitcoindservice.ts and bitcoindservice.spec.ts): setting up defau…#1360
TomLucasakaTGeek wants to merge 1 commit into
jamaljsr:masterfrom
TomLucasakaTGeek:default-wallet-config

TomLucasakaTGeek commented Feb 8, 2026

Uh oh!

TomLucasakaTGeek commented Feb 8, 2026

Uh oh!

greptile-apps Bot commented Feb 8, 2026

Important Files Changed

Uh oh!

greptile-apps Bot left a comment

Uh oh!

greptile-apps Bot Feb 8, 2026

Uh oh!

greptile-apps Bot commented Feb 8, 2026

Uh oh!

codecov Bot commented Feb 8, 2026

Uh oh!

jamaljsr commented Feb 8, 2026

Uh oh!

TomLucasakaTGeek commented Feb 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

TomLucasakaTGeek commented Feb 8, 2026

Description

The Problem

Verification Results

Steps to Test

Screenshots

Uh oh!

TomLucasakaTGeek commented Feb 8, 2026

Uh oh!

greptile-apps Bot commented Feb 8, 2026

Greptile Overview

Greptile Summary

Confidence Score: 2/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Feb 8, 2026

Uh oh!

codecov Bot commented Feb 8, 2026

Codecov Report

Uh oh!

jamaljsr commented Feb 8, 2026

Uh oh!

TomLucasakaTGeek commented Feb 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants