diff options
-rw-r--r-- | src/wallet/rpcdump.cpp | 6 | ||||
-rw-r--r-- | src/wallet/test/wallet_tests.cpp | 6 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 42 | ||||
-rw-r--r-- | src/wallet/wallet.h | 15 |
4 files changed, 43 insertions, 26 deletions
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index bc6df1cc99..ddb8bfaf17 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -364,10 +364,12 @@ UniValue importprunedfunds(const JSONRPCRequest& request) std::vector<uint256> vMatch; std::vector<unsigned int> vIndex; unsigned int txnIndex = 0; + Optional<int> 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 b6acaf4d99..3f0e40149c 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 2ee1d001b9..512273aa82 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<int> 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::vector<CTransaction m_last_block_processed_height = height; m_last_block_processed = block_hash; - for (size_t i = 0; i < block.vtx.size(); i++) { - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, m_last_block_processed, i); - SyncTransaction(block.vtx[i], confirm); - TransactionRemovedFromMempool(block.vtx[i]); + for (size_t index = 0; index < block.vtx.size(); index++) { + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, block_hash, index); + SyncTransaction(block.vtx[index], confirm); + TransactionRemovedFromMempool(block.vtx[index]); } for (const CTransactionRef& ptx : vtxConflicted) { TransactionRemovedFromMempool(ptx); @@ -1083,7 +1095,7 @@ void CWallet::BlockDisconnected(const CBlock& block, int height) m_last_block_processed_height = height - 1; m_last_block_processed = block.hashPrevBlock; for (const CTransactionRef& ptx : block.vtx) { - CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, {}, 0); + CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0); SyncTransaction(ptx, confirm); } } @@ -1630,7 +1642,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc break; } for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) { - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block_hash, posInBlock); + CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, *block_height, block_hash, posInBlock); SyncTransaction(block.vtx[posInBlock], confirm, fUpdate); } // scan succeeded, record block as most recent successfully scanned diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 10a3e17f47..f3691a6218 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -356,15 +356,17 @@ public: ABANDONED }; - /* Confirmation includes tx status and a pair of {block hash/tx index in block} at which tx has been confirmed. - * This pair is both 0 if tx hasn't confirmed yet. Meaning of these fields changes with CONFLICTED state - * where they instead point to block hash and index of the deepest conflicting tx. + /* Confirmation includes tx status and a triplet of {block height/block hash/tx index in block} + * at which tx has been confirmed. All three are set to 0 if tx is unconfirmed or abandoned. + * Meaning of these fields changes with CONFLICTED state where they instead point to block hash + * and block height of the deepest conflicting tx. */ struct Confirmation { Status status; + int block_height; uint256 hashBlock; int nIndex; - Confirmation(Status s = UNCONFIRMED, uint256 h = uint256(), int i = 0) : status(s), hashBlock(h), nIndex(i) {} + Confirmation(Status s = UNCONFIRMED, int b = 0, uint256 h = uint256(), int i = 0) : status(s), block_height(b), hashBlock(h), nIndex(i) {} }; Confirmation m_confirm; @@ -407,7 +409,6 @@ public: * compatibility (pre-commit 9ac63d6). */ if (serializedIndex == -1 && m_confirm.hashBlock == ABANDON_HASH) { - m_confirm.hashBlock = uint256(); setAbandoned(); } else if (serializedIndex == -1) { setConflicted(); @@ -512,12 +513,14 @@ public: { m_confirm.status = CWalletTx::ABANDONED; m_confirm.hashBlock = uint256(); + m_confirm.block_height = 0; m_confirm.nIndex = 0; } bool isConflicted() const { return m_confirm.status == CWalletTx::CONFLICTED; } void setConflicted() { m_confirm.status = CWalletTx::CONFLICTED; } bool isUnconfirmed() const { return m_confirm.status == CWalletTx::UNCONFIRMED; } void setUnconfirmed() { m_confirm.status = CWalletTx::UNCONFIRMED; } + bool isConfirmed() const { return m_confirm.status == CWalletTx::CONFIRMED; } void setConfirmed() { m_confirm.status = CWalletTx::CONFIRMED; } const uint256& GetHash() const { return tx->GetHash(); } 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); |