#35733 sign: Remove FillableSigningProvider

• Remove outdated signing abstractions left over from the legacy wallet
• Confine legacy signing logic strictly to migration code paths and clean up tool signing interfaces

Removes the `FillableSigningProvider` class across the codebase, inlining its remaining functionality into `LegacyDataSPKM` where it is still needed for legacy wallet migration. Call sites in tests and `bitcoin-tx` are converted to use `FlatSigningProvider` with explicit key and script population. A small safeguard is also added to `SignStep` to avoid signing `SCRIPTHASH` redeem scripts exceeding 520 bytes.

Problem: Following the removal of the legacy wallet, `FillableSigningProvider` remained as an unnecessary and confusing abstraction in the signing subsystem.

P4 · cleanup

• P3 because it modernizes bitcoin-tx signing to use standard flat keystores
• Ensures correct script size constraints are enforced when signing transactions

P4 because the change to bitcoin-tx is a straightforward refactor to replace the deprecated keystore class without introducing new user-facing functionality.

Membership: Modifies src/bitcoin-tx.cpp to use FlatSigningProvider instead of FillableSigningProvider and explicitly adds P2WPKH scripts.

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

P4 · cleanup

• P3 because it consolidates legacy keystore logic directly into migration handlers
• Removes legacy wallet baggage from general signing interfaces as an architectural cleanup

P4 because this is internal code cleanup that removes an obsolete signing provider abstraction leftover from legacy wallet code. It provides no direct user-facing benefit or fund safety change, though it helps isolate legacy migration logic.

Membership: Inlines FillableSigningProvider directly into LegacyDataSPKM in src/wallet/scriptpubkeyman.{cpp,h}.

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

• Ready for review; clean PR with feedback incorporated and passing CI

The PR has clean CI, is up to date, and the author addressed all reviewer comments in recent updates.

Author status: active, addressed reviewer comments and force-pushed updates

Resolved concerns:

• jeanpablojp noted that removing the 520-byte check allowed signing P2SH redeem scripts that produce consensus-invalid transactions; achow101 added an explicit size check in SignStep
• jeanpablojp pointed out a fuzz accounting mismatch in spkm_migration when LoadCScript rejected scripts over 520 bytes; fixed by checking HaveCScript
• jeanpablojp noted fuzz/script_sign lost P2WPKH script resolution previously provided implicitly; fixed by explicitly populating the FlatSigningProvider

• Broad support for isolating legacy wallet codepaths and modernizing signing providers
• Concept approval because it containerizes legacy wallet code paths (rkrux)
• Approach approval after verifying tests and script size limits (jeanpablojp)
• Verified by testing bitcoin-tx commands (vicjuma)

Strong consensus to remove the class, all reviewer objections regarding script limits and fuzz tests resolved

Maintainer rkrux gave Concept ACK citing the architectural benefit of containerizing legacy wallet logic, vicjuma tested and ACKed the bitcoin-tx changes, and jeanpablojp's technical findings were fully addressed in subsequent commits.

• 2026-08-10 vicjuma: 'ACK for the bitcoin-tx changes. Will try to review the remaining parts'
• 2026-08-25 jeanpablojp: 'Approach ACK. Built and ran the tests.'
• 2026-08-26 rkrux: 'Concept ACK... to containerise the codepath related to legacy wallets while discouraging usage of this seemingly outdated signing provider.'

Objections:

Support:

• vicjuma: Verified key and script paths in bitcoin-tx with test cases
• rkrux: Containerizes the legacy wallet migration codepath and removes outdated signing provider
• jeanpablojp: Approach ACK after building and running tests [not substantive]

Participants: vicjuma (support), jeanpablojp (objection), rkrux (support)

State derived from the lists: substantive support, no open objection (vicjuma, rkrux)

Review verdicts (DrahtBot): 0

• Approach ACK: jeanpablojp
• Concept ACK: vicjuma, rkrux

191 lines under test/bench/ci.

• src/script/signingprovider.cpp +0/-109
• src/script/signingprovider.h +0/-74
• src/wallet/scriptpubkeyman.cpp +62/-10
• src/test/transaction_tests.cpp +29/-23
• src/test/fuzz/key.cpp +14/-20
• src/test/script_p2sh_tests.cpp +16/-13
• src/bitcoin-tx.cpp +10/-4
• src/test/script_tests.cpp +7/-6
• src/wallet/scriptpubkeyman.h +10/-2
• src/wallet/test/wallet_tests.cpp +7/-5
• src/test/fuzz/script_sign.cpp +8/-2
• src/test/txvalidationcache_tests.cpp +4/-3
• src/wallet/test/fuzz/scriptpubkeyman.cpp +4/-2
• src/test/orphanage_tests.cpp +3/-2
• src/test/util/setup_common.cpp +3/-2
• src/wallet/test/spend_tests.cpp +3/-2
• src/test/multisig_tests.cpp +2/-2
• src/test/util/transaction_utils.cpp +2/-2
• doc/developer-notes.md +1/-2
• src/script/sign.cpp +3/-0
• src/test/util/transaction_utils.h +1/-2
• src/bench/ccoins_caching.cpp +1/-1

Removes the obsolete FillableSigningProvider class left over from legacy wallet architecture and inlines its remaining logic into LegacyDataSPKM for legacy wallet migration. Bitcoin-tx and test call sites are updated to use FlatSigningProvider. While an internal cleanup with low strategic urgency, it carries solid support from reviewers and the author has resolved technical concerns regarding 520-byte P2SH script limits.