#35429 wallet: avoid global access in external signer SPKM

ReviewabilityReviewsAgreementSize
#35429 w0xlt wallet: avoid global access in external signer SPKMAuthor silent 153d0Crickets44 + 11 tests
  • Improve wallet modularity by avoiding direct access to global state in external signer handling
  • Helps decouple the wallet engine from global arguments and chain parameters
  • Needs rebase
  • Author has been silent for 108 days
  • No review verdicts recorded
  • No reviews or substantive comments yet
  • Pinged author for a rebase (sedited)
  • 44 lines added or modified outside tests
  • 11 lines added or modified in tests
  • 27 lines removed in total
  • 5 files, 2 commits
Wallet#83 of 92
P4 · cleanup
  • P4 because it is an internal refactoring removing global state from wallet internals
  • Modularity benefit is modest with no direct user impact or stated dependencies
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

This PR refactors `ExternalSignerScriptPubKeyMan` to remove its direct calls to `gArgs` and `Params()`. The external signer command string and chain type are instead captured once in `CWallet::LoadWalletArgs`, stored on the `CWallet` instance, and passed into `ExternalSignerScriptPubKeyMan` during initialization.

Problem: `ExternalSignerScriptPubKeyMan::GetExternalSigner()` reached into global state (`gArgs` and `Params()`) deep inside wallet execution paths, hindering modularity and decoupling of the wallet engine.

Reviewability

Stale · Author silent 153d

No author activity for 153 days; a review may wait a long time for a response.

Waiting on (from the thread read):

  • author: rebase and response to whether the PR is still being worked on 2026-10-31
  • author: rebase and confirmation of whether PR is still being worked on 2026-10-31

Agreement

Crickets

no substantive comment either way

Thread notes: Reviewed statements under the updated rules; both comments by sedited are process remarks rather than claims about the change. / No substantive review or claims raised; only process remarks and pings for rebase from sedited.

Objections, suggestions, and questions: none recorded.

Participants:

  • sedited (member, neutral, 2 statements 2026-09-17 to 2026-10-31): pinged author for rebase and asked if still working on the PR

Files

11 lines under test/bench/ci.

  • src/wallet/external_signer_scriptpubkeyman.h +22/-13
  • src/wallet/external_signer_scriptpubkeyman.cpp +8/-10
  • src/wallet/wallet.cpp +9/-4
  • src/wallet/test/scriptpubkeyman_tests.cpp +11/-0
  • src/wallet/wallet.h +5/-0

Card

The short text the ranking pass compares across PRs.

This PR refactors ExternalSignerScriptPubKeyMan to take the external signer command and chain identifier as parameters from CWallet rather than reading gArgs and Params() directly. It is an internal architectural cleanup that eliminates global state access from the wallet's external signer manager with no behavior change. The PR has had no reviews, is currently conflicted with master, and the author has been inactive for over three months.

Data

dossier JSON · extract JSON · model ledger, generated 2026-11-01T01:16, confidence high, input hash fa8ccfee18f5647b
thread read through 2026-10-31 (2 statements), head c0dc4d5091
labels: Wallet, Needs rebase

Categories

Wallet

P4 · cleanup

Cleans up internal wallet architecture by avoiding direct calls to gArgs and Params() in ExternalSignerScriptPubKeyMan. While helpful for modularity and removing global state, it is an internal refactoring without direct user impact or stated dependencies on other high-priority work.

Membership: Matches the 'Wallet' label and modifies wallet SPKM and wallet files under src/wallet/.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 1