diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 30d93911bf97..16c2170cbc6d 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -907,3 +907,16 @@ A few guidelines for introducing and reviewing new RPC interfaces: - *Exception*: Using RPC method aliases may be appropriate in cases where a new RPC is replacing a deprecated RPC, to avoid both RPCs confusingly showing up in the command list. + +- Wallet RPCs call BlockUntilSyncedToCurrentChain to maintain consistency with + `getblockchaininfo`'s state immediately prior to the call's execution. Wallet + RPCs whose behavior does *not* depend on the current chainstate may omit this + call. + + - *Rationale*: In previous versions of Bitcoin Core, the wallet was always + in-sync with the chainstate (by virtue of them all being updated in the + same cs_main lock). In order to maintain the behavior that wallet RPCs + return results as of at least the highest best-known block an RPC + client may be aware of prior to entering a wallet RPC call, we must block + until the wallet is caught up to the chainstate as of the RPC call's entry. + This also makes the API much easier for RPC clients to reason about. diff --git a/src/blockassembler.cpp b/src/blockassembler.cpp index 4de1bc80462a..dd0d19db5cfa 100644 --- a/src/blockassembler.cpp +++ b/src/blockassembler.cpp @@ -70,6 +70,10 @@ bool SolveProofOfStake(CBlock* pblock, CBlockIndex* pindexPrev, CWallet* pwallet { boost::this_thread::interruption_point(); pblock->nBits = GetNextWorkRequired(pindexPrev, pblock); + + // Sync wallet before create coinstake + pwallet->BlockUntilSyncedToCurrentChain(); + CMutableTransaction txCoinStake; int64_t nTxNewTime = 0; if (!pwallet->CreateCoinStake(*pwallet, pindexPrev, pblock->nBits, txCoinStake, nTxNewTime, availableCoins)) { diff --git a/src/init.cpp b/src/init.cpp index db5a288358d8..3d922e203698 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -318,6 +318,7 @@ void PrepareShutdown() // Disconnect all slots UnregisterAllValidationInterfaces(); GetMainSignals().UnregisterBackgroundSignalScheduler(); + GetMainSignals().UnregisterWithMempoolSignals(mempool); #ifndef WIN32 try { @@ -1264,6 +1265,7 @@ bool AppInitMain() threadGroup.create_thread(std::bind(&TraceThread, "scheduler", serviceLoop)); GetMainSignals().RegisterBackgroundSignalScheduler(scheduler); + GetMainSignals().RegisterWithMempoolSignals(mempool); // Initialize Sapling circuit parameters LoadSaplingParams(); diff --git a/src/miner.cpp b/src/miner.cpp index bef8aba8715e..7e2e1499425c 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -39,10 +39,10 @@ double dHashesPerSec = 0.0; int64_t nHPSTimerStart = 0; -std::unique_ptr CreateNewBlockWithKey(CReserveKey& reservekey, CWallet* pwallet) +std::unique_ptr CreateNewBlockWithKey(CReserveKey* reservekey, CWallet* pwallet) { CPubKey pubkey; - if (!reservekey.GetReservedKey(pubkey)) + if (!reservekey->GetReservedKey(pubkey)) return nullptr; const int nHeightNext = chainActive.Tip()->nHeight + 1; @@ -169,7 +169,7 @@ void BitcoinMiner(CWallet* pwallet, bool fProofOfStake) std::unique_ptr pblocktemplate((fProofOfStake ? BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(CScript(), pwallet, true, &availableCoins) : - CreateNewBlockWithKey(*opReservekey, pwallet))); + CreateNewBlockWithKey(opReservekey.get_ptr(), pwallet))); if (!pblocktemplate) continue; std::shared_ptr pblock = std::make_shared(pblocktemplate->block); diff --git a/src/miner.h b/src/miner.h index 5755097ac189..c4feb04cfe09 100644 --- a/src/miner.h +++ b/src/miner.h @@ -27,7 +27,7 @@ static const bool DEFAULT_PRINTPRIORITY = false; /** Run the miner threads */ void GenerateBitcoins(bool fGenerate, CWallet* pwallet, int nThreads); /** Generate a new block, without valid proof-of-work */ - std::unique_ptr CreateNewBlockWithKey(CReserveKey& reservekey, CWallet* pwallet); + std::unique_ptr CreateNewBlockWithKey(CReserveKey* reservekey, CWallet* pwallet); void BitcoinMiner(CWallet* pwallet, bool fProofOfStake); void ThreadStakeMinter(); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f4a7209223e0..907cca1cab74 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -757,176 +757,215 @@ static void RelayAddress(const CAddress& addr, bool fReachable, CConnman& connma connman.ForEachNodeThen(std::move(sortfunc), std::move(pushfunc)); } -void static ProcessGetData(CNode* pfrom, CConnman& connman, std::atomic& interruptMsgProc) +bool static PushTierTwoGetDataRequest(const CInv& inv, + CNode* pfrom, + CConnman& connman, + CNetMsgMaker& msgMaker) { - AssertLockNotHeld(cs_main); + if (inv.type == MSG_SPORK) { + if (mapSporks.count(inv.hash)) { + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss.reserve(1000); + ss << mapSporks[inv.hash]; + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::SPORK, ss)); + return true; + } + } - std::deque::iterator it = pfrom->vRecvGetData.begin(); - std::vector vNotFound; - CNetMsgMaker msgMaker(pfrom->GetSendVersion()); - LOCK(cs_main); + if (inv.type == MSG_MASTERNODE_WINNER) { + if (masternodePayments.mapMasternodePayeeVotes.count(inv.hash)) { + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss.reserve(1000); + ss << masternodePayments.mapMasternodePayeeVotes[inv.hash]; + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNWINNER, ss)); + return true; + } + } - while (it != pfrom->vRecvGetData.end()) { - // Don't bother if send buffer is too full to respond anyway - if (pfrom->fPauseSend) - break; + if (inv.type == MSG_BUDGET_VOTE) { + if (g_budgetman.HaveSeenProposalVote(inv.hash)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BUDGETVOTE, g_budgetman.GetProposalVoteSerialized(inv.hash))); + return true; + } + } - const CInv& inv = *it; - { - if (interruptMsgProc) - return; - it++; + if (inv.type == MSG_BUDGET_PROPOSAL) { + if (g_budgetman.HaveProposal(inv.hash)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BUDGETPROPOSAL, g_budgetman.GetProposalSerialized(inv.hash))); + return true; + } + } - if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK) { - bool send = false; - BlockMap::iterator mi = mapBlockIndex.find(inv.hash); - if (mi != mapBlockIndex.end()) { - if (chainActive.Contains(mi->second)) { - send = true; - } else { - // To prevent fingerprinting attacks, only send blocks outside of the active - // chain if they are valid, and no more than a max reorg depth than the best header - // chain we know about. - send = mi->second->IsValid(BLOCK_VALID_SCRIPTS) && (pindexBestHeader != NULL) && - (chainActive.Height() - mi->second->nHeight < gArgs.GetArg("-maxreorg", DEFAULT_MAX_REORG_DEPTH)); - if (!send) { - LogPrint(BCLog::NET, "ProcessGetData(): ignoring request from peer=%i for old block that isn't in the main chain\n", pfrom->GetId()); - } - } - } - // Don't send not-validated blocks - if (send && (mi->second->nStatus & BLOCK_HAVE_DATA)) { - // Send block from disk - CBlock block; - if (!ReadBlockFromDisk(block, (*mi).second)) - assert(!"cannot load block from disk"); - if (inv.type == MSG_BLOCK) - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BLOCK, block)); - else // MSG_FILTERED_BLOCK) - { - bool send = false; - CMerkleBlock merkleBlock; - { - LOCK(pfrom->cs_filter); - if (pfrom->pfilter) { - send = true; - merkleBlock = CMerkleBlock(block, *pfrom->pfilter); - } - } - if (send) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock)); - // CMerkleBlock just contains hashes, so also push any transactions in the block the client did not see - // This avoids hurting performance by pointlessly requiring a round-trip - // Note that there is currently no way for a node to request any single transactions we didnt send here - - // they must either disconnect and retry or request the full block. - // Thus, the protocol spec specified allows for us to provide duplicate txn here, - // however we MUST always provide at least what the remote peer needs - for (std::pair& pair : merkleBlock.vMatchedTxn) - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *block.vtx[pair.first])); - } - // else - // no response - } + if (inv.type == MSG_BUDGET_FINALIZED_VOTE) { + if (g_budgetman.HaveSeenFinalizedBudgetVote(inv.hash)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::FINALBUDGETVOTE, g_budgetman.GetFinalizedBudgetVoteSerialized(inv.hash))); + return true; + } + } - // Trigger them to send a getblocks request for the next batch of inventory - if (inv.hash == pfrom->hashContinue) { - // Bypass PushInventory, this must send even if redundant, - // and we want it right after the last block so they don't - // wait for other stuff first. - std::vector vInv; - vInv.emplace_back(MSG_BLOCK, chainActive.Tip()->GetBlockHash()); - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::INV, vInv)); - pfrom->hashContinue.SetNull(); - } - } - } else if (inv.IsKnownType()) { - // Send stream from relay memory - bool pushed = false; - - if (inv.type == MSG_TX) { - auto txinfo = mempool.info(inv.hash); - if (txinfo.tx) { // future: add timeLastMempoolReq check - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss.reserve(1000); - ss << *txinfo.tx; - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, ss)); - pushed = true; - } - } + if (inv.type == MSG_BUDGET_FINALIZED) { + if (g_budgetman.HaveFinalizedBudget(inv.hash)) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::FINALBUDGET, g_budgetman.GetFinalizedBudgetSerialized(inv.hash))); + return true; + } + } - if (!pushed && inv.type == MSG_SPORK) { - if (mapSporks.count(inv.hash)) { - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss.reserve(1000); - ss << mapSporks[inv.hash]; - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::SPORK, ss)); - pushed = true; - } - } - if (!pushed && inv.type == MSG_MASTERNODE_WINNER) { - if (masternodePayments.mapMasternodePayeeVotes.count(inv.hash)) { - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss.reserve(1000); - ss << masternodePayments.mapMasternodePayeeVotes[inv.hash]; - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNWINNER, ss)); - pushed = true; - } - } - if (!pushed && inv.type == MSG_BUDGET_VOTE) { - if (g_budgetman.HaveSeenProposalVote(inv.hash)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BUDGETVOTE, g_budgetman.GetProposalVoteSerialized(inv.hash))); - pushed = true; - } - } + if (inv.type == MSG_MASTERNODE_ANNOUNCE) { + if (mnodeman.mapSeenMasternodeBroadcast.count(inv.hash)) { + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss.reserve(1000); + ss << mnodeman.mapSeenMasternodeBroadcast[inv.hash]; + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNBROADCAST, ss)); + return true; + } + } - if (!pushed && inv.type == MSG_BUDGET_PROPOSAL) { - if (g_budgetman.HaveProposal(inv.hash)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BUDGETPROPOSAL, g_budgetman.GetProposalSerialized(inv.hash))); - pushed = true; - } - } + if (inv.type == MSG_MASTERNODE_PING) { + if (mnodeman.mapSeenMasternodePing.count(inv.hash)) { + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss.reserve(1000); + ss << mnodeman.mapSeenMasternodePing[inv.hash]; + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNPING, ss)); + return true; + } + } - if (!pushed && inv.type == MSG_BUDGET_FINALIZED_VOTE) { - if (g_budgetman.HaveSeenFinalizedBudgetVote(inv.hash)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::FINALBUDGETVOTE, g_budgetman.GetFinalizedBudgetVoteSerialized(inv.hash))); - pushed = true; - } - } + // nothing was pushed. + return false; +} - if (!pushed && inv.type == MSG_BUDGET_FINALIZED) { - if (g_budgetman.HaveFinalizedBudget(inv.hash)) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::FINALBUDGET, g_budgetman.GetFinalizedBudgetSerialized(inv.hash))); - pushed = true; - } - } +void static ProcessGetBlockData(CNode* pfrom, const CInv& inv, CConnman& connman, const std::atomic& interruptMsgProc) +{ + LOCK(cs_main); + CNetMsgMaker msgMaker(pfrom->GetSendVersion()); - if (!pushed && inv.type == MSG_MASTERNODE_ANNOUNCE) { - if (mnodeman.mapSeenMasternodeBroadcast.count(inv.hash)) { - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss.reserve(1000); - ss << mnodeman.mapSeenMasternodeBroadcast[inv.hash]; - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNBROADCAST, ss)); - pushed = true; - } + bool send = false; + BlockMap::iterator mi = mapBlockIndex.find(inv.hash); + if (mi != mapBlockIndex.end()) { + if (chainActive.Contains(mi->second)) { + send = true; + } else { + // To prevent fingerprinting attacks, only send blocks outside of the active + // chain if they are valid, and no more than a max reorg depth than the best header + // chain we know about. + send = mi->second->IsValid(BLOCK_VALID_SCRIPTS) && (pindexBestHeader != NULL) && + (chainActive.Height() - mi->second->nHeight < gArgs.GetArg("-maxreorg", DEFAULT_MAX_REORG_DEPTH)); + if (!send) { + LogPrint(BCLog::NET, "ProcessGetData(): ignoring request from peer=%i for old block that isn't in the main chain\n", pfrom->GetId()); + } + } + } + // Don't send not-validated blocks + if (send && (mi->second->nStatus & BLOCK_HAVE_DATA)) { + // Send block from disk + CBlock block; + if (!ReadBlockFromDisk(block, (*mi).second)) + assert(!"cannot load block from disk"); + if (inv.type == MSG_BLOCK) + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::BLOCK, block)); + else // MSG_FILTERED_BLOCK) + { + bool send_ = false; + CMerkleBlock merkleBlock; + { + LOCK(pfrom->cs_filter); + if (pfrom->pfilter) { + send_ = true; + merkleBlock = CMerkleBlock(block, *pfrom->pfilter); } + } + if (send_) { + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock)); + // CMerkleBlock just contains hashes, so also push any transactions in the block the client did not see + // This avoids hurting performance by pointlessly requiring a round-trip + // Note that there is currently no way for a node to request any single transactions we didnt send here - + // they must either disconnect and retry or request the full block. + // Thus, the protocol spec specified allows for us to provide duplicate txn here, + // however we MUST always provide at least what the remote peer needs + for (std::pair& pair : merkleBlock.vMatchedTxn) + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *block.vtx[pair.first])); + } + // else + // no response + } - if (!pushed && inv.type == MSG_MASTERNODE_PING) { - if (mnodeman.mapSeenMasternodePing.count(inv.hash)) { - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss.reserve(1000); - ss << mnodeman.mapSeenMasternodePing[inv.hash]; - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNPING, ss)); - pushed = true; - } - } + // Trigger them to send a getblocks request for the next batch of inventory + if (inv.hash == pfrom->hashContinue) { + // Bypass PushInventory, this must send even if redundant, + // and we want it right after the last block so they don't + // wait for other stuff first. + std::vector vInv; + vInv.emplace_back(MSG_BLOCK, chainActive.Tip()->GetBlockHash()); + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::INV, vInv)); + pfrom->hashContinue.SetNull(); + } + } +} + +// Only return true if the inv type can be answered, not supported types return false. +bool static IsTierTwoInventoryTypeKnown(int type) +{ + return type == MSG_SPORK || + type == MSG_MASTERNODE_WINNER || + type == MSG_BUDGET_VOTE || + type == MSG_BUDGET_PROPOSAL || + type == MSG_BUDGET_FINALIZED || + type == MSG_BUDGET_FINALIZED_VOTE || + type == MSG_MASTERNODE_ANNOUNCE || + type == MSG_MASTERNODE_PING; +} + +void static ProcessGetData(CNode* pfrom, CConnman& connman, const std::atomic& interruptMsgProc) +{ + AssertLockNotHeld(cs_main); + + std::deque::iterator it = pfrom->vRecvGetData.begin(); + std::vector vNotFound; + CNetMsgMaker msgMaker(pfrom->GetSendVersion()); + { + LOCK(cs_main); - if (!pushed) { - vNotFound.push_back(inv); + while (it != pfrom->vRecvGetData.end() && (it->type == MSG_TX || IsTierTwoInventoryTypeKnown(it->type))) { + if (interruptMsgProc) + return; + // Don't bother if send buffer is too full to respond anyway + if (pfrom->fPauseSend) + break; + + const CInv &inv = *it; + it++; + + // Send stream from relay memory + bool pushed = false; + if (inv.type == MSG_TX) { + auto txinfo = mempool.info(inv.hash); + if (txinfo.tx) { // future: add timeLastMempoolReq check + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss.reserve(1000); + ss << *txinfo.tx; + connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, ss)); + pushed = true; } } - if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK) - break; + if (!pushed) { + // Now check if it's a tier two data request and push it. + pushed = PushTierTwoGetDataRequest(inv, pfrom, connman, msgMaker); + } + + if (!pushed) { + vNotFound.push_back(inv); + } + + // todo: inventory signal + } + } // release cs_main + + if (it != pfrom->vRecvGetData.end()) { + const CInv &inv = *it; + it++; + if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK) { + ProcessGetBlockData(pfrom, inv, connman, interruptMsgProc); } } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 704038da8ec9..632bd84322b9 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -21,6 +21,7 @@ #include "utilmoneystr.h" #include "utilstrencodings.h" #include "hash.h" +#include "validationinterface.h" #include "wallet/wallet.h" #include "zpiv/zpivmodule.h" #include "zpivchain.h" @@ -48,6 +49,21 @@ static CUpdatedBlock latestblock; extern void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry); void ScriptPubKeyToJSON(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex); +UniValue syncwithvalidationinterfacequeue(const JSONRPCRequest& request) +{ + if (request.fHelp || request.params.size() > 0) { + throw std::runtime_error( + "syncwithvalidationinterfacequeue\n" + "\nWaits for the validation interface queue to catch up on everything that was there when we entered this function.\n" + "\nExamples:\n" + + HelpExampleCli("syncwithvalidationinterfacequeue","") + + HelpExampleRpc("syncwithvalidationinterfacequeue","") + ); + } + SyncWithValidationInterfaceQueue(); + return NullUniValue; +} + double GetDifficulty(const CBlockIndex* blockindex) { // Floating point number that is a multiple of the minimum difficulty, @@ -1448,6 +1464,7 @@ static const CRPCCommand commands[] = { "hidden", "waitfornewblock", &waitfornewblock, true }, { "hidden", "waitforblock", &waitforblock, true }, { "hidden", "waitforblockheight", &waitforblockheight, true }, + { "hidden", "syncwithvalidationinterfacequeue", &syncwithvalidationinterfacequeue, true }, }; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 9c8ad36bf38d..d1f2ce630ed8 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -61,14 +61,17 @@ UniValue generate(const JSONRPCRequest& request) const Consensus::Params& consensus = Params().GetConsensus(); bool fPoS = consensus.NetworkUpgradeActive(nHeight + 1, Consensus::UPGRADE_POS); + std::unique_ptr reservekey; if (fPoS) { // If we are in PoS, wallet must be unlocked. EnsureWalletIsUnlocked(); + } else { + // Coinbase key + reservekey = MakeUnique(pwalletMain); } UniValue blockHashes(UniValue::VARR); - CReserveKey reservekey(pwalletMain); unsigned int nExtraNonce = 0; while (nHeight < nHeightEnd && !ShutdownRequested()) { @@ -81,7 +84,7 @@ UniValue generate(const JSONRPCRequest& request) std::unique_ptr pblocktemplate((fPoS ? BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(CScript(), pwalletMain, true, &availableCoins) : - CreateNewBlockWithKey(reservekey, pwalletMain))); + CreateNewBlockWithKey(reservekey.get(), pwalletMain))); if (!pblocktemplate.get()) break; std::shared_ptr pblock = std::make_shared(pblocktemplate->block); @@ -114,6 +117,12 @@ UniValue generate(const JSONRPCRequest& request) if (nGenerated == 0 || (!fPoS && nGenerated < nGenerate)) throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new blocks"); + // mark key as used, only for PoW coinbases + if (reservekey) { + // Remove key from key pool + reservekey->KeepKey(); + } + return blockHashes; } #endif // ENABLE_WALLET diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 1afdabe60772..2ccb6c3dddd4 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -9,6 +9,7 @@ #include "core_io.h" #include "init.h" #include "keystore.h" +#include "validationinterface.h" #include "net.h" #include "policy/policy.h" #include "primitives/transaction.h" @@ -25,6 +26,7 @@ #include "wallet/wallet.h" #endif +#include #include #include @@ -518,6 +520,10 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) if (!pwalletMain) throw std::runtime_error("wallet not initialized"); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + RPCTypeCheck(request.params, {UniValue::VSTR}); CTxDestination changeAddress = CNoDestination(); @@ -899,6 +905,8 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) "\nSend the transaction (signed hex)\n" + HelpExampleCli("sendrawtransaction", "\"signedhex\"") + "\nAs a json rpc call\n" + HelpExampleRpc("sendrawtransaction", "\"signedhex\"")); + std::promise promise; + RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VBOOL}); // parse hex string from parameter @@ -911,7 +919,8 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) if (request.params.size() > 1) fOverrideFees = request.params[1].get_bool(); - AssertLockNotHeld(cs_main); + { // cs_main scope + LOCK(cs_main); CCoinsViewCache& view = *pcoinsTip; bool fHaveChain = false; for (size_t o = 0; !fHaveChain && o < mtx.vout.size(); o++) { @@ -923,7 +932,6 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) CValidationState state; bool fMissingInputs; { - LOCK(cs_main); if (!AcceptToMemoryPool(mempool, state, MakeTransactionRef(std::move(mtx)), true, &fMissingInputs, false, !fOverrideFees)) { if (state.IsInvalid()) { throw JSONRPCError(RPC_TRANSACTION_REJECTED, strprintf("%i: %s", state.GetRejectCode(), state.GetRejectReason())); @@ -933,11 +941,25 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) } throw JSONRPCError(RPC_TRANSACTION_ERROR, state.GetRejectReason()); } + } else { + // If wallet is enabled, ensure that the wallet has been made aware + // of the new transaction prior to returning. This prevents a race + // where a user might call sendrawtransaction with a transaction + // to/from their wallet, immediately call some wallet RPC, and get + // a stale result because callbacks have not yet been processed. + CallFunctionInValidationInterfaceQueue([&promise] { + promise.set_value(); + }); } } } else if (fHaveChain) { throw JSONRPCError(RPC_TRANSACTION_ALREADY_IN_CHAIN, "transaction already in block chain"); } + + } // cs_main + + promise.get_future().wait(); + if(!g_connman) throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); @@ -946,6 +968,7 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) { pnode->PushInventory(inv); }); + return hashTx.GetHex(); } diff --git a/src/sapling/saplingscriptpubkeyman.cpp b/src/sapling/saplingscriptpubkeyman.cpp index 3d1955b93ecf..decc1753e4d5 100644 --- a/src/sapling/saplingscriptpubkeyman.cpp +++ b/src/sapling/saplingscriptpubkeyman.cpp @@ -760,7 +760,7 @@ CAmount SaplingScriptPubKeyMan::GetShieldedChange(const CWalletTx& wtx) const bool SaplingScriptPubKeyMan::IsNoteSaplingChange(const SaplingOutPoint& op, libzcash::SaplingPaymentAddress address) const { - LOCK(wallet->cs_KeyStore); + LOCK2(wallet->cs_wallet, wallet->cs_KeyStore); std::set shieldedAddresses = {address}; std::set> nullifierSet = GetNullifiersForAddresses(shieldedAddresses); return IsNoteSaplingChange(nullifierSet, address, op); diff --git a/src/scheduler.cpp b/src/scheduler.cpp index 0232ad2794ef..86c08510ce45 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -186,3 +186,8 @@ void SingleThreadedSchedulerClient::EmptyQueue() { should_continue = !m_callbacks_pending.empty(); } } + +size_t SingleThreadedSchedulerClient::CallbacksPending() { + LOCK(m_cs_callbacks_pending); + return m_callbacks_pending.size(); +} diff --git a/src/scheduler.h b/src/scheduler.h index 71ca40f4fb83..781bd52ea56f 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -109,6 +109,8 @@ class SingleThreadedSchedulerClient { // Processes all remaining queue members on the calling thread, blocking until queue is empty // Must be called after the CScheduler has no remaining processing threads! void EmptyQueue(); + + size_t CallbacksPending(); }; #endif diff --git a/src/test/librust/sapling_rpc_wallet_tests.cpp b/src/test/librust/sapling_rpc_wallet_tests.cpp index f6e8e0bca251..b08cc8b8735c 100644 --- a/src/test/librust/sapling_rpc_wallet_tests.cpp +++ b/src/test/librust/sapling_rpc_wallet_tests.cpp @@ -87,7 +87,11 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_getbalance) { SelectParams(CBaseChainParams::TESTNET); - LOCK2(cs_main, pwalletMain->cs_wallet); + { + LOCK(pwalletMain->cs_wallet); + pwalletMain->SetMinVersion(FEATURE_SAPLING); + pwalletMain->SetupSPKM(false); + } BOOST_CHECK_THROW(CallRPC("getshieldbalance too many args"), std::runtime_error); BOOST_CHECK_THROW(CallRPC("getshieldbalance invalidaddress"), std::runtime_error); @@ -249,7 +253,11 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_parameters) { SelectParams(CBaseChainParams::TESTNET); - LOCK2(cs_main, pwalletMain->cs_wallet); + { + LOCK(pwalletMain->cs_wallet); + pwalletMain->SetMinVersion(FEATURE_SAPLING); + pwalletMain->SetupSPKM(false); + } BOOST_CHECK_THROW(CallRPC("shieldsendmany"), std::runtime_error); BOOST_CHECK_THROW(CallRPC("shieldsendmany toofewargs"), std::runtime_error); @@ -299,7 +307,6 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_parameters) std::vector v (2 * (ZC_MEMO_SIZE+1)); // x2 for hexadecimal string format std::fill(v.begin(),v.end(), 'A'); std::string badmemo(v.begin(), v.end()); - pwalletMain->SetupSPKM(false); auto pa = pwalletMain->GenerateNewSaplingZKey(); std::string zaddr1 = KeyIO::EncodePaymentAddress(pa); BOOST_CHECK_THROW(CallRPC(std::string("shieldsendmany yBYhwgzufrZ6F5VVuK9nEChENArq934mqC ") @@ -543,8 +550,10 @@ BOOST_AUTO_TEST_CASE(rpc_listshieldunspent_parameters) { SelectParams(CBaseChainParams::TESTNET); - LOCK2(cs_main, pwalletMain->cs_wallet); - pwalletMain->SetupSPKM(false); + { + LOCK(pwalletMain->cs_wallet); + pwalletMain->SetupSPKM(false); + } UniValue retValue; diff --git a/src/test/test_pivx.cpp b/src/test/test_pivx.cpp index 8537114b1285..d1aa0e6baca5 100644 --- a/src/test/test_pivx.cpp +++ b/src/test/test_pivx.cpp @@ -54,10 +54,15 @@ TestingSetup::TestingSetup() fs::create_directories(pathTemp); gArgs.ForceSetArg("-datadir", pathTemp.string()); + // Start the lightweight task scheduler thread + CScheduler::Function serviceLoop = std::bind(&CScheduler::serviceQueue, &scheduler); + threadGroup.create_thread(std::bind(&TraceThread, "scheduler", serviceLoop)); + // Note that because we don't bother running a scheduler thread here, // callbacks via CValidationInterface are unreliable, but that's OK, // our unit tests aren't testing multiple parts of the code at once. GetMainSignals().RegisterBackgroundSignalScheduler(scheduler); + GetMainSignals().RegisterWithMempoolSignals(mempool); // Ideally we'd move all the RPC tests to the functional testing framework // instead of unit tests, but for now we need these here. @@ -87,7 +92,9 @@ TestingSetup::~TestingSetup() threadGroup.interrupt_all(); threadGroup.join_all(); GetMainSignals().FlushBackgroundCallbacks(); + UnregisterAllValidationInterfaces(); GetMainSignals().UnregisterBackgroundSignalScheduler(); + GetMainSignals().UnregisterWithMempoolSignals(mempool); UnloadBlockIndex(); delete pcoinsTip; delete pcoinsdbview; diff --git a/src/txmempool.h b/src/txmempool.h index 6e29cd796507..635606d2aa97 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -535,6 +535,9 @@ class CTxMemPool // to track size/count of descendant transactions. First version of // addUnchecked can be used to have it call CalculateMemPoolAncestors(), and // then invoke the second version. + // Note that addUnchecked is ONLY called from ATMP outside of tests + // and any other callers may break wallet's in-mempool tracking (due to + // lack of CValidationInterface::TransactionAddedToMempool callbacks). bool addUnchecked(const uint256& hash, const CTxMemPoolEntry& entry, bool fCurrentEstimate = true); bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true); diff --git a/src/validation.cpp b/src/validation.cpp index bdb5e25bb0f1..82a8208dc541 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -53,6 +53,8 @@ #include "zpiv/zerocoin.h" #include "zpiv/zpivmodule.h" +#include + #include #include #include @@ -1897,7 +1899,7 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara LogPrint(BCLog::BENCH, "- Disconnect block: %.2fms\n", (GetTimeMicros() - nStart) * 0.001); const uint256& saplingAnchorAfterDisconnect = pcoinsTip->GetBestAnchor(); // Write the chain state to disk, if necessary. - if (!FlushStateToDisk(state, FLUSH_STATE_ALWAYS)) + if (!FlushStateToDisk(state, FLUSH_STATE_IF_NEEDED)) return false; // Resurrect mempool transactions from the disconnected block. std::vector vHashUpdate; @@ -2258,6 +2260,13 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb do { boost::this_thread::interruption_point(); + if (GetMainSignals().CallbacksPending() > 10) { + // Block until the validation queue drains. This should largely + // never happen in normal operation, however may happen during + // reindex, causing memory blowup if we run too far ahead. + SyncWithValidationInterfaceQueue(); + } + const CBlockIndex *pindexFork; bool fInitialDownload; while (true) { @@ -2285,7 +2294,7 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb for (const PerBlockConnectTrace& trace : connectTrace.GetBlocksConnected()) { assert(trace.pblock && trace.pindex); - GetMainSignals().BlockConnected(trace.pblock, trace.pindex, *trace.conflictedTxs); + GetMainSignals().BlockConnected(trace.pblock, trace.pindex, trace.conflictedTxs); } break; @@ -2762,33 +2771,10 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo return state.Invalid(false, state.GetRejectCode(), state.GetRejectReason(), strprintf("Transaction check failed (tx hash %s) %s", tx.GetHash().ToString(), state.GetDebugMessage())); - // double check that there are no double spent zPIV spends in this block - if (tx.HasZerocoinSpendInputs()) { - for (const CTxIn& txIn : tx.vin) { - bool isPublicSpend = txIn.IsZerocoinPublicSpend(); - if (txIn.IsZerocoinSpend() || isPublicSpend) { - libzerocoin::CoinSpend spend; - if (isPublicSpend) { - libzerocoin::ZerocoinParams* params = Params().GetConsensus().Zerocoin_Params(false); - PublicCoinSpend publicSpend(params); - if (!ZPIVModule::ParseZerocoinPublicSpend(txIn, tx, state, publicSpend)){ - return false; - } - spend = publicSpend; - // check that the version matches the one enforced with SPORK_18 (don't ban if it fails) - if (!IsInitialBlockDownload() && !CheckPublicCoinSpendVersion(spend.getVersion())) { - return state.DoS(0, error("%s : Public Zerocoin spend version %d not accepted. must be version %d.", - __func__, spend.getVersion(), CurrentPublicCoinSpendVersion()), REJECT_INVALID, "bad-zcspend-version"); - } - } else { - spend = TxInToZerocoinSpend(txIn); - } - if (std::count(vBlockSerials.begin(), vBlockSerials.end(), spend.getCoinSerialNumber())) - return state.DoS(100, error("%s : Double spending of zPIV serial %s in block\n Block: %s", - __func__, spend.getCoinSerialNumber().GetHex(), block.ToString())); - vBlockSerials.emplace_back(spend.getCoinSerialNumber()); - } - } + // No need to check for zerocoin anymore, they are networkely disabled + // and checkpoints are preventing the chain for any massive reorganization. + if (fSaplingActive && tx.ContainsZerocoins()) { + return state.DoS(100, error("%s : v5 upgrade enforced, zerocoin disabled", __func__)); } } @@ -4170,7 +4156,7 @@ void DumpMempool(void) { LOCK(mempool.cs); for (const auto &i : mempool.mapDeltas) { - mapDeltas[i.first] = i.second.first; + mapDeltas[i.first] = i.second.second; } vinfo = mempool.infoAll(); } @@ -4199,7 +4185,9 @@ void DumpMempool(void) file << mapDeltas; FileCommit(file.Get()); file.fclose(); - RenameOver(GetDataDir() / "mempool.dat.new", GetDataDir() / "mempool.dat"); + if (!RenameOver(GetDataDir() / "mempool.dat.new", GetDataDir() / "mempool.dat")) { + throw std::runtime_error("Rename failed"); + } int64_t last = GetTimeMicros(); LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", (mid-start)*0.000001, (last-mid)*0.000001); } catch (const std::exception& e) { diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index d38e04bf1f42..ad4af45dd7b1 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -6,7 +6,10 @@ #include "validationinterface.h" #include "scheduler.h" +#include "txmempool.h" +#include "validation.h" +#include #include #include #include @@ -16,6 +19,7 @@ struct ValidationInterfaceConnections { boost::signals2::scoped_connection TransactionAddedToMempool; boost::signals2::scoped_connection BlockConnected; boost::signals2::scoped_connection BlockDisconnected; + boost::signals2::scoped_connection TransactionRemovedFromMempool; boost::signals2::scoped_connection SetBestChain; boost::signals2::scoped_connection Broadcast; boost::signals2::scoped_connection BlockChecked; @@ -34,6 +38,8 @@ struct MainSignalsInstance { boost::signals2::signal &, const CBlockIndex *pindex, const std::vector &)> BlockConnected; /** Notifies listeners of a block being disconnected */ boost::signals2::signal &, int nBlockHeight)> BlockDisconnected; + /** Notifies listeners of a transaction removal from the mempool */ + boost::signals2::signal TransactionRemovedFromMempool; /** Notifies listeners of a new active block chain. */ boost::signals2::signal SetBestChain; /** Tells listeners to broadcast their data. */ @@ -68,6 +74,19 @@ void CMainSignals::FlushBackgroundCallbacks() { } } +size_t CMainSignals::CallbacksPending() { + if (!m_internals) return 0; + return m_internals->m_schedulerClient.CallbacksPending(); +} + +void CMainSignals::RegisterWithMempoolSignals(CTxMemPool& pool) { + pool.NotifyEntryRemoved.connect(std::bind(&CMainSignals::MempoolEntryRemoved, this, std::placeholders::_1, std::placeholders::_2)); +} + +void CMainSignals::UnregisterWithMempoolSignals(CTxMemPool& pool) { + pool.NotifyEntryRemoved.disconnect_all_slots(); +} + CMainSignals& GetMainSignals() { return g_signals; @@ -80,6 +99,7 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) conns.TransactionAddedToMempool = g_signals.m_internals->TransactionAddedToMempool.connect(std::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, std::placeholders::_1)); conns.BlockConnected = g_signals.m_internals->BlockConnected.connect(std::bind(&CValidationInterface::BlockConnected, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); conns.BlockDisconnected = g_signals.m_internals->BlockDisconnected.connect(std::bind(&CValidationInterface::BlockDisconnected, pwalletIn, std::placeholders::_1, std::placeholders::_2)); + conns.TransactionRemovedFromMempool = g_signals.m_internals->TransactionRemovedFromMempool.connect(std::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, std::placeholders::_1)); conns.SetBestChain = g_signals.m_internals->SetBestChain.connect(std::bind(&CValidationInterface::SetBestChain, pwalletIn, std::placeholders::_1)); conns.Broadcast = g_signals.m_internals->Broadcast.connect(std::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, std::placeholders::_1)); conns.BlockChecked = g_signals.m_internals->BlockChecked.connect(std::bind(&CValidationInterface::BlockChecked, pwalletIn, std::placeholders::_1, std::placeholders::_2)); @@ -100,24 +120,59 @@ void UnregisterAllValidationInterfaces() g_signals.m_internals->m_connMainSignals.clear(); } +void CallFunctionInValidationInterfaceQueue(std::function func) { + g_signals.m_internals->m_schedulerClient.AddToProcessQueue(std::move(func)); +} + +void SyncWithValidationInterfaceQueue() { + AssertLockNotHeld(cs_main); + // if queue is empty, do not wait for nothing.s + if (g_signals.CallbacksPending() == 0) return; + + // Block until the validation queue drains + std::promise promise; + CallFunctionInValidationInterfaceQueue([&promise] { + promise.set_value(); + }); + promise.get_future().wait(); +} + +void CMainSignals::MempoolEntryRemoved(CTransactionRef ptx, MemPoolRemovalReason reason) { + if (reason != MemPoolRemovalReason::BLOCK && reason != MemPoolRemovalReason::CONFLICT) { + m_internals->m_schedulerClient.AddToProcessQueue([ptx, this] { + m_internals->TransactionRemovedFromMempool(ptx); + }); + } +} + void CMainSignals::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) { - m_internals->UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); + m_internals->m_schedulerClient.AddToProcessQueue([pindexNew, pindexFork, fInitialDownload, this] { + m_internals->UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); + }); } -void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptxn) { - m_internals->TransactionAddedToMempool(ptxn); +void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptx) { + m_internals->m_schedulerClient.AddToProcessQueue([ptx, this] { + m_internals->TransactionAddedToMempool(ptx); + }); } -void CMainSignals::BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex, const std::vector &txnConflicted) { - m_internals->BlockConnected(block, pindex, txnConflicted); +void CMainSignals::BlockConnected(const std::shared_ptr &pblock, const CBlockIndex *pindex, const std::shared_ptr>& pvtxConflicted) { + m_internals->m_schedulerClient.AddToProcessQueue([pblock, pindex, pvtxConflicted, this] { + m_internals->BlockConnected(pblock, pindex, *pvtxConflicted); + }); } -void CMainSignals::BlockDisconnected(const std::shared_ptr &block, int nBlockHeight) { - m_internals->BlockDisconnected(block, nBlockHeight); +void CMainSignals::BlockDisconnected(const std::shared_ptr &pblock, int nBlockHeight) { + m_internals->m_schedulerClient.AddToProcessQueue([pblock, nBlockHeight, this] { + m_internals->BlockDisconnected(pblock, nBlockHeight); + }); } -void CMainSignals::SetBestChain(const CBlockLocator& locator) { - m_internals->SetBestChain(locator); +void CMainSignals::SetBestChain(const CBlockLocator &locator) { + m_internals->m_schedulerClient.AddToProcessQueue([locator, this] { + m_internals->SetBestChain(locator); + }); } void CMainSignals::Broadcast(CConnman* connman) { diff --git a/src/validationinterface.h b/src/validationinterface.h index 0a4343070426..7377e13ae286 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -11,6 +11,9 @@ #include "sapling/incrementalmerkletree.h" #include "primitives/transaction.h" +#include +#include + class CBlock; struct CBlockLocator; class CBlockIndex; @@ -19,6 +22,8 @@ class CValidationInterface; class CValidationState; class uint256; class CScheduler; +class CTxMemPool; +enum class MemPoolRemovalReason; // These functions dispatch to one or all registered wallets @@ -28,17 +33,72 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn); void UnregisterValidationInterface(CValidationInterface* pwalletIn); /** Unregister all wallets from core */ void UnregisterAllValidationInterfaces(); +/** + * Pushes a function to callback onto the notification queue, guaranteeing any + * callbacks generated prior to now are finished when the function is called. + * + * Be very careful blocking on func to be called if any locks are held - + * validation interface clients may not be able to make progress as they often + * wait for things like cs_main, so blocking until func is called with cs_main + * will result in a deadlock (that DEBUG_LOCKORDER will miss). + */ +void CallFunctionInValidationInterfaceQueue(std::function func); +/** + * This is a synonym for the following, which asserts certain locks are not + * held: + * std::promise promise; + * CallFunctionInValidationInterfaceQueue([&promise] { + * promise.set_value(); + * }); + * promise.get_future().wait(); + */ +void SyncWithValidationInterfaceQueue(); class CValidationInterface { public: virtual ~CValidationInterface() = default; protected: - /** Notifies listeners of updated block chain tip */ + /** + * Notifies listeners of updated block chain tip + * + * Called on a background thread. + */ virtual void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) {} + /** + * Notifies listeners of a transaction having been added to mempool. + * + * Called on a background thread. + */ virtual void TransactionAddedToMempool(const CTransactionRef &ptxn) {} + /** + * Notifies listeners of a transaction leaving mempool. + * + * This only fires for transactions which leave mempool because of expiry, + * size limiting, reorg (changes in lock times/coinbase/coinstake maturity), or + * replacement. This does not include any transactions which are included + * in BlockConnectedDisconnected either in block->vtx or in txnConflicted. + * + * Called on a background thread. + */ + virtual void TransactionRemovedFromMempool(const CTransactionRef &ptx) {} + /** + * Notifies listeners of a block being connected. + * Provides a vector of transactions evicted from the mempool as a result. + * + * Called on a background thread. + */ virtual void BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex, const std::vector &txnConflicted) {} + /** + * Notifies listeners of a block being disconnected + * + * Called on a background thread. + */ virtual void BlockDisconnected(const std::shared_ptr &block, int nBlockHeight) {} - /** Notifies listeners of the new active block chain on-disk. */ + /** + * Notifies listeners of the new active block chain on-disk. + * + * Called on a background thread. + */ virtual void SetBestChain(const CBlockLocator &locator) {} /** Tells listeners to broadcast their data. */ virtual void ResendWalletTransactions(CConnman* connman) {} @@ -53,9 +113,12 @@ class CMainSignals { private: std::unique_ptr m_internals; + void MempoolEntryRemoved(CTransactionRef tx, MemPoolRemovalReason reason); + friend void ::RegisterValidationInterface(CValidationInterface*); friend void ::UnregisterValidationInterface(CValidationInterface*); friend void ::UnregisterAllValidationInterfaces(); + friend void ::CallFunctionInValidationInterfaceQueue(std::function func); public: /** Register a CScheduler to give callbacks which should run in the background (may only be called once) */ @@ -65,9 +128,16 @@ class CMainSignals { /** Call any remaining callbacks on the calling thread */ void FlushBackgroundCallbacks(); + size_t CallbacksPending(); + + /** Register with mempool to call TransactionRemovedFromMempool callbacks */ + void RegisterWithMempoolSignals(CTxMemPool& pool); + /** Unregister with mempool */ + void UnregisterWithMempoolSignals(CTxMemPool& pool); + void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload); void TransactionAddedToMempool(const CTransactionRef &ptxn); - void BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex, const std::vector &txnConflicted); + void BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex, const std::shared_ptr> &); void BlockDisconnected(const std::shared_ptr &block, int nBlockHeight); void SetBestChain(const CBlockLocator &); void Broadcast(CConnman* connman); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index f5dced6e313f..63ece49de707 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -468,6 +468,10 @@ UniValue dumpwallet(const JSONRPCRequest& request) throw JSONRPCError(RPC_MISC_ERROR, "Scam attempt detected!"); } + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); EnsureWalletIsUnlocked(); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 2b5dd3355693..fbf1c98b4a31 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -34,8 +34,6 @@ #include "zpiv/deterministicmint.h" #include -#include - int64_t nWalletUnlockTime; static RecursiveMutex cs_nWalletUnlockTime; @@ -440,6 +438,10 @@ UniValue sethdseed(const JSONRPCRequest& request) throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Cannot set a new HD seed while still in Initial Block Download"); } + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwallet->cs_wallet); // Do not do anything to non-HD wallets @@ -1033,6 +1035,10 @@ UniValue sendtoaddress(const JSONRPCRequest& request) EnsureWalletIsUnlocked(); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + bool isStaking = false, isShielded = false; const std::string addrStr = request.params[0].get_str(); const CWDestination& destination = Standard::DecodeDestination(addrStr, isStaking, isShielded); @@ -1209,6 +1215,10 @@ UniValue delegatestake(const JSONRPCRequest& request) HelpExampleCli("delegatestake", "\"S1t2a3kab9c8c71VA78xxxy4MxZg6vgeS6\" 1000 \"DMJRSsuU9zfyrvxVaAEFQqK4MxZg34fk\"") + HelpExampleRpc("delegatestake", "\"S1t2a3kab9c8c71VA78xxxy4MxZg6vgeS6\", 1000, \"DMJRSsuU9zfyrvxVaAEFQqK4MxZg34fk\"")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); CTransactionRef wtx; @@ -1251,6 +1261,10 @@ UniValue rawdelegatestake(const JSONRPCRequest& request) HelpExampleCli("rawdelegatestake", "\"S1t2a3kab9c8c71VA78xxxy4MxZg6vgeS6\" 1000 \"DMJRSsuU9zfyrvxVaAEFQqK4MxZg34fk\"") + HelpExampleRpc("rawdelegatestake", "\"S1t2a3kab9c8c71VA78xxxy4MxZg6vgeS6\", 1000, \"DMJRSsuU9zfyrvxVaAEFQqK4MxZg34fk\"")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); CTransactionRef wtx; @@ -1375,6 +1389,11 @@ UniValue viewshieldtransaction(const JSONRPCRequest& request) } EnsureWalletIsUnlocked(); + + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); uint256 hash; @@ -1687,6 +1706,10 @@ UniValue shieldsendmany(const JSONRPCRequest& request) "\"DMJRSsuU9zfyrvxVaAEFQqK4MxZg6vgeS6\", [{\"address\": \"ps1ra969yfhvhp73rw5ak2xvtcm9fkuqsnmad7qln79mphhdrst3lwu9vvv03yuyqlh42p42st47qd\" ,\"amount\": 5.0}]") ); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + SaplingOperation operation = CreateShieldedTransaction(request); std::string txHash; auto res = operation.send(txHash); @@ -1731,6 +1754,10 @@ UniValue rawshieldsendmany(const JSONRPCRequest& request) "\"DMJRSsuU9zfyrvxVaAEFQqK4MxZg6vgeS6\", [{\"address\": \"ps1ra969yfhvhp73rw5ak2xvtcm9fkuqsnmad7qln79mphhdrst3lwu9vvv03yuyqlh42p42st47qd\" ,\"amount\": 5.0}]") ); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + CTransaction tx = CreateShieldedTransaction(request).getFinalTx(); return EncodeHexTx(tx); } @@ -1760,6 +1787,10 @@ UniValue listaddressgroupings(const JSONRPCRequest& request) "\nExamples:\n" + HelpExampleCli("listaddressgroupings", "") + HelpExampleRpc("listaddressgroupings", "")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); UniValue jsonGroupings(UniValue::VARR); @@ -1860,6 +1891,10 @@ UniValue getreceivedbyaddress(const JSONRPCRequest& request) "\nAs a json rpc call\n" + HelpExampleRpc("getreceivedbyaddress", "\"DMJRSsuU9zfyrvxVaAEFQqK4MxZg6vgeS6\", 6")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); // pivx address @@ -1916,6 +1951,10 @@ UniValue getreceivedbylabel(const JSONRPCRequest& request) "\nAs a json rpc call\n" + HelpExampleRpc("getreceivedbylabel", "\"tabby\", 6")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); // Minimum confirmations @@ -1971,6 +2010,10 @@ UniValue getbalance(const JSONRPCRequest& request) "\nAs a json rpc call\n" + HelpExampleRpc("getbalance", "6")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); const int paramsSize = request.params.size(); @@ -2002,6 +2045,10 @@ UniValue getcoldstakingbalance(const JSONRPCRequest& request) "\nAs a json rpc call\n" + HelpExampleRpc("getcoldstakingbalance", "\"*\"")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); return ValueFromAmount(pwalletMain->GetColdStakingBalance()); @@ -2024,6 +2071,10 @@ UniValue getdelegatedbalance(const JSONRPCRequest& request) "\nAs a json rpc call\n" + HelpExampleRpc("getdelegatedbalance", "\"*\"")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); return ValueFromAmount(pwalletMain->GetDelegatedBalance()); @@ -2036,6 +2087,10 @@ UniValue getunconfirmedbalance(const JSONRPCRequest& request) "getunconfirmedbalance\n" "Returns the server's total unconfirmed balance\n"); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); return ValueFromAmount(pwalletMain->GetUnconfirmedBalance()); @@ -2156,6 +2211,10 @@ UniValue sendmany(const JSONRPCRequest& request) EnsureWalletIsUnlocked(); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + // Read Params if (!request.params[0].isNull() && !request.params[0].get_str().empty()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Dummy value must be set to \"\""); @@ -2414,6 +2473,10 @@ UniValue listreceivedbyaddress(const JSONRPCRequest& request) HelpExampleRpc("listreceivedbyaddress", "6, true, true") + HelpExampleRpc("listreceivedbyaddress", "6, true, true, \"DMJRSsuU9zfyrvxVaAEFQqK4MxZg6vgeS6\"")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); return ListReceived(request.params, false); @@ -2448,6 +2511,10 @@ UniValue listreceivedbyshieldaddress(const JSONRPCRequest& request) + HelpExampleRpc("listreceivedbyshieldaddress", "\"ps1ra969yfhvhp73rw5ak2xvtcm9fkuqsnmad7qln79mphhdrst3lwu9vvv03yuyqlh42p42st47qd\"") ); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); int nMinDepth = 1; @@ -2542,6 +2609,10 @@ UniValue listreceivedbylabel(const JSONRPCRequest& request) "\nExamples:\n" + HelpExampleCli("listreceivedbylabel", "") + HelpExampleCli("listreceivedbylabel", "6 true") + HelpExampleRpc("listreceivedbylabel", "6, true, true")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); return ListReceived(request.params, true); @@ -2574,6 +2645,10 @@ UniValue listcoldutxos(const JSONRPCRequest& request) "\nExamples:\n" + HelpExampleCli("listcoldutxos", "") + HelpExampleCli("listcoldutxos", "true")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); bool fExcludeWhitelisted = false; @@ -2740,6 +2815,10 @@ UniValue listtransactions(const JSONRPCRequest& request) HelpExampleRpc("listtransactions", "\"*\", 20, 100") ); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); if (!request.params[0].isNull() && request.params[0].get_str() != "*") { @@ -2841,6 +2920,10 @@ UniValue listsinceblock(const JSONRPCRequest& request) HelpExampleCli("listsinceblock", "\"000000000000000bacf66f7497b7dc45ef753ee9a7d38571037cdb1a57f663ad\" 6") + HelpExampleRpc("listsinceblock", "\"000000000000000bacf66f7497b7dc45ef753ee9a7d38571037cdb1a57f663ad\", 6")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); CBlockIndex* pindex = NULL; @@ -2928,6 +3011,10 @@ UniValue gettransaction(const JSONRPCRequest& request) HelpExampleCli("gettransaction", "\"1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d\" true") + HelpExampleRpc("gettransaction", "\"1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d\"")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); uint256 hash; @@ -2984,6 +3071,10 @@ UniValue abandontransaction(const JSONRPCRequest& request) EnsureWalletIsUnlocked(); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); uint256 hash; @@ -3011,6 +3102,10 @@ UniValue backupwallet(const JSONRPCRequest& request) "\nExamples:\n" + HelpExampleCli("backupwallet", "\"backup.dat\"") + HelpExampleRpc("backupwallet", "\"backup.dat\"")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); std::string strDest = request.params[0].get_str(); @@ -3199,6 +3294,10 @@ UniValue walletlock(const JSONRPCRequest& request) "\nAs json rpc call\n" + HelpExampleRpc("walletlock", "")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); if (request.fHelp) @@ -3315,6 +3414,10 @@ UniValue listunspent(const JSONRPCRequest& request) RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VNUM, UniValue::VARR, UniValue::VNUM}); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + int nMinDepth = 1; if (request.params.size() > 0) nMinDepth = request.params[0].get_int(); @@ -3350,7 +3453,6 @@ UniValue listunspent(const JSONRPCRequest& request) UniValue results(UniValue::VARR); std::vector vecOutputs; - assert(pwalletMain != NULL); LOCK2(cs_main, pwalletMain->cs_wallet); CWallet::AvailableCoinsFilter coinFilter; coinFilter.fOnlyConfirmed = false; @@ -3359,7 +3461,7 @@ UniValue listunspent(const JSONRPCRequest& request) if (out.nDepth < nMinDepth || out.nDepth > nMaxDepth) continue; - if (destinations.size()) { + if (!destinations.empty()) { CTxDestination address; if (!ExtractDestination(out.tx->tx->vout[out.i].scriptPubKey, address)) continue; @@ -3439,6 +3541,10 @@ UniValue lockunspent(const JSONRPCRequest& request) "\nAs a json rpc call\n" + HelpExampleRpc("lockunspent", "false, \"[{\\\"txid\\\":\\\"a08e6907dbbd3d809776dbfc5d82e371b764ed838b5655e72f463568df1aadf0\\\",\\\"vout\\\":1}]\"")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); if (request.params.size() == 1) @@ -3622,6 +3728,10 @@ UniValue getwalletinfo(const JSONRPCRequest& request) "\nExamples:\n" + HelpExampleCli("getwalletinfo", "") + HelpExampleRpc("getwalletinfo", "")); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); UniValue obj(UniValue::VOBJ); @@ -4052,6 +4162,10 @@ UniValue getsaplingnotescount(const JSONRPCRequest& request) + HelpExampleRpc("getsaplingnotescount", "0") ); + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + pwalletMain->BlockUntilSyncedToCurrentChain(); + LOCK2(cs_main, pwalletMain->cs_wallet); int nMinDepth = !request.params.empty() ? request.params[0].get_int() : 1; diff --git a/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp b/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp index 439bea930f96..1503a721061e 100644 --- a/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp +++ b/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp @@ -33,6 +33,9 @@ void generateBlock(const CScript& scriptPubKey, int expectedBlockHeight) CValidationState state; BOOST_CHECK_MESSAGE(ProcessNewBlock(state, nullptr, pblock, nullptr), strprintf("Failed creating block at height %d", expectedBlockHeight)); BOOST_CHECK(state.IsValid()); + + // Let the wallet sync the blocks + SyncWithValidationInterfaceQueue(); } SaplingOperation createOperationAndBuildTx(std::vector recipients, diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 0e26f9002167..e1822f6a356f 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -11,7 +11,6 @@ #include "wallet/wallet.h" #include -#include #include #include @@ -334,6 +333,7 @@ CBlockIndex* SimpleFakeMine(CWalletTx& wtx, CBlockIndex* pprev = nullptr) BOOST_CHECK(chainActive.Contains(fakeIndex)); wtx.SetMerkleBranch(fakeIndex->GetBlockHash(), 0); removeTxFromMempool(wtx); + wtx.fInMempool = false; return fakeIndex; } @@ -436,6 +436,7 @@ BOOST_AUTO_TEST_CASE(cached_balances_tests) // GetUnconfirmedBalance requires tx in mempool. fakeMempoolInsertion(wtxCredit.tx); + wtxCredit.fInMempool = true; BOOST_CHECK_EQUAL(wallet.GetUnconfirmedBalance(), nCredit); // 2) Confirm tx and verify diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c4f4377a3fa0..6489db9a6551 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -21,6 +21,7 @@ #include "utilmoneystr.h" #include "zpivchain.h" +#include #include CWallet* pwalletMain = nullptr; @@ -1250,6 +1251,19 @@ void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) { LOCK2(cs_main, cs_wallet); SyncTransaction(ptx, NULL, -1); + + auto it = mapWallet.find(ptx->GetHash()); + if (it != mapWallet.end()) { + it->second.fInMempool = true; + } +} + +void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx) { + LOCK(cs_wallet); + auto it = mapWallet.find(ptx->GetHash()); + if (it != mapWallet.end()) { + it->second.fInMempool = false; + } } void CWallet::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) @@ -1265,9 +1279,11 @@ void CWallet::BlockConnected(const std::shared_ptr& pblock, const for (const CTransactionRef& ptx : vtxConflicted) { SyncTransaction(ptx, nullptr, -1); + TransactionRemovedFromMempool(ptx); } for (size_t i = 0; i < pblock->vtx.size(); i++) { SyncTransaction(pblock->vtx[i], pindex, i); + TransactionRemovedFromMempool(pblock->vtx[i]); } // Sapling: notify about the connected block @@ -1285,6 +1301,8 @@ void CWallet::BlockConnected(const std::shared_ptr& pblock, const // Sapling: Update cached incremental witnesses ChainTipAdded(pindex, pblock.get(), oldSaplingTree); + + m_last_block_processed = pindex; } void CWallet::BlockDisconnected(const std::shared_ptr& pblock, int nBlockHeight) @@ -1301,6 +1319,30 @@ void CWallet::BlockDisconnected(const std::shared_ptr& pblock, int } } +void CWallet::BlockUntilSyncedToCurrentChain() { + AssertLockNotHeld(cs_main); + AssertLockNotHeld(cs_wallet); + + if (m_last_block_processed) { + // Skip the queue-draining stuff if we know we're caught up with + // chainActive.Tip()... + // We could also take cs_wallet here, and call m_last_block_processed + // protected by cs_wallet instead of cs_main, but as long as we need + // cs_main here anyway, its easier to just call it cs_main-protected. + LOCK(cs_main); + const CBlockIndex* initialChainTip = chainActive.Tip(); + + if (m_last_block_processed->GetAncestor(initialChainTip->nHeight) == initialChainTip) { + return; + } + } + + // ...otherwise put a callback in the validation interface queue and wait + // for the queue to drain enough to execute it (indicating we are caught up + // at least with the time we entered this function). + SyncWithValidationInterfaceQueue(); +} + void CWallet::MarkAffectedTransactionsDirty(const CTransaction& tx) { // If a transaction changes 'conflicted' state, that changes the balance @@ -1850,8 +1892,7 @@ void CWallet::ReacceptWalletTransactions(bool fFirstLoad) bool CWalletTx::InMempool() const { - LOCK(mempool.cs); - return mempool.exists(GetHash()); + return fInMempool; } void CWalletTx::RelayWalletTransaction(CConnman* connman) @@ -2998,7 +3039,9 @@ bool CWallet::CreateCoinStake( if (IsLocked() || ShutdownRequested()) return false; // Make sure the stake input hasn't been spent since last check - if (IsSpent(outPoint)) { + // for now, IsSpent() requires cs_main lock due its internal call to GetDepthInMainChain. + // This dependency will be completely removed moving forward, in #2209. + if (WITH_LOCK(cs_main, return IsSpent(outPoint))) { // remove it from the available coins it = availableCoins->erase(it); continue; @@ -3161,9 +3204,13 @@ CWallet::CommitResult CWallet::CommitTransaction(CTransactionRef tx, CReserveKey res.hashTx = wtxNew.GetHash(); + // Get the inserted-CWalletTx from mapWallet so that the + // fInMempool flag is cached properly + CWalletTx& wtx = mapWallet.at(wtxNew.GetHash()); + // Try ATMP. This must not fail. The transaction has already been signed and recorded. CValidationState state; - if (!wtxNew.AcceptToMemoryPool(state, true, true, false)) { + if (!wtx.AcceptToMemoryPool(state, true, true, false)) { res.state = state; // Abandon the transaction if (AbandonTransaction(res.hashTx)) { @@ -3179,7 +3226,7 @@ CWallet::CommitResult CWallet::CommitTransaction(CTransactionRef tx, CReserveKey res.status = CWallet::CommitStatus::OK; // Broadcast - wtxNew.RelayWalletTransaction(connman); + wtx.RelayWalletTransaction(connman); } return res; } @@ -4112,8 +4159,6 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) LogPrintf("Wallet completed loading in %15dms\n", GetTimeMillis() - nStart); - RegisterValidationInterface(walletInstance); - CBlockIndex* pindexRescan = chainActive.Tip(); if (gArgs.GetBoolArg("-rescan", false)) pindexRescan = chainActive.Genesis(); @@ -4125,6 +4170,10 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) else pindexRescan = chainActive.Genesis(); } + + walletInstance->m_last_block_processed = chainActive.Tip(); + RegisterValidationInterface(walletInstance); + if (chainActive.Tip() && chainActive.Tip() != pindexRescan) { uiInterface.InitMessage(_("Rescanning...")); LogPrintf("Rescanning last %i blocks (from block %i)...\n", chainActive.Height() - pindexRescan->nHeight, pindexRescan->nHeight); @@ -4276,7 +4325,18 @@ bool CWalletTx::IsInMainChainImmature() const bool CWalletTx::AcceptToMemoryPool(CValidationState& state, bool fLimitFree, bool fRejectInsaneFee, bool ignoreFees) { + // Quick check to avoid re-setting fInMempool to false + if (mempool.exists(tx->GetHash())) { + return false; + } + + // We must set fInMempool here - while it will be re-set to true by the + // entered-mempool callback, if we did not there would be a race where a + // user could call sendmoney in a loop and hit spurious out of funds errors + // because we think that the transaction they just generated's change is + // unavailable as we're not yet aware its in mempool. bool fAccepted = ::AcceptToMemoryPool(mempool, state, tx, fLimitFree, nullptr, false, fRejectInsaneFee, ignoreFees); + fInMempool = fAccepted; if (!fAccepted) LogPrintf("%s : %s\n", __func__, state.GetRejectReason()); return fAccepted; @@ -4542,6 +4602,7 @@ void CWalletTx::Init(const CWallet* pwalletIn) nTimeSmart = 0; fFromMe = false; fChangeCached = false; + fInMempool = false; nChangeCached = 0; fStakeDelegationVoided = false; fShieldedChangeCached = false; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 220a887baec0..c01e445e5ff0 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -337,6 +337,7 @@ class CWalletTx mutable bool fStakeDelegationVoided; mutable bool fChangeCached; + mutable bool fInMempool; mutable CAmount nChangeCached; mutable bool fShieldedChangeCached; mutable CAmount nShieldedChangeCached; @@ -518,6 +519,18 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface std::unique_ptr dbw; + /** + * The following is used to keep track of how far behind the wallet is + * from the chain sync, and to allow clients to block on us being caught up. + * + * Note that this is *not* how far we've processed, we may need some rescan + * to have seen all transactions in the chain, but is only used to track + * live BlockConnected callbacks. + * + * Protected by cs_main (see BlockUntilSyncedToCurrentChain) + */ + const CBlockIndex* m_last_block_processed{nullptr}; + int64_t nNextResend; int64_t nLastResend; @@ -851,6 +864,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool ActivateSaplingWallet(bool memOnly = false); int ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false, bool fromStartup = false); + void TransactionRemovedFromMempool(const CTransactionRef &ptx) override; void ReacceptWalletTransactions(bool fFirstLoad = false); void ResendWalletTransactions(CConnman* connman) override; @@ -1023,6 +1037,14 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface */ void postInitProcess(CScheduler& scheduler); + /** + * Blocks until the wallet state is up-to-date to /at least/ the current + * chain at the time this function is entered + * Obviously holding cs_main/cs_wallet when going into this call may cause + * deadlock + */ + void BlockUntilSyncedToCurrentChain(); + /** * Address book entry changed. * @note called with lock cs_wallet held. diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py index 5067beadf8da..60382bd1ff78 100755 --- a/test/functional/feature_dbcrash.py +++ b/test/functional/feature_dbcrash.py @@ -67,7 +67,7 @@ def set_test_params(self): self.node2_args = ["-dbcrashratio=24", "-dbcache=16", "-dbbatchsize=200000"] + self.base_args # Node3 is a normal node with default args, except will mine full blocks - self.node3_args = ["-blockmaxweight=4000000"] + self.chain_params # future: back port blockmaxweight + self.node3_args = ["-blockmaxsize=1999000"] + self.chain_params # future: back port blockmaxweight self.extra_args = [self.node0_args, self.node1_args, self.node2_args, self.node3_args] def setup_network(self): diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index 52fa6d3e57d3..4f5eeded70ae 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -59,7 +59,9 @@ def run_test(self): self.log.debug("Stop-start node0 and node1. Verify that node0 has the transactions in its mempool and node1 does not.") self.stop_nodes() - self.start_node(1) # Give this one a head-start, so we can be "extra-sure" that it didn't load anything later + # Give this node a head-start, so we can be "extra-sure" that it didn't load anything later + # Also don't store the mempool, to keep the datadir clean + self.start_node(1, extra_args=["-persistmempool=0"]) self.start_node(0) self.start_node(2) # Give pivxd a second to reload the mempool @@ -69,7 +71,7 @@ def run_test(self): assert_equal(len(self.nodes[1].getrawmempool()), 0) # Verify accounting of mempool transactions after restart is correct - #self.nodes[2].syncwithvalidationinterfacequeue() # Flush mempool to wallet + self.nodes[2].syncwithvalidationinterfacequeue() # Flush mempool to wallet assert_equal(node2_balance, self.nodes[2].getbalance()) self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.") diff --git a/test/functional/sapling_fillblock.py b/test/functional/sapling_fillblock.py index 9dc69af05eaa..82eb0710bac5 100755 --- a/test/functional/sapling_fillblock.py +++ b/test/functional/sapling_fillblock.py @@ -77,6 +77,7 @@ def send_shielded(self, node, n_txes, from_address, shield_to): txids.append(node.shieldsendmany(from_address, shield_to)) if (i + 1) % 200 == 0: self.log.info("...%d Transactions created..." % (i + 1)) + sync_mempools(self.nodes) return txids diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index a5eb464dee2f..30495e47b296 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -420,9 +420,9 @@ def sync_mempools(rpc_connections, *, wait=1, timeout=60, flush_scheduler=True): while time.time() <= stop_time: pool = [set(r.getrawmempool()) for r in rpc_connections] if pool.count(pool[0]) == len(rpc_connections): - #if flush_scheduler: - # for r in rpc_connections: - # r.syncwithvalidationinterfacequeue() + if flush_scheduler: + for r in rpc_connections: + r.syncwithvalidationinterfacequeue() return # Check that each peer has at least one connection assert (all([len(x.getpeerinfo()) for x in rpc_connections])) diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py index 58969c2b9620..749f26ce4e64 100755 --- a/test/functional/wallet_dump.py +++ b/test/functional/wallet_dump.py @@ -103,7 +103,7 @@ def run_test (self): found_addr, found_addr_chg, found_addr_rsv, hd_master_addr_unenc = \ read_dump(dumpUnencrypted, addrs, None) assert_equal(found_addr, test_addr_count) # all keys must be in the dump - assert_equal(found_addr_chg, 0) # 0 blocks where mined + assert_equal(found_addr_chg, 50) # 50 blocks where mined assert_equal(found_addr_rsv, 90 * 3) # 90 keys external plus 100% internal keys plus 100% staking keys #encrypt wallet, restart, unlock and dump @@ -118,7 +118,7 @@ def run_test (self): found_addr, found_addr_chg, found_addr_rsv, hd_master_addr_enc = \ read_dump(dumpEncrypted, addrs, hd_master_addr_unenc) assert_equal(found_addr, test_addr_count) - assert_equal(found_addr_chg, 90 * 3 + 1) # old reserve keys are marked as change now. todo: The +1 needs to be removed once this is updated (master seed taken as an internal key) + assert_equal(found_addr_chg, 90 * 3 + 1 + 50) # old reserve keys are marked as change now. todo: The +1 needs to be removed once this is updated (master seed taken as an internal key) assert_equal(found_addr_rsv, 90 * 3) # 90 external + 90 internal + 90 staking # Overwriting should fail diff --git a/test/functional/wallet_labels.py b/test/functional/wallet_labels.py index 981de7f30aad..21488d9c3d49 100755 --- a/test/functional/wallet_labels.py +++ b/test/functional/wallet_labels.py @@ -30,17 +30,17 @@ def run_test(self): assert_equal(node.getbalance(), 500) # there should be 2 address groups - # each with 1 address with a balance of 50 Bitcoins + # each with 1 address with a balance of 250 PIVs address_groups = node.listaddressgroupings() - assert_equal(len(address_groups), 1) + assert_equal(len(address_groups), 2) # the addresses aren't linked now, but will be after we send to the # common address linked_addresses = set() - #for address_group in address_groups: - # assert_equal(len(address_group), 1) - # assert_equal(len(address_group[0]), 2) - # assert_equal(address_group[0][1], 250) - # linked_addresses.add(address_group[0][0]) + for address_group in address_groups: + assert_equal(len(address_group), 1) + assert_equal(len(address_group[0]), 2) + assert_equal(address_group[0][1], 250) + linked_addresses.add(address_group[0][0]) # send 50 from each address to a third address not in this wallet # There's some fee that will come back to us when the miner reward diff --git a/test/functional/wallet_upgrade.py b/test/functional/wallet_upgrade.py index ea995034b105..4c07713ef303 100755 --- a/test/functional/wallet_upgrade.py +++ b/test/functional/wallet_upgrade.py @@ -76,7 +76,7 @@ def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 - def check_keys(self, addrs, mined_blocks = 0): + def check_keys(self, addrs): self.log.info("Checking old keys existence in the upgraded wallet..") # Now check that all of the pre upgrade addresses are still in the wallet for addr in addrs: @@ -87,7 +87,7 @@ def check_keys(self, addrs, mined_blocks = 0): self.log.info("All pre-upgrade keys found in the wallet :)") # Use all of the keys in the pre-HD keypool - for _ in range(0, 60 + mined_blocks): + for _ in range(0, 60): self.nodes[0].getnewaddress() self.log.info("All pre-upgrade keys should have been marked as used by now, creating new HD keys") @@ -135,7 +135,7 @@ def run_test(self): copyPreHDWallet(self.options.tmpdir, False) self.start_node(0) - # Generating a block to not be in IBD + # Generating a block to not be in IBD, here we create a new key for the coinbase script self.nodes[0].generate(1) self.log.info("Upgrading wallet..") @@ -143,7 +143,7 @@ def run_test(self): self.log.info("upgrade completed, checking keys now..") # Now check if the upgrade went fine - self.check_keys(addrs, 1) + self.check_keys(addrs) self.log.info("Upgrade via RPC completed, all good :)")