wallet: allow importprunedfunds for spending transactions #34371

8144225309 commented at 3:54 PM on January 21, 2026: none

importprunedfunds only allowed importing transactions that credit the wallet (checked via IsMine), rejecting transactions that spend from it. This could leave a pruned wallet showing an incorrect balance: if a spending transaction was removed with removeprunedfunds, it couldn't be re-imported and would fail with "No addresses in wallet correspond to included transaction".

This routes the import through AddToWalletIfInvolvingMe(), which already checks both IsMine() and IsFromMe(), so spending transactions are accepted and the involvement check is no longer duplicated in importprunedfunds. A functional test imports a transaction that spends from the wallet but has no outputs to it (entire UTXO sent externally).

Fixes #21647

DrahtBot added the label Wallet on Jan 21, 2026

DrahtBot commented at 3:54 PM on January 21, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34371.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Stale ACK	achow101, Bicaru20

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35501 (wallet: store all witness variants of a transaction by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

in test/functional/wallet_importprunedfunds.py:124 in af80d2c367

 116 | @@ -117,6 +117,20 @@ def run_test(self):
 117 |          w1.removeprunedfunds(txnid3)
 118 |          assert not [tx for tx in w1.listtransactions() if tx['txid'] == txnid3]
 119 |  
 120 | +        # Import a spending transaction with no wallet outputs (issue #21647)
 121 | +        self.nodes[0].sendtoaddress(address3, 0.1)
 122 | +        self.generate(self.nodes[0], 1)
 123 | +        external_addr = self.nodes[0].getnewaddress()
 124 | +        spend_txid = w1.sendtoaddress(address=external_addr, amount=0.1, subtractfeefromamount=True)

achow101 commented at 11:11 PM on January 26, 2026:

Let's not use subtractfeefromamount. It's also better if we explicitly specify the UTXO to use here rather than hoping that SFFO will pick the right one.

        txid = self.nodes[0].sendtoaddress(address3, 0.1)
        vout = find_vout_for_address(self.nodes[0], txid, address3)
        self.generate(self.nodes[0], 1)
        external_addr = self.nodes[0].getnewaddress()
        spend_txid = w1.sendall(recipients=[self.nodes[0].getnewaddress()], inputs=[{"txid": txid, "vout": vout}])["txid"]

8144225309 commented at 3:36 PM on January 27, 2026:

Ah! Thank you, testing the fixed version now. edit: changes applied

8144225309 force-pushed on Jan 27, 2026

achow101 commented at 10:38 PM on March 31, 2026: member

ACK 13b880a5aa37648e8ed3f5ab0459d3c9784b006e

Bicaru20 commented at 10:20 PM on April 16, 2026: none

ACK 13b880a5aa37648e8ed3f5ab0459d3c9784b006e The PR does what it is intended. It adds the IsFromMe check so importprunedfunds also allows importing transactions that spend the outputs that are form the wallet. That way the UTXO is marked spent and the balance is correct since one outputs was spend.

I just have one little question, is it worth it to add a test case where a transaction that has nothing to do with the wallet is imported? In this case the message No addresses in wallet correspond to included transaction should be thrown. Just asking just in case this makes the test more complete.

8144225309 commented at 7:29 AM on April 17, 2026: none

Thanks for the ACK! That case is already covered at line 83 — assert_raises_rpc_error(-5, "No addresses", ...) imports a transaction unrelated to the wallet and verifies the rejection.

in src/wallet/rpc/backup.cpp:88 in 13b880a5aa outdated

  81 | @@ -82,7 +82,7 @@ RPCHelpMan importprunedfunds()
  82 |      unsigned int txnIndex = vIndex[it - vMatch.begin()];
  83 |  
  84 |      CTransactionRef tx_ref = MakeTransactionRef(tx);
  85 | -    if (pwallet->IsMine(*tx_ref)) {
  86 | +    if (pwallet->IsMine(*tx_ref) || pwallet->IsFromMe(*tx_ref)) {
  87 |          pwallet->AddToWallet(std::move(tx_ref), TxStateConfirmed{merkleBlock.header.GetHash(), height, static_cast<int>(txnIndex)});
  88 |          return UniValue::VNULL;
  89 |      }

davidgumberg commented at 5:42 PM on May 20, 2026:

(I'm also OK with merging as-is since I think author may be gone...)

Shouldn't this logic live outside of the RPC, as in:

    const CTransactionRef tx_ref = MakeTransactionRef(tx);
    auto tx_state = TxStateConfirmed{merkleBlock.header.GetHash(), height, static_cast<int>(txnIndex)};
    if (pwallet->AddToWalletIfInvolvingMe(tx_ref, tx_state, /*fUpdate=*/false, /*rescanning_old_block=*/false)) {
            return UniValue::VNULL;
    }

8144225309 commented at 10:39 PM on May 22, 2026: none

I'm here, looked into this further. The entry point that other wallet sync paths use (mempool, block connection, rescan) is SyncTransaction, which wraps AddToWalletIfInvolvingMe and also calls MarkInputsDirty. Both sit in the private section of CWallet, so external callers need a small public wrapper following the same pattern as transactionAddedToMempool/blockConnected:

// wallet.h (public section):
bool AddImportedTransaction(const CTransactionRef& tx, const SyncTxState& state);

// wallet.cpp:
bool CWallet::AddImportedTransaction(const CTransactionRef& tx, const SyncTxState& state)
{
    LOCK(cs_wallet);
    return SyncTransaction(tx, state);
}

Ran it against synced and pruned (-prune=550) scenarios. No observable difference in the cases I could isolate: AddWalletDescriptor (wallet.cpp:3779) already populates the address book for non-ranged descriptors, and chain notifications cover the rest. (Ranged descriptor + pruning might be the case where the refactor's side effects do appear, but I couldn't construct a clean test for it.)

Could do this as a focused follow-up for architectural consistency (importprunedfunds is the only wallet RPC that doesn't go through this helper), though I couldn't find a user-visible payoff in the cases I tested. Keeping this PR scoped to the bug fix; happy to file the follow-up if the cleanup is worth it on its own.

sedited commented at 8:22 PM on May 28, 2026: contributor

@davidgumberg can you respond to the latest comment here?

davidgumberg commented at 5:35 PM on June 10, 2026: contributor

Both sit in the private section of CWallet, so external callers need a small public wrapper following the same pattern as transactionAddedToMempool/blockConnected:

Adding a public wrapper that calls SyncTransaction() seems a bit silly instead of making SyncTransaction() or AddToWalletIfInvolvingMe() public.

Could do this as a focused follow-up for architectural consistency (importprunedfunds is the only wallet RPC that doesn't go through this helper), though I couldn't find a user-visible payoff in the cases I tested. Keeping this PR scoped to the bug fix; happy to file the follow-up if the cleanup is worth it on its own.

I think it's a pretty trivial change, and avoids putting wallet business logic inside of RPC code, which is the root cause of #21647 in my opinion.

wallet: allow importprunedfunds for spending transactions

importprunedfunds only accepted transactions that pay the wallet: it
checked IsMine and added the transaction directly, so a transaction that
only spends the wallet's outputs was rejected (#21647).

Route the import through AddToWalletIfInvolvingMe, which already checks
both IsMine and IsFromMe, so spending transactions are accepted without
duplicating the involvement check in the RPC.

Fixes #21647

72186b7697

8144225309 force-pushed on Jun 23, 2026

8144225309 commented at 10:12 PM on June 23, 2026: none

importprunedfunds now uses AddToWalletIfInvolvingMe (made public) instead of duplicating the involvement check, as suggested. fUpdate was removed in master (#35123), so it's the 3-arg call. Force pushed; range-diff against 13b880a5:

<details> <summary>range-diff</summary>

1:  13b880a5aa ! 1:  72186b7697 wallet: allow importprunedfunds for spending transactions
    @@ Metadata
      ## Commit message ##
         wallet: allow importprunedfunds for spending transactions
     
    -    importprunedfunds only accepted receiving transactions (via IsMine).
    -    Add IsFromMe to also accept spending transactions.
    +    importprunedfunds only accepted transactions that pay the wallet: it
    +    checked IsMine and added the transaction directly, so a transaction that
    +    only spends the wallet's outputs was rejected (#21647).
     
    -    Test covers a transaction with wallet inputs but no wallet outputs.
    +    Route the import through AddToWalletIfInvolvingMe, which already checks
    +    both IsMine and IsFromMe, so spending transactions are accepted without
    +    duplicating the involvement check in the RPC.
     
         Fixes [#21647](/bitcoin-bitcoin/21647/)
     
      ## src/wallet/rpc/backup.cpp ##
    -@@ src/wallet/rpc/backup.cpp: RPCHelpMan importprunedfunds()
    +@@ src/wallet/rpc/backup.cpp: RPCMethod importprunedfunds()
    + 
          unsigned int txnIndex = vIndex[it - vMatch.begin()];
      
    -     CTransactionRef tx_ref = MakeTransactionRef(tx);
    +-    CTransactionRef tx_ref = MakeTransactionRef(tx);
     -    if (pwallet->IsMine(*tx_ref)) {
    -+    if (pwallet->IsMine(*tx_ref) || pwallet->IsFromMe(*tx_ref)) {
    -         pwallet->AddToWallet(std::move(tx_ref), TxStateConfirmed{merkleBlock.header.GetHash(), height, static_cast<int>(txnIndex)});
    +-        pwallet->AddToWallet(std::move(tx_ref), TxStateConfirmed{merkleBlock.header.GetHash(), height, static_cast<int>(txnIndex)});
    ++    const CTransactionRef tx_ref = MakeTransactionRef(tx);
    ++    auto tx_state = TxStateConfirmed{merkleBlock.header.GetHash(), height, static_cast<int>(txnIndex)};
    ++    if (pwallet->AddToWalletIfInvolvingMe(tx_ref, tx_state, /*rescanning_old_block=*/false)) {
              return UniValue::VNULL;
          }
    + 
    +
    + ## src/wallet/wallet.h ##
    +@@ src/wallet/wallet.h: private:
    +     void AddToSpends(const COutPoint& outpoint, const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
    +     void AddToSpends(const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
    + 
    +-    /**
    +-     * Add a transaction to the wallet, or update it.  confirm.block_* should
    +-     * be set when the transaction was known to be included in a block.  When
    +-     * block_hash.IsNull(), then wallet state is not updated in AddToWallet, but
    +-     * notifications happen and cached balances are marked dirty.
    +-     *
    +-     * TODO: One exception to this is that the abandoned state is cleared under the
    +-     * assumption that any further notification of a transaction that was considered
    +-     * abandoned is an indication that it is not safe to be considered abandoned.
    +-     * Abandoned state should probably be more carefully tracked via different
    +-     * chain notifications or by checking mempool presence when necessary.
    +-     *
    +-     * Should be called with rescanning_old_block set to true, if the transaction is
    +-     * not discovered in real time, but during a rescan of old blocks.
    +-     */
    +-    bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const SyncTxState& state, bool rescanning_old_block) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
    +-
    +     /** Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */
    +     void MarkConflicted(const uint256& hashBlock, int conflicting_height, const Txid& hashTx);
    + 
    +@@ src/wallet/wallet.h: public:
    +      * [@return](/bitcoin-bitcoin/contributor/return/) the recently added wtx pointer or nullptr if there was a db write error.
    +      */
    +     CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool rescanning_old_block = false);
    ++
    ++    /**
    ++     * Add a transaction to the wallet, or update it.  confirm.block_* should
    ++     * be set when the transaction was known to be included in a block.  When
    ++     * block_hash.IsNull(), then wallet state is not updated in AddToWallet, but
    ++     * notifications happen and cached balances are marked dirty.
    ++     *
    ++     * TODO: One exception to this is that the abandoned state is cleared under the
    ++     * assumption that any further notification of a transaction that was considered
    ++     * abandoned is an indication that it is not safe to be considered abandoned.
    ++     * Abandoned state should probably be more carefully tracked via different
    ++     * chain notifications or by checking mempool presence when necessary.
    ++     *
    ++     * Should be called with rescanning_old_block set to true, if the transaction is
    ++     * not discovered in real time, but during a rescan of old blocks.
    ++     */
    ++    bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const SyncTxState& state, bool rescanning_old_block) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
    +     bool LoadToWallet(const Txid& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
    +     void transactionAddedToMempool(const CTransactionRef& tx) override;
    +     void blockConnected(const kernel::ChainstateRole& role, const interfaces::BlockInfo& block) override;
     
      ## test/functional/wallet_importprunedfunds.py ##
    -@@ test/functional/wallet_importprunedfunds.py: from test_framework.test_framework import BitcoinTestFramework
    - from test_framework.util import (
    +@@ test/functional/wallet_importprunedfunds.py: from test_framework.util import (
          assert_equal,
    +     assert_not_equal,
          assert_raises_rpc_error,
     +    find_vout_for_address,
          wallet_importprivkey,
    @@ test/functional/wallet_importprunedfunds.py: from test_framework.test_framework
      from test_framework.wallet_util import generate_keypair
     @@ test/functional/wallet_importprunedfunds.py: class ImportPrunedFundsTest(BitcoinTestFramework):
              w1.removeprunedfunds(txnid3)
    -         assert not [tx for tx in w1.listtransactions() if tx['txid'] == txnid3]
    +         assert txnid3 not in [tx['txid'] for tx in w1.listtransactions()]
      
     +        # Import a spending transaction with no wallet outputs (issue [#21647](/bitcoin-bitcoin/21647/))
     +        txid = self.nodes[0].sendtoaddress(address3, 0.1)

</details>