From 9addbd78901124a48fd41a82a9557fcf3490191d Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 14 Nov 2022 15:14:12 -0500 Subject: [PATCH 1/2] wallet: Automatically abandon orphaned coinbases and their children --- src/wallet/transaction.h | 1 + src/wallet/wallet.cpp | 33 +++++++++++++++++++++++- test/functional/wallet_orphanedreward.py | 12 +-------- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index 27983e356d7..8da5f5532dc 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -293,6 +293,7 @@ public: bool isAbandoned() const { return state() && state()->abandoned; } bool isConflicted() const { return state(); } + bool isInactive() const { return state(); } bool isUnconfirmed() const { return !isAbandoned() && !isConflicted() && !isConfirmed(); } bool isConfirmed() const { return state(); } const uint256& GetHash() const { return tx->GetHash(); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 431e970edc1..22d8fcb67a4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1067,6 +1067,33 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const } } + // Mark inactive coinbase transactions and their descendants as abandoned + if (wtx.IsCoinBase() && wtx.isInactive()) { + std::vector txs{&wtx}; + + TxStateInactive inactive_state = TxStateInactive{/*abandoned=*/true}; + + while (!txs.empty()) { + CWalletTx* desc_tx = txs.back(); + txs.pop_back(); + desc_tx->m_state = inactive_state; + // Break caches since we have changed the state + desc_tx->MarkDirty(); + batch.WriteTx(*desc_tx); + MarkInputsDirty(desc_tx->tx); + for (unsigned int i = 0; i < desc_tx->tx->vout.size(); ++i) { + COutPoint outpoint(desc_tx->GetHash(), i); + std::pair range = mapTxSpends.equal_range(outpoint); + for (TxSpends::const_iterator it = range.first; it != range.second; ++it) { + const auto wit = mapWallet.find(it->second); + if (wit != mapWallet.end()) { + txs.push_back(&wit->second); + } + } + } + } + } + //// debug print WalletLogPrintf("AddToWallet %s %s%s\n", hash.ToString(), (fInsertedNew ? "new" : ""), (fUpdated ? "update" : "")); @@ -1276,7 +1303,11 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) 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 + // 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 range = mapTxSpends.equal_range(COutPoint(now, i)); for (TxSpends::const_iterator iter = range.first; iter != range.second; ++iter) { diff --git a/test/functional/wallet_orphanedreward.py b/test/functional/wallet_orphanedreward.py index 7295db46536..37f39929364 100755 --- a/test/functional/wallet_orphanedreward.py +++ b/test/functional/wallet_orphanedreward.py @@ -37,17 +37,7 @@ class OrphanedBlockRewardTest(BitcoinTestFramework): # from the wallet can still be spent. self.nodes[0].invalidateblock(blk) self.generate(self.nodes[0], 152) - # Without the following abandontransaction call, the coins are - # not considered available yet. - assert_equal(self.nodes[1].getbalances()["mine"], { - "trusted": 0, - "untrusted_pending": 0, - "immature": 0, - }) - # The following abandontransaction is necessary to make the later - # lines succeed, and probably should not be needed; see - # https://github.com/bitcoin/bitcoin/issues/14148. - self.nodes[1].abandontransaction(txid) + # We expect the descendants of orphaned rewards to no longer be considered assert_equal(self.nodes[1].getbalances()["mine"], { "trusted": 10, "untrusted_pending": 0, From b0fa5989e1b77a343349bd36f8bc407f9366a589 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 14 Nov 2022 16:02:36 -0500 Subject: [PATCH 2/2] test: Check that orphaned coinbase unconf spend is still abandoned When an orphaned coinbase is reorged back into the main chain, any unconfirmed ancestors should still be marked as abandoned due to the original reorg that orphaned that coinbase. --- test/functional/wallet_orphanedreward.py | 25 ++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/test/functional/wallet_orphanedreward.py b/test/functional/wallet_orphanedreward.py index 37f39929364..a617abae832 100755 --- a/test/functional/wallet_orphanedreward.py +++ b/test/functional/wallet_orphanedreward.py @@ -31,19 +31,40 @@ class OrphanedBlockRewardTest(BitcoinTestFramework): # the existing balance and the block reward. self.generate(self.nodes[0], 150) assert_equal(self.nodes[1].getbalance(), 10 + 25) + pre_reorg_conf_bals = self.nodes[1].getbalances() txid = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 30) + orig_chain_tip = self.nodes[0].getbestblockhash() + self.sync_mempools() # Orphan the block reward and make sure that the original coins # from the wallet can still be spent. self.nodes[0].invalidateblock(blk) - self.generate(self.nodes[0], 152) + blocks = self.generate(self.nodes[0], 152) + conflict_block = blocks[0] # We expect the descendants of orphaned rewards to no longer be considered assert_equal(self.nodes[1].getbalances()["mine"], { "trusted": 10, "untrusted_pending": 0, "immature": 0, }) - self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 9) + # And the unconfirmed tx to be abandoned + assert_equal(self.nodes[1].gettransaction(txid)["details"][0]["abandoned"], True) + + # The abandoning should persist through reloading + self.nodes[1].unloadwallet(self.default_wallet_name) + self.nodes[1].loadwallet(self.default_wallet_name) + assert_equal(self.nodes[1].gettransaction(txid)["details"][0]["abandoned"], True) + + # If the orphaned reward is reorged back into the main chain, any unconfirmed + # descendant txs at the time of the original reorg remain abandoned. + self.nodes[0].invalidateblock(conflict_block) + self.nodes[0].reconsiderblock(blk) + assert_equal(self.nodes[0].getbestblockhash(), orig_chain_tip) + self.generate(self.nodes[0], 3) + + assert_equal(self.nodes[1].getbalances(), pre_reorg_conf_bals) + assert_equal(self.nodes[1].gettransaction(txid)["details"][0]["abandoned"], True) + if __name__ == '__main__': OrphanedBlockRewardTest().main()