wallet: fix persistent coin lock upgrade #36313

ArvinFarrelP commented at 2:27 AM on September 22, 2026: contributor

Fix a stale persistent coin lock when a temporary coin lock is upgraded to persistent.

Previously, upgrading an existing non-persistent lock to a persistent lock used emplace() without updating the existing value. As a result, the in-memory lock remained non-persistent.

When the coin was later spent, UnlockCoin() therefore did not erase the persistent LOCKED_UTXO database record. After restarting the wallet, the stale lock was loaded again.

This change updates the existing lock state when the coin is already present.

Adds a functional test covering:

temporary coin lock
upgrade to persistent lock
spending the locked coin
wallet reload
verification that the spent lock is not restored

Fixes #36304

wallet: fix persistent coin lock upgrade 8efd73f405

DrahtBot added the label Wallet on Sep 22, 2026

DrahtBot commented at 2:28 AM on September 22, 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/36313.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	vicjuma

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

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

lockunspent(False, [unspent_0], True) in test/functional/wallet_basic.py

in src/wallet/wallet.cpp:2537 in 8efd73f405

2532 | @@ -2533,7 +2533,10 @@ util::Result<void> CWallet::DisplayAddress(const CTxDestination& dest)
2533 |  void CWallet::LoadLockedCoin(const COutPoint& coin, bool persistent)
2534 |  {
2535 |      AssertLockHeld(cs_wallet);
2536 | -    m_locked_coins.emplace(coin, persistent);
2537 | +    auto [it, inserted] = m_locked_coins.emplace(coin, persistent);
2538 | +    if (!inserted) {

vicjuma commented at 1:11 PM on September 23, 2026:

Testing

Your solution works as expected

Before:

ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli -regtest lockunspent false '[{"txid":"'"56e8518c816e3e0f4f7c3320bb7fc8cdebc4997ff8bb46777a2e2955283dd9a5"'","vout":0}]' true
true
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$  RAW=$(./bitcoin-cli -regtest createrawtransaction \
  '[{"txid":"'"56e8518c816e3e0f4f7c3320bb7fc8cdebc4997ff8bb46777a2e2955283dd9a5"'","vout":0}]' \
  '{"'"$ADDR"'":49.999}')
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ SIGNED=$(./bitcoin-cli -regtest signrawtransactionwithwallet $RAW | jq -r .hex)
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli -regtest sendrawtransaction $SIGNED
c5ceb2a49687731fc359f9bfa4bc4deb0353bac100b2267325dc149d6d85720c
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli -regtest generatetoaddress 1 $ADDR
[
  "54a452dcaa240214de9de207fa470cbaad9e037b6da66280c019abd2ed9ea868"
]
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli -regtest listlockunspent
[
]
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli stop
Bitcoin Core stopping
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoind
Bitcoin Core starting
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli loadwallet locktest
{
  "name": "locktest"
}
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli -regtest listlockunspent
[
  {
    "txid": "56e8518c816e3e0f4f7c3320bb7fc8cdebc4997ff8bb46777a2e2955283dd9a5",
    "vout": 0
  }
]

After:

ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ ./bitcoin-cli -regtest lockunspent false '[{"txid":"'"642c8da8bd3c98c34a9fcbbc6361ebb33847efb60fc37b4f0dbee5779fde8caf"'","vout":0}]' true
true
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ ./bitcoin-cli getbalance
50.00000000
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ RAW=$(./bitcoin-cli -regtest createrawtransaction \
  '[{"txid":"'"$TXID"'","vout":0}]' \
  '{"'"$ADDR"'":49.999}')
error code: -8
error message:
txid must be of length 64 (not 0, for '')
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ RAW=$(./bitcoin-cli -regtest createrawtransaction \
  '[{"txid":"'"642c8da8bd3c98c34a9fcbbc6361ebb33847efb60fc37b4f0dbee5779fde8caf"'","vout":0}]' \
  '{"'"$ADDR"'":49.999}')
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ SIGNED=$(./bitcoin-cli -regtest signrawtransactionwithwallet $RAW | jq -r .hex)
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ ./bitcoin-cli -regtest sendrawtransaction $SIGNED
bf2dbc5d32806197c9beddd750a5b90dd50f855fe2736ed8a7101db1618664aa
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ ./bitcoin-cli -regtest generatetoaddress 1 $ADDR
[
  "1821ed455cf0c0b5c5b5dbd537922cef873b802e732ae1b2acb53835a5588e06"
]
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ ./bitcoin-cli -regtest listlockunspent
[
]
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ ./bitcoin-cli stop
Bitcoin Core stopping
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ ./bitcoind
Bitcoin Core starting
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ ./bitcoin-cli loadwallet locktest
{
  "name": "locktest"
}
ratedg@0xratedg:~/projects/contributions/bitcoin/build2/bin$ ./bitcoin-cli -regtest listlockunspent
[
]

ArvinFarrelP commented at 1:02 AM on September 24, 2026:

Thanks for the Concept ACK and testing! I didn't go with unlock-then-lock because it requires two RPC calls and isn't atomic — another process could lock the coin in between. The fix updates the existing entry when emplace reports the coin was already present, handling the upgrade in a single call and matching the existing behavior in coins.cpp:328.

vicjuma commented at 1:22 PM on September 23, 2026: contributor

Concept ACK.

But why won't the user just unlock then lock it again with persistence:-)? So that after

ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli lockunspent false '[{"txid":"'"ad83e72a347518506d6043643b42a7be682d2178aa2905d63c918e7eeb800feb"'","vout":0}]'
true
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli listlockunspent
[
  {
    "txid": "ad83e72a347518506d6043643b42a7be682d2178aa2905d63c918e7eeb800feb",
    "vout": 0
  }
]

they can

ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli lockunspent true '[{"txid":"'"ad83e72a347518506d6043643b42a7be682d2178aa2905d63c918e7eeb800feb"'","vout":0}]'
true
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli listlockunspent
[
]
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli lockunspent false '[{"txid":"'"ad83e72a347518506d6043643b42a7be682d2178aa2905d63c918e7eeb800feb"'","vout":0}]' true
true
ratedg@0xratedg:~/projects/contributions/bitcoin/build/bin$ ./bitcoin-cli listlockunspent
[
  {
    "txid": "ad83e72a347518506d6043643b42a7be682d2178aa2905d63c918e7eeb800feb",
    "vout": 0
  }
]

Maybe your version offers a great UX. This code src/wallet/rpc/coins.cpp:328: throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output already locked");

    if (!fUnlock && is_locked && !persistent) {
            throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output already locked");
        }

also supports your solution cause it dictates that we could go from non-persistence to persistence in a single call but not the other way around