diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index 6ad222864a..290ef4eaa9 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 e47a696188..aadad258a9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1065,6 +1065,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" : "")); @@ -1274,7 +1301,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 d9f7c14ded..d8931fa620 100755 --- a/test/functional/wallet_orphanedreward.py +++ b/test/functional/wallet_orphanedreward.py @@ -34,29 +34,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) - # 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) + 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()