aboutsummaryrefslogtreecommitdiff
path: root/src/wallet/wallet.cpp
diff options
context:
space:
mode:
authorAndrew Chow <github@achow101.com>2023-05-27 12:45:54 -0400
committerAndrew Chow <github@achow101.com>2023-05-27 13:07:09 -0400
commit7d33ae755de2bff806fe600bdaebedbd7fa67aba (patch)
tree136fb71453158f362f8dbb0d99aa3b4dac63f9cb /src/wallet/wallet.cpp
parent927b001502a74a7224f04cfe6ffddc9a59409ba1 (diff)
parent89df7987c2f1eea42454c2b0efc31a924fbfd3a8 (diff)
Merge bitcoin/bitcoin#27145: wallet: when a block is disconnected, update transactions that are no longer conflicted
89df7987c2f1eea42454c2b0efc31a924fbfd3a8 Add wallets_conflicts (Antoine Riard) dced203162d45e542f187f8d0d07dab85c52cf28 wallet, tests: mark unconflicted txs as inactive (ishaanam) 096487c4dcfadebe5ca959927f5426cae1c304d5 wallet: introduce generic recursive tx state updating function (ishaanam) Pull request description: This implements a fix for #7315. Previously when a block was disconnected any transactions that were conflicting with transactions mined in that block were not updated to be marked as inactive. The fix implemented here is described on the [Bitcoin DevWiki](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking#idea-refresh-conflicted). A test which tested the previous behavior has also been updated. Second attempt at #17543 ACKs for top commit: achow101: ACK 89df7987c2f1eea42454c2b0efc31a924fbfd3a8 rajarshimaitra: tACK 89df7987c2f1eea42454c2b0efc31a924fbfd3a8. glozow: ACK 89df7987c2f1eea42454c2b0efc31a924fbfd3a8 furszy: Tested ACK 89df7987 Tree-SHA512: 3133b151477e8818302fac29e96de30cd64c09a8fe3a7612074a34ba1a332e69148162e6cb3f1074d9d9c9bab5ef9996967d325d8c4c99ba42b5fe3b66a60546
Diffstat (limited to 'src/wallet/wallet.cpp')
-rw-r--r--src/wallet/wallet.cpp121
1 files changed, 72 insertions, 49 deletions
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index d5268afbf6..0b241c4ef4 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1266,11 +1266,6 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
{
LOCK(cs_wallet);
- WalletBatch batch(GetDatabase());
-
- std::set<uint256> todo;
- std::set<uint256> done;
-
// Can't mark abandoned if confirmed or in mempool
auto it = mapWallet.find(hashTx);
assert(it != mapWallet.end());
@@ -1279,44 +1274,25 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
return false;
}
- todo.insert(hashTx);
-
- while (!todo.empty()) {
- uint256 now = *todo.begin();
- todo.erase(now);
- done.insert(now);
- auto it = mapWallet.find(now);
- assert(it != mapWallet.end());
- CWalletTx& wtx = it->second;
- int currentconfirm = GetTxDepthInMainChain(wtx);
- // If the orig tx was not in block, none of its spends can be
- assert(currentconfirm <= 0);
- // if (currentconfirm < 0) {Tx and spends are already conflicted, no need to abandon}
- if (currentconfirm == 0 && !wtx.isAbandoned()) {
- // If the orig tx was not in block/mempool, none of its spends can be in mempool
- assert(!wtx.InMempool());
+ auto try_updating_state = [](CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) {
+ // If the orig tx was not in block/mempool, none of its spends can be.
+ assert(!wtx.isConfirmed());
+ assert(!wtx.InMempool());
+ // If already conflicted or abandoned, no need to set abandoned
+ if (!wtx.isConflicted() && !wtx.isAbandoned()) {
wtx.m_state = TxStateInactive{/*abandoned=*/true};
- wtx.MarkDirty();
- batch.WriteTx(wtx);
- NotifyTransactionChanged(wtx.GetHash(), CT_UPDATED);
- // Iterate over all its outputs, and mark transactions in the wallet that spend them abandoned too.
- // States are not permanent, so these transactions can become unabandoned if they are re-added to the
- // mempool, or confirmed in a block, or conflicted.
- // Note: If the reorged coinbase is re-added to the main chain, the descendants that have not had their
- // states change will remain abandoned and will require manual broadcast if the user wants them.
- for (unsigned int i = 0; i < wtx.tx->vout.size(); ++i) {
- std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(COutPoint(now, i));
- for (TxSpends::const_iterator iter = range.first; iter != range.second; ++iter) {
- if (!done.count(iter->second)) {
- todo.insert(iter->second);
- }
- }
- }
- // If a transaction changes 'conflicted' state, that changes the balance
- // available of the outputs it spends. So force those to be recomputed
- MarkInputsDirty(wtx.tx);
+ return TxUpdate::NOTIFY_CHANGED;
}
- }
+ return TxUpdate::UNCHANGED;
+ };
+
+ // Iterate over all its outputs, and mark transactions in the wallet that spend them abandoned too.
+ // States are not permanent, so these transactions can become unabandoned if they are re-added to the
+ // mempool, or confirmed in a block, or conflicted.
+ // Note: If the reorged coinbase is re-added to the main chain, the descendants that have not had their
+ // states change will remain abandoned and will require manual broadcast if the user wants them.
+
+ RecursiveUpdateTxState(hashTx, try_updating_state);
return true;
}
@@ -1333,13 +1309,29 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c
if (conflictconfirms >= 0)
return;
+ auto try_updating_state = [&](CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) {
+ if (conflictconfirms < GetTxDepthInMainChain(wtx)) {
+ // Block is 'more conflicted' than current confirm; update.
+ // Mark transaction as conflicted with this block.
+ wtx.m_state = TxStateConflicted{hashBlock, conflicting_height};
+ return TxUpdate::CHANGED;
+ }
+ return TxUpdate::UNCHANGED;
+ };
+
+ // Iterate over all its outputs, and mark transactions in the wallet that spend them conflicted too.
+ RecursiveUpdateTxState(hashTx, try_updating_state);
+
+}
+
+void CWallet::RecursiveUpdateTxState(const uint256& tx_hash, const TryUpdatingStateFn& try_updating_state) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) {
// Do not flush the wallet here for performance reasons
WalletBatch batch(GetDatabase(), false);
std::set<uint256> todo;
std::set<uint256> done;
- todo.insert(hashTx);
+ todo.insert(tx_hash);
while (!todo.empty()) {
uint256 now = *todo.begin();
@@ -1348,14 +1340,12 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c
auto it = mapWallet.find(now);
assert(it != mapWallet.end());
CWalletTx& wtx = it->second;
- int currentconfirm = GetTxDepthInMainChain(wtx);
- if (conflictconfirms < currentconfirm) {
- // Block is 'more conflicted' than current confirm; update.
- // Mark transaction as conflicted with this block.
- wtx.m_state = TxStateConflicted{hashBlock, conflicting_height};
+
+ TxUpdate update_state = try_updating_state(wtx);
+ if (update_state != TxUpdate::UNCHANGED) {
wtx.MarkDirty();
batch.WriteTx(wtx);
- // Iterate over all its outputs, and mark transactions in the wallet that spend them conflicted too
+ // Iterate over all its outputs, and update those tx states as well (if applicable)
for (unsigned int i = 0; i < wtx.tx->vout.size(); ++i) {
std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(COutPoint(now, i));
for (TxSpends::const_iterator iter = range.first; iter != range.second; ++iter) {
@@ -1364,7 +1354,12 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c
}
}
}
- // If a transaction changes 'conflicted' state, that changes the balance
+
+ if (update_state == TxUpdate::NOTIFY_CHANGED) {
+ NotifyTransactionChanged(wtx.GetHash(), CT_UPDATED);
+ }
+
+ // If a transaction changes its tx state, that usually changes the balance
// available of the outputs it spends. So force those to be recomputed
MarkInputsDirty(wtx.tx);
}
@@ -1459,8 +1454,36 @@ void CWallet::blockDisconnected(const interfaces::BlockInfo& block)
// future with a stickier abandoned state or even removing abandontransaction call.
m_last_block_processed_height = block.height - 1;
m_last_block_processed = *Assert(block.prev_hash);
+
+ int disconnect_height = block.height;
+
for (const CTransactionRef& ptx : Assert(block.data)->vtx) {
SyncTransaction(ptx, TxStateInactive{});
+
+ for (const CTxIn& tx_in : ptx->vin) {
+ // No other wallet transactions conflicted with this transaction
+ if (mapTxSpends.count(tx_in.prevout) < 1) continue;
+
+ std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(tx_in.prevout);
+
+ // For all of the spends that conflict with this transaction
+ for (TxSpends::const_iterator _it = range.first; _it != range.second; ++_it) {
+ CWalletTx& wtx = mapWallet.find(_it->second)->second;
+
+ if (!wtx.isConflicted()) continue;
+
+ auto try_updating_state = [&](CWalletTx& tx) {
+ if (!tx.isConflicted()) return TxUpdate::UNCHANGED;
+ if (tx.state<TxStateConflicted>()->conflicting_block_height >= disconnect_height) {
+ tx.m_state = TxStateInactive{};
+ return TxUpdate::CHANGED;
+ }
+ return TxUpdate::UNCHANGED;
+ };
+
+ RecursiveUpdateTxState(wtx.tx->GetHash(), try_updating_state);
+ }
+ }
}
}
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-17 04:37:39 +0000