diff options
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/load.h | 2 | ||||
-rw-r--r-- | src/wallet/rpcdump.cpp | 3 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 33 | ||||
-rw-r--r-- | src/wallet/test/wallet_tests.cpp | 22 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 171 | ||||
-rw-r--r-- | src/wallet/wallet.h | 92 |
6 files changed, 202 insertions, 121 deletions
diff --git a/src/wallet/load.h b/src/wallet/load.h index 81f078fd10..5a62e29303 100644 --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -17,7 +17,7 @@ class Chain; //! Responsible for reading and validating the -wallet arguments and verifying the wallet database. //! This function will perform salvage on the wallet if requested, as long as only one wallet is -//! being loaded (WalletParameterInteraction forbids -salvagewallet, -zapwallettxes or -upgradewallet with multiwallet). +//! being loaded (WalletInit::ParameterInteraction() forbids -salvagewallet, -zapwallettxes or -upgradewallet with multiwallet). bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files); //! Load wallet databases. diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 7707d6233b..f52e4318c8 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -384,8 +384,7 @@ UniValue importprunedfunds(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Something wrong with merkleblock"); } - wtx.nIndex = txnIndex; - wtx.hashBlock = merkleBlock.header.GetHash(); + wtx.SetConf(CWalletTx::Status::CONFIRMED, merkleBlock.header.GetHash(), txnIndex); auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 098778d878..96fa50d42e 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -134,10 +134,10 @@ static void WalletTxToJSON(interfaces::Chain& chain, interfaces::Chain::Lock& lo entry.pushKV("generated", true); if (confirms > 0) { - entry.pushKV("blockhash", wtx.hashBlock.GetHex()); - entry.pushKV("blockindex", wtx.nIndex); + entry.pushKV("blockhash", wtx.m_confirm.hashBlock.GetHex()); + entry.pushKV("blockindex", wtx.m_confirm.nIndex); int64_t block_time; - bool found_block = chain.findBlock(wtx.hashBlock, nullptr /* block */, &block_time); + bool found_block = chain.findBlock(wtx.m_confirm.hashBlock, nullptr /* block */, &block_time); assert(found_block); entry.pushKV("blocktime", block_time); } else { @@ -1648,7 +1648,10 @@ static UniValue gettransaction(const JSONRPCRequest& request) "\nGet detailed information about in-wallet transaction <txid>\n", { {"txid", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction id"}, - {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Whether to include watch-only addresses in balance calculation and details[]"}, + {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", + "Whether to include watch-only addresses in balance calculation and details[]"}, + {"verbose", RPCArg::Type::BOOL, /* default */ "false", + "Whether to include a `decoded` field containing the decoded transaction (equivalent to RPC decoderawtransaction)"}, }, RPCResult{ "{\n" @@ -1684,11 +1687,14 @@ static UniValue gettransaction(const JSONRPCRequest& request) " ,...\n" " ],\n" " \"hex\" : \"data\" (string) Raw data for transaction\n" + " \"decoded\" : transaction (json object) Optional, the decoded transaction (only present when `verbose` is passed), equivalent to the\n" + " RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed.\n" "}\n" }, RPCExamples{ HelpExampleCli("gettransaction", "\"1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d\"") + HelpExampleCli("gettransaction", "\"1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d\" true") + + HelpExampleCli("gettransaction", "\"1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d\" false true") + HelpExampleRpc("gettransaction", "\"1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d\"") }, }.Check(request); @@ -1708,6 +1714,8 @@ static UniValue gettransaction(const JSONRPCRequest& request) filter |= ISMINE_WATCH_ONLY; } + bool verbose = request.params[2].isNull() ? false : request.params[2].get_bool(); + UniValue entry(UniValue::VOBJ); auto it = pwallet->mapWallet.find(hash); if (it == pwallet->mapWallet.end()) { @@ -1733,6 +1741,12 @@ static UniValue gettransaction(const JSONRPCRequest& request) std::string strHex = EncodeHexTx(*wtx.tx, pwallet->chain().rpcSerializationFlags()); entry.pushKV("hex", strHex); + if (verbose) { + UniValue decoded(UniValue::VOBJ); + TxToUniv(*wtx.tx, uint256(), decoded, false); + entry.pushKV("decoded", decoded); + } + return entry; } @@ -3121,7 +3135,9 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) {"changeAddress", RPCArg::Type::STR, /* default */ "pool address", "The bitcoin address to receive the change"}, {"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"}, {"change_type", RPCArg::Type::STR, /* default */ "set by -changetype", "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."}, - {"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only"}, + {"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only.\n" + "Only solvable inputs can be used. Watch-only destinations are solvable if the public key and/or output script was imported,\n" + "e.g. with 'importpubkey' or 'importmulti' with the 'pubkeys' or 'desc' field."}, {"lockUnspents", RPCArg::Type::BOOL, /* default */ "false", "Lock selected unspent outputs"}, {"feeRate", RPCArg::Type::AMOUNT, /* default */ "not set: makes wallet determine the fee", "Set a specific fee rate in " + CURRENCY_UNIT + "/kB"}, {"subtractFeeFromOutputs", RPCArg::Type::ARR, /* default */ "empty array", "A json array of integers.\n" @@ -3269,7 +3285,10 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request) } pwallet->chain().findCoins(coins); - return SignTransaction(mtx, request.params[1], pwallet, coins, false, request.params[2]); + // Parse the prevtxs array + ParsePrevouts(request.params[1], nullptr, coins); + + return SignTransaction(mtx, pwallet, coins, request.params[2]); } static UniValue bumpfee(const JSONRPCRequest& request) @@ -4186,7 +4205,7 @@ static const CRPCCommand commands[] = { "wallet", "getrawchangeaddress", &getrawchangeaddress, {"address_type"} }, { "wallet", "getreceivedbyaddress", &getreceivedbyaddress, {"address","minconf"} }, { "wallet", "getreceivedbylabel", &getreceivedbylabel, {"label","minconf"} }, - { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly"} }, + { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly","verbose"} }, { "wallet", "getunconfirmedbalance", &getunconfirmedbalance, {} }, { "wallet", "getbalances", &getbalances, {} }, { "wallet", "getwalletinfo", &getwalletinfo, {} }, diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 8af05dea45..fc3be2b6ab 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -249,8 +249,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) LockAssertion lock(::cs_main); LOCK(wallet.cs_wallet); - wtx.hashBlock = ::ChainActive().Tip()->GetBlockHash(); - wtx.nIndex = 0; + wtx.SetConf(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 0); // Call GetImmatureCredit() once before adding the key to the wallet to // cache the current immature credit amount, which is 0. @@ -281,14 +280,19 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 } CWalletTx wtx(&wallet, MakeTransactionRef(tx)); - if (block) { - wtx.SetMerkleBranch(block->GetBlockHash(), 0); - } - { - LOCK(cs_main); + LOCK(cs_main); + LOCK(wallet.cs_wallet); + // If transaction is already in map, to avoid inconsistencies, unconfirmation + // is needed before confirm again with different block. + std::map<uint256, CWalletTx>::iterator it = wallet.mapWallet.find(wtx.GetHash()); + if (it != wallet.mapWallet.end()) { + wtx.setUnconfirmed(); wallet.AddToWallet(wtx); } - LOCK(wallet.cs_wallet); + if (block) { + wtx.SetConf(CWalletTx::Status::CONFIRMED, block->GetBlockHash(), 0); + } + wallet.AddToWallet(wtx); return wallet.mapWallet.at(wtx.GetHash()).nTimeSmart; } @@ -382,7 +386,7 @@ public: LOCK(wallet->cs_wallet); auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); - it->second.SetMerkleBranch(::ChainActive().Tip()->GetBlockHash(), 1); + it->second.SetConf(CWalletTx::Status::CONFIRMED, ::ChainActive().Tip()->GetBlockHash(), 1); return it->second; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 03acf23508..23f61602d2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -93,13 +93,14 @@ std::shared_ptr<CWallet> GetWallet(const std::string& name) static Mutex g_wallet_release_mutex; static std::condition_variable g_wallet_release_cv; -static std::set<CWallet*> g_unloading_wallet_set; +static std::set<std::string> g_unloading_wallet_set; // Custom deleter for shared_ptr<CWallet>. static void ReleaseWallet(CWallet* wallet) { // Unregister and delete the wallet right after BlockUntilSyncedToCurrentChain // so that it's in sync with the current chainstate. + const std::string name = wallet->GetName(); wallet->WalletLogPrintf("Releasing wallet\n"); wallet->BlockUntilSyncedToCurrentChain(); wallet->Flush(); @@ -108,7 +109,7 @@ static void ReleaseWallet(CWallet* wallet) // Wallet is now released, notify UnloadWallet, if any. { LOCK(g_wallet_release_mutex); - if (g_unloading_wallet_set.erase(wallet) == 0) { + if (g_unloading_wallet_set.erase(name) == 0) { // UnloadWallet was not called for this wallet, all done. return; } @@ -119,21 +120,21 @@ static void ReleaseWallet(CWallet* wallet) void UnloadWallet(std::shared_ptr<CWallet>&& wallet) { // Mark wallet for unloading. - CWallet* pwallet = wallet.get(); + const std::string name = wallet->GetName(); { LOCK(g_wallet_release_mutex); - auto it = g_unloading_wallet_set.insert(pwallet); + auto it = g_unloading_wallet_set.insert(name); assert(it.second); } // The wallet can be in use so it's not possible to explicitly unload here. // Notify the unload intent so that all remaining shared pointers are // released. - pwallet->NotifyUnload(); + wallet->NotifyUnload(); // Time to ditch our shared_ptr and wait for ReleaseWallet call. wallet.reset(); { WAIT_LOCK(g_wallet_release_mutex, lock); - while (g_unloading_wallet_set.count(pwallet) == 1) { + while (g_unloading_wallet_set.count(name) == 1) { g_wallet_release_cv.wait(lock); } } @@ -523,18 +524,9 @@ bool CWallet::LoadCScript(const CScript& redeemScript) static bool ExtractPubKey(const CScript &dest, CPubKey& pubKeyOut) { - //TODO: Use Solver to extract this? - CScript::const_iterator pc = dest.begin(); - opcodetype opcode; - std::vector<unsigned char> vch; - if (!dest.GetOp(pc, opcode, vch) || !CPubKey::ValidSize(vch)) - return false; - pubKeyOut = CPubKey(vch); - if (!pubKeyOut.IsFullyValid()) - return false; - if (!dest.GetOp(pc, opcode, vch) || opcode != OP_CHECKSIG || dest.GetOp(pc, opcode, vch)) - return false; - return true; + std::vector<std::vector<unsigned char>> solutions; + return Solver(dest, solutions) == TX_PUBKEY && + (pubKeyOut = CPubKey(solutions[0])).IsFullyValid(); } bool CWallet::AddWatchOnlyInMem(const CScript &dest) @@ -1118,22 +1110,14 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) bool fUpdated = false; if (!fInsertedNew) { - // Merge - if (!wtxIn.hashUnset() && wtxIn.hashBlock != wtx.hashBlock) - { - wtx.hashBlock = wtxIn.hashBlock; - fUpdated = true; - } - // If no longer abandoned, update - if (wtxIn.hashBlock.IsNull() && wtx.isAbandoned()) - { - wtx.hashBlock = wtxIn.hashBlock; - fUpdated = true; - } - if (wtxIn.nIndex != -1 && (wtxIn.nIndex != wtx.nIndex)) - { - wtx.nIndex = wtxIn.nIndex; + if (wtxIn.m_confirm.status != wtx.m_confirm.status) { + wtx.m_confirm.status = wtxIn.m_confirm.status; + wtx.m_confirm.nIndex = wtxIn.m_confirm.nIndex; + wtx.m_confirm.hashBlock = wtxIn.m_confirm.hashBlock; fUpdated = true; + } else { + assert(wtx.m_confirm.nIndex == wtxIn.m_confirm.nIndex); + assert(wtx.m_confirm.hashBlock == wtxIn.m_confirm.hashBlock); } if (wtxIn.fFromMe && wtxIn.fFromMe != wtx.fFromMe) { @@ -1180,8 +1164,19 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) return true; } -void CWallet::LoadToWallet(const CWalletTx& wtxIn) +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)) { + wtxIn.setUnconfirmed(); + wtxIn.m_confirm.hashBlock = uint256(); + wtxIn.m_confirm.nIndex = 0; + } + } uint256 hash = wtxIn.GetHash(); const auto& ins = mapWallet.emplace(hash, wtxIn); CWalletTx& wtx = ins.first->second; @@ -1194,14 +1189,14 @@ void CWallet::LoadToWallet(const CWalletTx& wtxIn) auto it = mapWallet.find(txin.prevout.hash); if (it != mapWallet.end()) { CWalletTx& prevtx = it->second; - if (prevtx.nIndex == -1 && !prevtx.hashUnset()) { - MarkConflicted(prevtx.hashBlock, wtx.GetHash()); + if (prevtx.isConflicted()) { + MarkConflicted(prevtx.m_confirm.hashBlock, wtx.GetHash()); } } } } -bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const uint256& block_hash, int posInBlock, bool fUpdate) +bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Status status, const uint256& block_hash, int posInBlock, bool fUpdate) { const CTransaction& tx = *ptx; { @@ -1248,9 +1243,9 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const uint256 CWalletTx wtx(this, ptx); - // Get merkle branch if transaction was found in a block - if (!block_hash.IsNull()) - wtx.SetMerkleBranch(block_hash, posInBlock); + // Block disconnection override an abandoned tx as unconfirmed + // which means user may have to call abandontransaction again + wtx.SetConf(status, block_hash, posInBlock); return AddToWallet(wtx, false); } @@ -1310,7 +1305,7 @@ 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.nIndex = -1; + wtx.m_confirm.nIndex = 0; wtx.setAbandoned(); wtx.MarkDirty(); batch.WriteTx(wtx); @@ -1364,8 +1359,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) if (conflictconfirms < currentconfirm) { // Block is 'more conflicted' than current confirm; update. // Mark transaction as conflicted with this block. - wtx.nIndex = -1; - wtx.hashBlock = hashBlock; + wtx.m_confirm.nIndex = 0; + wtx.m_confirm.hashBlock = hashBlock; + wtx.setConflicted(); wtx.MarkDirty(); batch.WriteTx(wtx); // Iterate over all its outputs, and mark transactions in the wallet that spend them conflicted too @@ -1383,8 +1379,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) } } -void CWallet::SyncTransaction(const CTransactionRef& ptx, const uint256& block_hash, int posInBlock, bool update_tx) { - if (!AddToWalletIfInvolvingMe(ptx, block_hash, posInBlock, update_tx)) +void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Status status, const uint256& block_hash, int posInBlock, bool update_tx) +{ + if (!AddToWalletIfInvolvingMe(ptx, status, block_hash, posInBlock, update_tx)) return; // Not one of ours // If a transaction changes 'conflicted' state, that changes the balance @@ -1396,7 +1393,7 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, const uint256& block_h void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) { auto locked_chain = chain().lock(); LOCK(cs_wallet); - SyncTransaction(ptx, {} /* block hash */, 0 /* position in block */); + SyncTransaction(ptx, CWalletTx::Status::UNCONFIRMED, {} /* block hash */, 0 /* position in block */); auto it = mapWallet.find(ptx->GetHash()); if (it != mapWallet.end()) { @@ -1416,22 +1413,14 @@ void CWallet::BlockConnected(const CBlock& block, const std::vector<CTransaction const uint256& block_hash = block.GetHash(); auto locked_chain = chain().lock(); LOCK(cs_wallet); - // TODO: Temporarily ensure that mempool removals are notified before - // connected transactions. This shouldn't matter, but the abandoned - // state of transactions in our wallet is currently cleared when we - // receive another notification and there is a race condition where - // notification of a connected conflict might cause an outside process - // to abandon a transaction and then have it inadvertently cleared by - // the notification that the conflicted transaction was evicted. - for (const CTransactionRef& ptx : vtxConflicted) { - SyncTransaction(ptx, {} /* block hash */, 0 /* position in block */); - TransactionRemovedFromMempool(ptx); - } for (size_t i = 0; i < block.vtx.size(); i++) { - SyncTransaction(block.vtx[i], block_hash, i); + SyncTransaction(block.vtx[i], CWalletTx::Status::CONFIRMED, block_hash, i); TransactionRemovedFromMempool(block.vtx[i]); } + for (const CTransactionRef& ptx : vtxConflicted) { + TransactionRemovedFromMempool(ptx); + } m_last_block_processed = block_hash; } @@ -1440,8 +1429,12 @@ void CWallet::BlockDisconnected(const CBlock& block) { auto locked_chain = chain().lock(); LOCK(cs_wallet); + // At block disconnection, this will change an abandoned transaction to + // be unconfirmed, whether or not the transaction is added back to the mempool. + // User may have to call abandontransaction again. It may be addressed in the + // future with a stickier abandoned state or even removing abandontransaction call. for (const CTransactionRef& ptx : block.vtx) { - SyncTransaction(ptx, {} /* block hash */, 0 /* position in block */); + SyncTransaction(ptx, CWalletTx::Status::UNCONFIRMED, {} /* block hash */, 0 /* position in block */); } } @@ -2078,7 +2071,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc break; } for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) { - SyncTransaction(block.vtx[posInBlock], block_hash, posInBlock, fUpdate); + SyncTransaction(block.vtx[posInBlock], CWalletTx::Status::CONFIRMED, block_hash, posInBlock, fUpdate); } // scan succeeded, record block as most recent successfully scanned result.last_scanned_block = block_hash; @@ -3340,6 +3333,11 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve DBErrors CWallet::LoadWallet(bool& fFirstRunRet) { + // Even if we don't use this lock in this function, we want to preserve + // lock order in LoadToWallet if query of chain state is needed to know + // tx status. If lock can't be taken (e.g wallet-tool), tx confirmation + // status may be not reliable. + auto locked_chain = LockChain(); LOCK(cs_wallet); fFirstRunRet = false; @@ -3378,6 +3376,7 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256 const auto& it = mapWallet.find(hash); wtxOrdered.erase(it->second.m_it_wtxOrdered); mapWallet.erase(it); + NotifyTransactionChanged(this, hash, CT_DELETED); } if (nZapSelectTxRet == DBErrors::NEED_REWRITE) @@ -4050,7 +4049,7 @@ void CWallet::GetKeyBirthTimes(interfaces::Chain::Lock& locked_chain, std::map<C for (const auto& entry : mapWallet) { // iterate over all wallet transactions... const CWalletTx &wtx = entry.second; - if (Optional<int> height = locked_chain.getBlockHeight(wtx.hashBlock)) { + if (Optional<int> height = locked_chain.getBlockHeight(wtx.m_confirm.hashBlock)) { // ... which are already in a block for (const CTxOut &txout : wtx.tx->vout) { // iterate over all their outputs @@ -4093,9 +4092,9 @@ void CWallet::GetKeyBirthTimes(interfaces::Chain::Lock& locked_chain, std::map<C unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const { unsigned int nTimeSmart = wtx.nTimeReceived; - if (!wtx.hashUnset()) { + if (!wtx.isUnconfirmed() && !wtx.isAbandoned()) { int64_t blocktime; - if (chain().findBlock(wtx.hashBlock, nullptr /* block */, &blocktime)) { + if (chain().findBlock(wtx.m_confirm.hashBlock, nullptr /* block */, &blocktime)) { int64_t latestNow = wtx.nTimeReceived; int64_t latestEntry = 0; @@ -4123,7 +4122,7 @@ unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const nTimeSmart = std::max(latestEntry, std::min(blocktime, latestNow)); } else { - WalletLogPrintf("%s: found %s in block %s not in index\n", __func__, wtx.GetHash().ToString(), wtx.hashBlock.ToString()); + WalletLogPrintf("%s: found %s in block %s not in index\n", __func__, wtx.GetHash().ToString(), wtx.m_confirm.hashBlock.ToString()); } } return nTimeSmart; @@ -4241,6 +4240,11 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b // Recover readable keypairs: CWallet dummyWallet(&chain, WalletLocation(), WalletDatabase::CreateDummy()); std::string backup_filename; + // Even if we don't use this lock in this function, we want to preserve + // lock order in LoadToWallet if query of chain state is needed to know + // tx status. If lock can't be taken, tx confirmation status may be not + // reliable. + auto locked_chain = dummyWallet.LockChain(); if (!WalletBatch::Recover(wallet_path, (void *)&dummyWallet, WalletBatch::RecoverKeysOnlyFilter, backup_filename)) { return false; } @@ -4251,7 +4255,7 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, uint64_t wallet_creation_flags) { - const std::string& walletFile = WalletDataFilePath(location.GetPath()).string(); + const std::string walletFile = WalletDataFilePath(location.GetPath()).string(); // needed to restore wallet transaction meta data after -zapwallettxes std::vector<CWalletTx> vWtx; @@ -4395,23 +4399,23 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, } if (!gArgs.GetArg("-addresstype", "").empty() && !ParseOutputType(gArgs.GetArg("-addresstype", ""), walletInstance->m_default_address_type)) { - chain.initError(strprintf("Unknown address type '%s'", gArgs.GetArg("-addresstype", ""))); + chain.initError(strprintf(_("Unknown address type '%s'").translated, gArgs.GetArg("-addresstype", ""))); return nullptr; } if (!gArgs.GetArg("-changetype", "").empty() && !ParseOutputType(gArgs.GetArg("-changetype", ""), walletInstance->m_default_change_type)) { - chain.initError(strprintf("Unknown change type '%s'", gArgs.GetArg("-changetype", ""))); + chain.initError(strprintf(_("Unknown change type '%s'").translated, gArgs.GetArg("-changetype", ""))); return nullptr; } if (gArgs.IsArgSet("-mintxfee")) { CAmount n = 0; if (!ParseMoney(gArgs.GetArg("-mintxfee", ""), n) || 0 == n) { - chain.initError(AmountErrMsg("mintxfee", gArgs.GetArg("-mintxfee", ""))); + chain.initError(AmountErrMsg("mintxfee", gArgs.GetArg("-mintxfee", "")).translated); return nullptr; } if (n > HIGH_TX_FEE_PER_KB) { - chain.initWarning(AmountHighWarn("-mintxfee") + " " + + chain.initWarning(AmountHighWarn("-mintxfee").translated + " " + _("This is the minimum transaction fee you pay on every transaction.").translated); } walletInstance->m_min_fee = CFeeRate(n); @@ -4425,7 +4429,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, return nullptr; } if (nFeePerK > HIGH_TX_FEE_PER_KB) { - chain.initWarning(AmountHighWarn("-fallbackfee") + " " + + chain.initWarning(AmountHighWarn("-fallbackfee").translated + " " + _("This is the transaction fee you may pay when fee estimates are not available.").translated); } walletInstance->m_fallback_fee = CFeeRate(nFeePerK); @@ -4438,7 +4442,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, return nullptr; } if (nFeePerK > HIGH_TX_FEE_PER_KB) { - chain.initWarning(AmountHighWarn("-discardfee") + " " + + chain.initWarning(AmountHighWarn("-discardfee").translated + " " + _("This is the transaction fee you may discard if change is smaller than dust at this level").translated); } walletInstance->m_discard_rate = CFeeRate(nFeePerK); @@ -4446,11 +4450,11 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, if (gArgs.IsArgSet("-paytxfee")) { CAmount nFeePerK = 0; if (!ParseMoney(gArgs.GetArg("-paytxfee", ""), nFeePerK)) { - chain.initError(AmountErrMsg("paytxfee", gArgs.GetArg("-paytxfee", ""))); + chain.initError(AmountErrMsg("paytxfee", gArgs.GetArg("-paytxfee", "")).translated); return nullptr; } if (nFeePerK > HIGH_TX_FEE_PER_KB) { - chain.initWarning(AmountHighWarn("-paytxfee") + " " + + chain.initWarning(AmountHighWarn("-paytxfee").translated + " " + _("This is the transaction fee you will pay if you send a transaction.").translated); } walletInstance->m_pay_tx_fee = CFeeRate(nFeePerK, 1000); @@ -4465,7 +4469,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, { CAmount nMaxFee = 0; if (!ParseMoney(gArgs.GetArg("-maxtxfee", ""), nMaxFee)) { - chain.initError(AmountErrMsg("maxtxfee", gArgs.GetArg("-maxtxfee", ""))); + chain.initError(AmountErrMsg("maxtxfee", gArgs.GetArg("-maxtxfee", "")).translated); return nullptr; } if (nMaxFee > HIGH_MAX_TX_FEE) { @@ -4479,9 +4483,10 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, walletInstance->m_default_max_tx_fee = nMaxFee; } - if (chain.relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) - chain.initWarning(AmountHighWarn("-minrelaytxfee") + " " + + if (chain.relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) { + chain.initWarning(AmountHighWarn("-minrelaytxfee").translated + " " + _("The wallet will avoid paying less than the minimum relay fee.").translated); + } walletInstance->m_confirm_target = gArgs.GetArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET); walletInstance->m_spend_zero_conf_change = gArgs.GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE); @@ -4634,21 +4639,23 @@ CKeyPool::CKeyPool(const CPubKey& vchPubKeyIn, bool internalIn) m_pre_split = false; } -void CWalletTx::SetMerkleBranch(const uint256& block_hash, int posInBlock) +void CWalletTx::SetConf(Status status, const uint256& block_hash, int posInBlock) { + // Update tx status + m_confirm.status = status; + // Update the tx's hashBlock - hashBlock = block_hash; + m_confirm.hashBlock = block_hash; // set the position of the transaction in the block - nIndex = posInBlock; + m_confirm.nIndex = posInBlock; } int CWalletTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const { - if (hashUnset()) - return 0; + if (isUnconfirmed() || isAbandoned()) return 0; - return locked_chain.getBlockDepth(hashBlock) * (nIndex == -1 ? -1 : 1); + return locked_chain.getBlockDepth(m_confirm.hashBlock) * (isConflicted() ? -1 : 1); } int CWalletTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 984be3e301..3428e8e001 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -396,7 +396,9 @@ class CWalletTx private: const CWallet* pwallet; - /** Constant used in hashBlock to indicate tx has been abandoned */ + /** Constant used in hashBlock to indicate tx has been abandoned, only used at + * serialization/deserialization to avoid ambiguity with conflicted. + */ static const uint256 ABANDON_HASH; public: @@ -457,9 +459,7 @@ public: mutable CAmount nChangeCached; CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) - : tx(std::move(arg)), - hashBlock(uint256()), - nIndex(-1) + : tx(std::move(arg)) { Init(pwalletIn); } @@ -477,16 +477,37 @@ public: fInMempool = false; nChangeCached = 0; nOrderPos = -1; + m_confirm = Confirmation{}; } CTransactionRef tx; - uint256 hashBlock; - /* An nIndex == -1 means that hashBlock (in nonzero) refers to the earliest - * block in the chain we know this or any in-wallet dependency conflicts - * with. Older clients interpret nIndex == -1 as unconfirmed for backward - * compatibility. + + /* New transactions start as UNCONFIRMED. At BlockConnected, + * they will transition to CONFIRMED. In case of reorg, at BlockDisconnected, + * they roll back to UNCONFIRMED. If we detect a conflicting transaction at + * block connection, we update conflicted tx and its dependencies as CONFLICTED. + * If tx isn't confirmed and outside of mempool, the user may switch it to ABANDONED + * by using the abandontransaction call. This last status may be override by a CONFLICTED + * or CONFIRMED transition. + */ + enum Status { + UNCONFIRMED, + CONFIRMED, + CONFLICTED, + 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. */ - int nIndex; + struct Confirmation { + Status status = UNCONFIRMED; + uint256 hashBlock = uint256(); + int nIndex = 0; + }; + + Confirmation m_confirm; template<typename Stream> void Serialize(Stream& s) const @@ -502,7 +523,9 @@ public: std::vector<char> dummy_vector1; //!< Used to be vMerkleBranch std::vector<char> dummy_vector2; //!< Used to be vtxPrev bool dummy_bool = false; //!< Used to be fSpent - s << tx << hashBlock << dummy_vector1 << nIndex << dummy_vector2 << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << dummy_bool; + uint256 serializedHash = isAbandoned() ? ABANDON_HASH : m_confirm.hashBlock; + int serializedIndex = isAbandoned() || isConflicted() ? -1 : m_confirm.nIndex; + s << tx << serializedHash << dummy_vector1 << serializedIndex << dummy_vector2 << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << dummy_bool; } template<typename Stream> @@ -513,7 +536,25 @@ public: std::vector<uint256> dummy_vector1; //!< Used to be vMerkleBranch std::vector<CMerkleTx> dummy_vector2; //!< Used to be vtxPrev bool dummy_bool; //! Used to be fSpent - s >> tx >> hashBlock >> dummy_vector1 >> nIndex >> dummy_vector2 >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> dummy_bool; + int serializedIndex; + s >> tx >> m_confirm.hashBlock >> dummy_vector1 >> serializedIndex >> dummy_vector2 >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> dummy_bool; + + /* At serialization/deserialization, an nIndex == -1 means that hashBlock refers to + * the earliest block in the chain we know this or any in-wallet ancestor conflicts + * with. If nIndex == -1 and hashBlock is ABANDON_HASH, it means transaction is abandoned. + * In same context, an nIndex >= 0 refers to a confirmed transaction (if hashBlock set) or + * unconfirmed one. Older clients interpret nIndex == -1 as unconfirmed for backward + * compatibility (pre-commit 9ac63d6). + */ + if (serializedIndex == -1 && m_confirm.hashBlock == ABANDON_HASH) { + m_confirm.hashBlock = uint256(); + setAbandoned(); + } else if (serializedIndex == -1) { + setConflicted(); + } else if (!m_confirm.hashBlock.IsNull()) { + m_confirm.nIndex = serializedIndex; + setConfirmed(); + } ReadOrderPos(nOrderPos, mapValue); nTimeSmart = mapValue.count("timesmart") ? (unsigned int)atoi64(mapValue["timesmart"]) : 0; @@ -590,7 +631,7 @@ public: // in place. std::set<uint256> GetConflicts() const NO_THREAD_SAFETY_ANALYSIS; - void SetMerkleBranch(const uint256& block_hash, int posInBlock); + void SetConf(Status status, const uint256& block_hash, int posInBlock); /** * Return depth of transaction in blockchain: @@ -607,10 +648,18 @@ public: * >0 : is a coinbase transaction which matures in this many blocks */ int GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const; - bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); } - bool isAbandoned() const { return (hashBlock == ABANDON_HASH); } - void setAbandoned() { hashBlock = ABANDON_HASH; } - + bool isAbandoned() const { return m_confirm.status == CWalletTx::ABANDONED; } + void setAbandoned() + { + m_confirm.status = CWalletTx::ABANDONED; + m_confirm.hashBlock = uint256(); + 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; } + void setConfirmed() { m_confirm.status = CWalletTx::CONFIRMED; } const uint256& GetHash() const { return tx->GetHash(); } bool IsCoinBase() const { return tx->IsCoinBase(); } bool IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const; @@ -750,7 +799,7 @@ private: * Abandoned state should probably be more carefully tracked via different * posInBlock signals or by checking mempool presence when necessary. */ - bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const uint256& block_hash, int posInBlock, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Status status, const uint256& block_hash, int posInBlock, 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); @@ -762,7 +811,7 @@ private: /* Used by TransactionAddedToMemorypool/BlockConnected/Disconnected/ScanForWalletTransactions. * Should be called with non-zero block_hash and posInBlock if this is for a transaction that is included in a block. */ - void SyncTransaction(const CTransactionRef& tx, const uint256& block_hash, int posInBlock = 0, bool update_tx = true) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void SyncTransaction(const CTransactionRef& tx, CWalletTx::Status status, const uint256& block_hash, int posInBlock = 0, bool update_tx = true) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /* the HD chain data model (external chain counters) */ CHDChain hdChain; @@ -897,6 +946,9 @@ public: bool IsLocked() const; bool Lock(); + /** Interface to assert chain access and if successful lock it */ + std::unique_ptr<interfaces::Chain::Lock> LockChain() { return m_chain ? m_chain->lock() : nullptr; } + std::map<uint256, CWalletTx> mapWallet GUARDED_BY(cs_wallet); typedef std::multimap<int64_t, CWalletTx*> TxItems; @@ -1042,7 +1094,7 @@ public: void MarkDirty(); bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true); - void LoadToWallet(const CWalletTx& wtxIn) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void LoadToWallet(CWalletTx& wtxIn) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void TransactionAddedToMempool(const CTransactionRef& tx) override; void BlockConnected(const CBlock& block, const std::vector<CTransactionRef>& vtxConflicted) override; void BlockDisconnected(const CBlock& block) override; |