diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index bc6df1cc99e..ddb8bfaf179 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -364,10 +364,12 @@ UniValue importprunedfunds(const JSONRPCRequest& request) std::vector vMatch; std::vector vIndex; unsigned int txnIndex = 0; + Optional height; if (merkleBlock.txn.ExtractMatches(vMatch, vIndex) == merkleBlock.header.hashMerkleRoot) { auto locked_chain = pwallet->chain().lock(); - if (locked_chain->getBlockHeight(merkleBlock.header.GetHash()) == nullopt) { + height = locked_chain->getBlockHeight(merkleBlock.header.GetHash()); + if (height == nullopt) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain"); } @@ -382,7 +384,7 @@ UniValue importprunedfunds(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Something wrong with merkleblock"); } - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, merkleBlock.header.GetHash(), txnIndex); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, *height, merkleBlock.header.GetHash(), txnIndex); wtx.m_confirm = confirm; auto locked_chain = pwallet->chain().lock(); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index b6acaf4d999..3f0e40149cf 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -276,7 +276,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) AssertLockHeld(spk_man->cs_wallet); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 0); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash(), 0); wtx.m_confirm = confirm; // Call GetImmatureCredit() once before adding the key to the wallet to @@ -318,7 +318,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 wallet.AddToWallet(wtx); } if (block) { - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block->GetBlockHash(), 0); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block->nHeight, block->GetBlockHash(), 0); wtx.m_confirm = confirm; } wallet.AddToWallet(wtx); @@ -499,7 +499,7 @@ public: wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, ::ChainActive().Tip()->GetBlockHash()); auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 1); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash(), 1); it->second.m_confirm = confirm; return it->second; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2ee1d001b91..512273aa822 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -766,10 +766,12 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) wtx.m_confirm.status = wtxIn.m_confirm.status; wtx.m_confirm.nIndex = wtxIn.m_confirm.nIndex; wtx.m_confirm.hashBlock = wtxIn.m_confirm.hashBlock; + wtx.m_confirm.block_height = wtxIn.m_confirm.block_height; fUpdated = true; } else { assert(wtx.m_confirm.nIndex == wtxIn.m_confirm.nIndex); assert(wtx.m_confirm.hashBlock == wtxIn.m_confirm.hashBlock); + assert(wtx.m_confirm.block_height == wtxIn.m_confirm.block_height); } if (wtxIn.fFromMe && wtxIn.fFromMe != wtx.fFromMe) { @@ -820,12 +822,22 @@ void CWallet::LoadToWallet(CWalletTx& wtxIn) { // If wallet doesn't have a chain (e.g wallet-tool), lock can't be taken. auto locked_chain = LockChain(); - // If tx hasn't been reorged out of chain while wallet being shutdown - // change tx status to UNCONFIRMED and reset hashBlock/nIndex. - if (!wtxIn.m_confirm.hashBlock.IsNull()) { - if (locked_chain && !locked_chain->getBlockHeight(wtxIn.m_confirm.hashBlock)) { + if (locked_chain) { + Optional block_height = locked_chain->getBlockHeight(wtxIn.m_confirm.hashBlock); + if (block_height) { + // Update cached block height variable since it not stored in the + // serialized transaction. + wtxIn.m_confirm.block_height = *block_height; + } else if (wtxIn.isConflicted() || wtxIn.isConfirmed()) { + // If tx block (or conflicting block) was reorged out of chain + // while the wallet was shutdown, change tx status to UNCONFIRMED + // and reset block height, hash, and index. ABANDONED tx don't have + // associated blocks and don't need to be updated. The case where a + // transaction was reorged out while online and then reconfirmed + // while offline is covered by the rescan logic. wtxIn.setUnconfirmed(); wtxIn.m_confirm.hashBlock = uint256(); + wtxIn.m_confirm.block_height = 0; wtxIn.m_confirm.nIndex = 0; } } @@ -842,7 +854,7 @@ void CWallet::LoadToWallet(CWalletTx& wtxIn) if (it != mapWallet.end()) { CWalletTx& prevtx = it->second; if (prevtx.isConflicted()) { - MarkConflicted(prevtx.m_confirm.hashBlock, wtx.GetHash()); + MarkConflicted(prevtx.m_confirm.hashBlock, prevtx.m_confirm.block_height, wtx.GetHash()); } } } @@ -860,7 +872,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Co while (range.first != range.second) { if (range.first->second != tx.GetHash()) { WalletLogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), confirm.hashBlock.ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n); - MarkConflicted(confirm.hashBlock, range.first->second); + MarkConflicted(confirm.hashBlock, confirm.block_height, range.first->second); } range.first++; } @@ -948,7 +960,6 @@ bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const ui 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()); - wtx.m_confirm.nIndex = 0; wtx.setAbandoned(); wtx.MarkDirty(); batch.WriteTx(wtx); @@ -970,7 +981,7 @@ bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const ui return true; } -void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) +void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx) { auto locked_chain = chain().lock(); LOCK(cs_wallet); @@ -1004,6 +1015,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) // Mark transaction as conflicted with this block. wtx.m_confirm.nIndex = 0; wtx.m_confirm.hashBlock = hashBlock; + wtx.m_confirm.block_height = conflicting_height; wtx.setConflicted(); wtx.MarkDirty(); batch.WriteTx(wtx); @@ -1036,7 +1048,7 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmatio void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) { auto locked_chain = chain().lock(); LOCK(cs_wallet); - CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, {}, 0); + CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0); SyncTransaction(ptx, confirm); auto it = mapWallet.find(ptx->GetHash()); @@ -1061,10 +1073,10 @@ void CWallet::BlockConnected(const CBlock& block, const std::vectorGetHash(); } bool IsCoinBase() const { return tx->IsCoinBase(); } @@ -644,7 +647,7 @@ private: bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /* Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */ - void MarkConflicted(const uint256& hashBlock, const uint256& hashTx); + void MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx); /* Mark a transaction's inputs dirty, thus forcing the outputs to be recomputed */ void MarkInputsDirty(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);