diff options
Diffstat (limited to 'src')
79 files changed, 1963 insertions, 1242 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index 0a9370c85c..7c2fe56d9d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -172,6 +172,7 @@ BITCOIN_CORE_H = \ wallet/wallet.h \ wallet/walletdb.h \ wallet/walletutil.h \ + wallet/coinselection.h \ warnings.h \ zmq/zmqabstractnotifier.h \ zmq/zmqconfig.h\ @@ -253,6 +254,7 @@ libbitcoin_wallet_a_SOURCES = \ wallet/wallet.cpp \ wallet/walletdb.cpp \ wallet/walletutil.cpp \ + wallet/coinselection.cpp \ $(BITCOIN_CORE_H) # crypto primitives library diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 4ee9102519..4d0819ab79 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -94,7 +94,8 @@ BITCOIN_TESTS += \ wallet/test/wallet_test_fixture.h \ wallet/test/accounting_tests.cpp \ wallet/test/wallet_tests.cpp \ - wallet/test/crypto_tests.cpp + wallet/test/crypto_tests.cpp \ + wallet/test/coinselector_tests.cpp endif test_test_bitcoin_SOURCES = $(BITCOIN_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES) diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 98965840c7..4b2a0e72fe 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -4,6 +4,7 @@ #include <bench/bench.h> #include <wallet/wallet.h> +#include <wallet/coinselection.h> #include <set> @@ -44,7 +45,11 @@ static void CoinSelection(benchmark::State& state) std::set<CInputCoin> setCoinsRet; CAmount nValueRet; - bool success = wallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet); + bool bnb_used; + CoinEligibilityFilter filter_standard(1, 6, 0); + CoinSelectionParams coin_selection_params(false, 34, 148, CFeeRate(0), 0); + bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) + || wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used); assert(success); assert(nValueRet == 1003 * COIN); assert(setCoinsRet.size() == 2); @@ -57,4 +62,47 @@ static void CoinSelection(benchmark::State& state) } } +typedef std::set<CInputCoin> CoinSet; + +// Copied from src/wallet/test/coinselector_tests.cpp +static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set) +{ + CMutableTransaction tx; + tx.vout.resize(nInput + 1); + tx.vout[nInput].nValue = nValue; + set.emplace_back(MakeTransactionRef(tx), nInput); +} +// Copied from src/wallet/test/coinselector_tests.cpp +static CAmount make_hard_case(int utxos, std::vector<CInputCoin>& utxo_pool) +{ + utxo_pool.clear(); + CAmount target = 0; + for (int i = 0; i < utxos; ++i) { + target += (CAmount)1 << (utxos+i); + add_coin((CAmount)1 << (utxos+i), 2*i, utxo_pool); + add_coin(((CAmount)1 << (utxos+i)) + ((CAmount)1 << (utxos-1-i)), 2*i + 1, utxo_pool); + } + return target; +} + +static void BnBExhaustion(benchmark::State& state) +{ + // Setup + std::vector<CInputCoin> utxo_pool; + CoinSet selection; + CAmount value_ret = 0; + CAmount not_input_fees = 0; + + while (state.KeepRunning()) { + // Benchmark + CAmount target = make_hard_case(17, utxo_pool); + SelectCoinsBnB(utxo_pool, target, 0, selection, value_ret, not_input_fees); // Should exhaust + + // Cleanup + utxo_pool.clear(); + selection.clear(); + } +} + BENCHMARK(CoinSelection, 650); +BENCHMARK(BnBExhaustion, 650); diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index fcd836fb45..8218e883a6 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -551,7 +551,6 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) // mergedTx will end up with all the signatures; it // starts as a clone of the raw tx: CMutableTransaction mergedTx(txVariants[0]); - bool fComplete = true; CCoinsView viewDummy; CCoinsViewCache view(&viewDummy); @@ -637,7 +636,6 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) CTxIn& txin = mergedTx.vin[i]; const Coin& coin = view.AccessCoin(txin.prevout); if (coin.IsSpent()) { - fComplete = false; continue; } const CScript& prevPubKey = coin.out.scriptPubKey; @@ -652,14 +650,6 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) for (const CTransaction& txv : txVariants) sigdata = CombineSignatures(prevPubKey, MutableTransactionSignatureChecker(&mergedTx, i, amount), sigdata, DataFromTransaction(txv, i)); UpdateTransaction(mergedTx, i, sigdata); - - if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker(&mergedTx, i, amount))) - fComplete = false; - } - - if (fComplete) { - // do nothing... for now - // perhaps store this for later optional JSON output } tx = mergedTx; diff --git a/src/blockencodings.h b/src/blockencodings.h index ba8c1d6a2a..f80821aa65 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -90,11 +90,11 @@ public: while (txn.size() < txn_size) { txn.resize(std::min((uint64_t)(1000 + txn.size()), txn_size)); for (; i < txn.size(); i++) - READWRITE(REF(TransactionCompressor(txn[i]))); + READWRITE(TransactionCompressor(txn[i])); } } else { for (size_t i = 0; i < txn.size(); i++) - READWRITE(REF(TransactionCompressor(txn[i]))); + READWRITE(TransactionCompressor(txn[i])); } } }; @@ -115,7 +115,7 @@ struct PrefilledTransaction { if (idx > std::numeric_limits<uint16_t>::max()) throw std::ios_base::failure("index overflowed 16-bits"); index = idx; - READWRITE(REF(TransactionCompressor(tx))); + READWRITE(TransactionCompressor(tx)); } }; diff --git a/src/chain.h b/src/chain.h index 3728f768c4..757840bb23 100644 --- a/src/chain.h +++ b/src/chain.h @@ -91,7 +91,7 @@ struct CDiskBlockPos template <typename Stream, typename Operation> inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITE(VARINT(nFile)); + READWRITE(VARINT(nFile, VarIntMode::NONNEGATIVE_SIGNED)); READWRITE(VARINT(nPos)); } @@ -386,13 +386,13 @@ public: inline void SerializationOp(Stream& s, Operation ser_action) { int _nVersion = s.GetVersion(); if (!(s.GetType() & SER_GETHASH)) - READWRITE(VARINT(_nVersion)); + READWRITE(VARINT(_nVersion, VarIntMode::NONNEGATIVE_SIGNED)); - READWRITE(VARINT(nHeight)); + READWRITE(VARINT(nHeight, VarIntMode::NONNEGATIVE_SIGNED)); READWRITE(VARINT(nStatus)); READWRITE(VARINT(nTx)); if (nStatus & (BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO)) - READWRITE(VARINT(nFile)); + READWRITE(VARINT(nFile, VarIntMode::NONNEGATIVE_SIGNED)); if (nStatus & BLOCK_HAVE_DATA) READWRITE(VARINT(nDataPos)); if (nStatus & BLOCK_HAVE_UNDO) diff --git a/src/checkpoints.cpp b/src/checkpoints.cpp index 9189c9a8ad..816d854db3 100644 --- a/src/checkpoints.cpp +++ b/src/checkpoints.cpp @@ -21,9 +21,10 @@ namespace Checkpoints { for (const MapCheckpoints::value_type& i : reverse_iterate(checkpoints)) { const uint256& hash = i.second; - BlockMap::const_iterator t = mapBlockIndex.find(hash); - if (t != mapBlockIndex.end()) - return t->second; + CBlockIndex* pindex = LookupBlockIndex(hash); + if (pindex) { + return pindex; + } } return nullptr; } diff --git a/src/checkpoints.h b/src/checkpoints.h index bf935f80a7..564b486393 100644 --- a/src/checkpoints.h +++ b/src/checkpoints.h @@ -19,7 +19,7 @@ struct CCheckpointData; namespace Checkpoints { -//! Returns last CBlockIndex* in mapBlockIndex that is a checkpoint +//! Returns last CBlockIndex* that is a checkpoint CBlockIndex* GetLastCheckpoint(const CCheckpointData& data); } //namespace Checkpoints diff --git a/src/coins.h b/src/coins.h index c6850947e2..a73f016a31 100644 --- a/src/coins.h +++ b/src/coins.h @@ -69,7 +69,7 @@ public: ::Unserialize(s, VARINT(code)); nHeight = code >> 1; fCoinBase = code & 1; - ::Unserialize(s, REF(CTxOutCompressor(out))); + ::Unserialize(s, CTxOutCompressor(out)); } bool IsSpent() const { diff --git a/src/compressor.h b/src/compressor.h index ee26f4c533..6fcecd27e9 100644 --- a/src/compressor.h +++ b/src/compressor.h @@ -73,7 +73,7 @@ public: s >> VARINT(nSize); if (nSize < nSpecialScripts) { std::vector<unsigned char> vch(GetSpecialSize(nSize), 0x00); - s >> REF(CFlatData(vch)); + s >> CFlatData(vch); Decompress(nSize, vch); return; } @@ -84,7 +84,7 @@ public: s.ignore(nSize); } else { script.resize(nSize); - s >> REF(CFlatData(script)); + s >> CFlatData(script); } } }; diff --git a/src/consensus/validation.h b/src/consensus/validation.h index c2a343c155..757df518ae 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -101,5 +101,10 @@ static inline int64_t GetBlockWeight(const CBlock& block) { return ::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * (WITNESS_SCALE_FACTOR - 1) + ::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION); } +static inline int64_t GetTransationInputWeight(const CTxIn& txin) +{ + // scriptWitness size is added here because witnesses and txins are split up in segwit serialization. + return ::GetSerializeSize(txin, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * (WITNESS_SCALE_FACTOR - 1) + ::GetSerializeSize(txin, SER_NETWORK, PROTOCOL_VERSION) + ::GetSerializeSize(txin.scriptWitness.stack, SER_NETWORK, PROTOCOL_VERSION); +} #endif // BITCOIN_CONSENSUS_VALIDATION_H diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index 6cac625abc..fb0d4215a2 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -218,14 +218,10 @@ void HandleError(const leveldb::Status& status) { if (status.ok()) return; - LogPrintf("%s\n", status.ToString()); - if (status.IsCorruption()) - throw dbwrapper_error("Database corrupted"); - if (status.IsIOError()) - throw dbwrapper_error("Database I/O error"); - if (status.IsNotFound()) - throw dbwrapper_error("Database entry missing"); - throw dbwrapper_error("Unknown database error"); + const std::string errmsg = "Fatal LevelDB error: " + status.ToString(); + LogPrintf("%s\n", errmsg); + LogPrintf("You can use -debug=leveldb to get more complete diagnostic messages\n"); + throw dbwrapper_error(errmsg); } const std::vector<unsigned char>& GetObfuscateKey(const CDBWrapper &w) diff --git a/src/hash.h b/src/hash.h index 35995a2d15..75353e0c0f 100644 --- a/src/hash.h +++ b/src/hash.h @@ -173,7 +173,7 @@ public: } template<typename T> - CHashVerifier<Source>& operator>>(T& obj) + CHashVerifier<Source>& operator>>(T&& obj) { // Unserialize from this stream ::Unserialize(*this, obj); diff --git a/src/init.cpp b/src/init.cpp index 659f97fec6..e0efe5c0a2 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1224,7 +1224,7 @@ bool AppInitMain() } if (!fLogTimestamps) - LogPrintf("Startup time: %s\n", DateTimeStrFormat("%Y-%m-%d %H:%M:%S", GetTime())); + LogPrintf("Startup time: %s\n", FormatISO8601DateTime(GetTime())); LogPrintf("Default data directory %s\n", GetDefaultDataDir().string()); LogPrintf("Using data directory %s\n", GetDataDir().string()); LogPrintf("Using config file %s\n", GetConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME)).string()); @@ -1424,6 +1424,8 @@ bool AppInitMain() uiInterface.InitMessage(_("Loading block index...")); + LOCK(cs_main); + nStart = GetTimeMillis(); do { try { @@ -1457,8 +1459,9 @@ bool AppInitMain() // If the loaded chain has a wrong genesis, bail out immediately // (we're likely using a testnet datadir, or the other way around). - if (!mapBlockIndex.empty() && mapBlockIndex.count(chainparams.GetConsensus().hashGenesisBlock) == 0) + if (!mapBlockIndex.empty() && !LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { return InitError(_("Incorrect or no genesis block found. Wrong datadir for network?")); + } // Check for changed -txindex state if (fTxIndex != gArgs.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { @@ -1532,16 +1535,13 @@ bool AppInitMain() MIN_BLOCKS_TO_KEEP); } - { - LOCK(cs_main); - CBlockIndex* tip = chainActive.Tip(); - RPCNotifyBlockChange(true, tip); - if (tip && tip->nTime > GetAdjustedTime() + 2 * 60 * 60) { - strLoadError = _("The block database contains a block which appears to be from the future. " - "This may be due to your computer's date and time being set incorrectly. " - "Only rebuild the block database if you are sure that your computer's date and time are correct"); - break; - } + CBlockIndex* tip = chainActive.Tip(); + RPCNotifyBlockChange(true, tip); + if (tip && tip->nTime > GetAdjustedTime() + 2 * 60 * 60) { + strLoadError = _("The block database contains a block which appears to be from the future. " + "This may be due to your computer's date and time being set incorrectly. " + "Only rebuild the block database if you are sure that your computer's date and time are correct"); + break; } if (!CVerifyDB().VerifyDB(chainparams, pcoinsdbview.get(), gArgs.GetArg("-checklevel", DEFAULT_CHECKLEVEL), diff --git a/src/miner.cpp b/src/miner.cpp index fcb376c6cb..4b86446774 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -282,7 +282,7 @@ bool BlockAssembler::SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_tran return mapModifiedTx.count(it) || inBlock.count(it) || failedTx.count(it); } -void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, CTxMemPool::txiter entry, std::vector<CTxMemPool::txiter>& sortedEntries) +void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, std::vector<CTxMemPool::txiter>& sortedEntries) { // Sort package by ancestor count // If a transaction A depends on transaction B, then A's ancestor count @@ -418,7 +418,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda // Package can be added. Sort the entries in a valid order. std::vector<CTxMemPool::txiter> sortedEntries; - SortForBlock(ancestors, iter, sortedEntries); + SortForBlock(ancestors, sortedEntries); for (size_t i=0; i<sortedEntries.size(); ++i) { AddToBlock(sortedEntries[i]); diff --git a/src/miner.h b/src/miner.h index 9c086332d4..33a22ba75f 100644 --- a/src/miner.h +++ b/src/miner.h @@ -185,7 +185,7 @@ private: * or if the transaction's cached data in mapTx is incorrect. */ bool SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set &mapModifiedTx, CTxMemPool::setEntries &failedTx); /** Sort the package in an order that is valid to appear in a block */ - void SortForBlock(const CTxMemPool::setEntries& package, CTxMemPool::txiter entry, std::vector<CTxMemPool::txiter>& sortedEntries); + void SortForBlock(const CTxMemPool::setEntries& package, std::vector<CTxMemPool::txiter>& sortedEntries); /** Add descendants of given transactions to mapModifiedTx with ancestor * state updated assuming given transactions are inBlock. Returns number * of updated descendants. */ diff --git a/src/net.cpp b/src/net.cpp index 33a60ac96e..4849e067aa 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -42,12 +42,13 @@ // We add a random period time (0 to 1 seconds) to feeler connections to prevent synchronization. #define FEELER_SLEEP_WINDOW 1 -#if !defined(HAVE_MSG_NOSIGNAL) +// MSG_NOSIGNAL is not available on some platforms, if it doesn't exist define it as 0 +#if !defined(MSG_NOSIGNAL) #define MSG_NOSIGNAL 0 #endif // MSG_DONTWAIT is not available on some platforms, if it doesn't exist define it as 0 -#if !defined(HAVE_MSG_DONTWAIT) +#if !defined(MSG_DONTWAIT) #define MSG_DONTWAIT 0 #endif @@ -2795,7 +2796,7 @@ void CNode::AskFor(const CInv& inv) nRequestTime = it->second; else nRequestTime = 0; - LogPrint(BCLog::NET, "askfor %s %d (%s) peer=%d\n", inv.ToString(), nRequestTime, DateTimeStrFormat("%H:%M:%S", nRequestTime/1000000), id); + LogPrint(BCLog::NET, "askfor %s %d (%s) peer=%d\n", inv.ToString(), nRequestTime, FormatISO8601Time(nRequestTime/1000000), id); // Make sure not to reuse time indexes to keep things in the same order int64_t nNow = GetTimeMicros() - 1000000; @@ -469,6 +469,13 @@ public: virtual bool SendMessages(CNode* pnode, std::atomic<bool>& interrupt) = 0; virtual void InitializeNode(CNode* pnode) = 0; virtual void FinalizeNode(NodeId id, bool& update_connection_time) = 0; + +protected: + /** + * Protected destructor so that instances can only be deleted by derived classes. + * If that restriction is no longer desired, this should be made public and virtual. + */ + ~NetEventsInterface() = default; }; enum diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 482a206c8b..f5073fe903 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -374,10 +374,11 @@ void ProcessBlockAvailability(NodeId nodeid) { assert(state != nullptr); if (!state->hashLastUnknownBlock.IsNull()) { - BlockMap::iterator itOld = mapBlockIndex.find(state->hashLastUnknownBlock); - if (itOld != mapBlockIndex.end() && itOld->second->nChainWork > 0) { - if (state->pindexBestKnownBlock == nullptr || itOld->second->nChainWork >= state->pindexBestKnownBlock->nChainWork) - state->pindexBestKnownBlock = itOld->second; + const CBlockIndex* pindex = LookupBlockIndex(state->hashLastUnknownBlock); + if (pindex && pindex->nChainWork > 0) { + if (state->pindexBestKnownBlock == nullptr || pindex->nChainWork >= state->pindexBestKnownBlock->nChainWork) { + state->pindexBestKnownBlock = pindex; + } state->hashLastUnknownBlock.SetNull(); } } @@ -390,11 +391,12 @@ void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) { ProcessBlockAvailability(nodeid); - BlockMap::iterator it = mapBlockIndex.find(hash); - if (it != mapBlockIndex.end() && it->second->nChainWork > 0) { + const CBlockIndex* pindex = LookupBlockIndex(hash); + if (pindex && pindex->nChainWork > 0) { // An actually better block was announced. - if (state->pindexBestKnownBlock == nullptr || it->second->nChainWork >= state->pindexBestKnownBlock->nChainWork) - state->pindexBestKnownBlock = it->second; + if (state->pindexBestKnownBlock == nullptr || pindex->nChainWork >= state->pindexBestKnownBlock->nChainWork) { + state->pindexBestKnownBlock = pindex; + } } else { // An unknown block was announced; just assume that the latest one is the best one. state->hashLastUnknownBlock = hash; @@ -1015,7 +1017,7 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) } case MSG_BLOCK: case MSG_WITNESS_BLOCK: - return mapBlockIndex.count(inv.hash); + return LookupBlockIndex(inv.hash) != nullptr; } // Don't know what it is, just say we already got one return true; @@ -1082,11 +1084,10 @@ void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensus bool need_activate_chain = false; { LOCK(cs_main); - BlockMap::iterator mi = mapBlockIndex.find(inv.hash); - if (mi != mapBlockIndex.end()) - { - if (mi->second->nChainTx && !mi->second->IsValid(BLOCK_VALID_SCRIPTS) && - mi->second->IsValid(BLOCK_VALID_TREE)) { + const CBlockIndex* pindex = LookupBlockIndex(inv.hash); + if (pindex) { + if (pindex->nChainTx && !pindex->IsValid(BLOCK_VALID_SCRIPTS) && + pindex->IsValid(BLOCK_VALID_TREE)) { // If we have the block and all of its parents, but have not yet validated it, // we might be in the middle of connecting it (ie in the unlock of cs_main // before ActivateBestChain but after AcceptBlock). @@ -1102,9 +1103,9 @@ void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensus } LOCK(cs_main); - BlockMap::iterator mi = mapBlockIndex.find(inv.hash); - if (mi != mapBlockIndex.end()) { - send = BlockRequestAllowed(mi->second, consensusParams); + const CBlockIndex* pindex = LookupBlockIndex(inv.hash); + if (pindex) { + send = BlockRequestAllowed(pindex, consensusParams); if (!send) { LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom->GetId()); } @@ -1112,7 +1113,7 @@ void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensus const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); // disconnect node in case we have reached the outbound limit for serving historical blocks // never disconnect whitelisted nodes - if (send && connman->OutboundTargetReached(true) && ( ((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - mi->second->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && !pfrom->fWhitelisted) + if (send && connman->OutboundTargetReached(true) && ( ((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && !pfrom->fWhitelisted) { LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom->GetId()); @@ -1122,7 +1123,7 @@ void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensus } // Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold if (send && !pfrom->fWhitelisted && ( - (((pfrom->GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((pfrom->GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) && (chainActive.Tip()->nHeight - mi->second->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS + 2 /* add two blocks buffer extension for possible races */) ) + (((pfrom->GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((pfrom->GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) && (chainActive.Tip()->nHeight - pindex->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS + 2 /* add two blocks buffer extension for possible races */) ) )) { LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold from peer=%d\n", pfrom->GetId()); @@ -1132,15 +1133,15 @@ void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensus } // Pruned nodes may have deleted the block, so check whether // it's available before trying to send. - if (send && (mi->second->nStatus & BLOCK_HAVE_DATA)) + if (send && (pindex->nStatus & BLOCK_HAVE_DATA)) { std::shared_ptr<const CBlock> pblock; - if (a_recent_block && a_recent_block->GetHash() == (*mi).second->GetBlockHash()) { + if (a_recent_block && a_recent_block->GetHash() == pindex->GetBlockHash()) { pblock = a_recent_block; } else { // Send block from disk std::shared_ptr<CBlock> pblockRead = std::make_shared<CBlock>(); - if (!ReadBlockFromDisk(*pblockRead, (*mi).second, consensusParams)) + if (!ReadBlockFromDisk(*pblockRead, pindex, consensusParams)) assert(!"cannot load block from disk"); pblock = pblockRead; } @@ -1182,8 +1183,8 @@ void static ProcessGetBlockData(CNode* pfrom, const Consensus::Params& consensus // instead we respond with the full, non-compact block. bool fPeerWantsWitness = State(pfrom->GetId())->fWantsCmpctWitness; int nSendFlags = fPeerWantsWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS; - if (CanDirectFetch(consensusParams) && mi->second->nHeight >= chainActive.Height() - MAX_CMPCTBLOCK_DEPTH) { - if ((fPeerWantsWitness || !fWitnessesPresentInARecentCompactBlock) && a_recent_compact_block && a_recent_compact_block->header.GetHash() == mi->second->GetBlockHash()) { + if (CanDirectFetch(consensusParams) && pindex->nHeight >= chainActive.Height() - MAX_CMPCTBLOCK_DEPTH) { + if ((fPeerWantsWitness || !fWitnessesPresentInARecentCompactBlock) && a_recent_compact_block && a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) { connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); } else { CBlockHeaderAndShortTxIDs cmpctblock(*pblock, fPeerWantsWitness); @@ -1323,7 +1324,7 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve // don't connect before giving DoS points // - Once a headers message is received that is valid and does connect, // nUnconnectingHeaders gets reset back to 0. - if (mapBlockIndex.find(headers[0].hashPrevBlock) == mapBlockIndex.end() && nCount < MAX_BLOCKS_TO_ANNOUNCE) { + if (!LookupBlockIndex(headers[0].hashPrevBlock) && nCount < MAX_BLOCKS_TO_ANNOUNCE) { nodestate->nUnconnectingHeaders++; connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(pindexBestHeader), uint256())); LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d, nUnconnectingHeaders=%d)\n", @@ -1353,7 +1354,7 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve // If we don't have the last header, then they'll have given us // something new (if these headers are valid). - if (mapBlockIndex.find(hashLastBlock) == mapBlockIndex.end()) { + if (!LookupBlockIndex(hashLastBlock)) { received_new_header = true; } } @@ -1369,7 +1370,7 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve } else { LogPrint(BCLog::NET, "peer=%d: invalid header received\n", pfrom->GetId()); } - if (punish_duplicate_invalid && mapBlockIndex.find(first_invalid_header.GetHash()) != mapBlockIndex.end()) { + if (punish_duplicate_invalid && LookupBlockIndex(first_invalid_header.GetHash())) { // Goal: don't allow outbound peers to use up our outbound // connection slots if they are on incompatible chains. // @@ -2050,13 +2051,13 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LOCK(cs_main); - BlockMap::iterator it = mapBlockIndex.find(req.blockhash); - if (it == mapBlockIndex.end() || !(it->second->nStatus & BLOCK_HAVE_DATA)) { + const CBlockIndex* pindex = LookupBlockIndex(req.blockhash); + if (!pindex || !(pindex->nStatus & BLOCK_HAVE_DATA)) { LogPrint(BCLog::NET, "Peer %d sent us a getblocktxn for a block we don't have", pfrom->GetId()); return true; } - if (it->second->nHeight < chainActive.Height() - MAX_BLOCKTXN_DEPTH) { + if (pindex->nHeight < chainActive.Height() - MAX_BLOCKTXN_DEPTH) { // If an older block is requested (should never happen in practice, // but can happen in tests) send a block response instead of a // blocktxn response. Sending a full block response instead of a @@ -2074,7 +2075,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } CBlock block; - bool ret = ReadBlockFromDisk(block, it->second, chainparams.GetConsensus()); + bool ret = ReadBlockFromDisk(block, pindex, chainparams.GetConsensus()); assert(ret); SendBlockTransactions(block, req, pfrom, connman); @@ -2098,10 +2099,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (locator.IsNull()) { // If locator is null, return the hashStop block - BlockMap::iterator mi = mapBlockIndex.find(hashStop); - if (mi == mapBlockIndex.end()) + pindex = LookupBlockIndex(hashStop); + if (!pindex) { return true; - pindex = (*mi).second; + } if (!BlockRequestAllowed(pindex, chainparams.GetConsensus())) { LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block header that isn't in the main chain\n", __func__, pfrom->GetId()); @@ -2340,14 +2341,14 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr { LOCK(cs_main); - if (mapBlockIndex.find(cmpctblock.header.hashPrevBlock) == mapBlockIndex.end()) { + if (!LookupBlockIndex(cmpctblock.header.hashPrevBlock)) { // Doesn't connect (or is genesis), instead of DoSing in AcceptBlockHeader, request deeper headers if (!IsInitialBlockDownload()) connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(pindexBestHeader), uint256())); return true; } - if (mapBlockIndex.find(cmpctblock.header.GetHash()) == mapBlockIndex.end()) { + if (!LookupBlockIndex(cmpctblock.header.GetHash())) { received_new_header = true; } } @@ -3329,9 +3330,8 @@ bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic<bool>& interruptM // then send all headers past that one. If we come across any // headers that aren't on chainActive, give up. for (const uint256 &hash : pto->vBlockHashesToAnnounce) { - BlockMap::iterator mi = mapBlockIndex.find(hash); - assert(mi != mapBlockIndex.end()); - const CBlockIndex *pindex = mi->second; + const CBlockIndex* pindex = LookupBlockIndex(hash); + assert(pindex); if (chainActive[pindex->nHeight] != pindex) { // Bail out if we reorged away from this block fRevertToInv = true; @@ -3422,9 +3422,8 @@ bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic<bool>& interruptM // in the past. if (!pto->vBlockHashesToAnnounce.empty()) { const uint256 &hashToAnnounce = pto->vBlockHashesToAnnounce.back(); - BlockMap::iterator mi = mapBlockIndex.find(hashToAnnounce); - assert(mi != mapBlockIndex.end()); - const CBlockIndex *pindex = mi->second; + const CBlockIndex* pindex = LookupBlockIndex(hashToAnnounce); + assert(pindex); // Warn if we're announcing a block that is not on the main chain. // This should be very rare and could be optimized out. diff --git a/src/net_processing.h b/src/net_processing.h index ff1ebc59da..11543129cf 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -35,7 +35,7 @@ static constexpr int64_t EXTRA_PEER_CHECK_INTERVAL = 45; /** Minimum time an outbound-peer-eviction candidate must be connected for, in order to evict, in seconds */ static constexpr int64_t MINIMUM_CONNECT_TIME = 30; -class PeerLogicValidation : public CValidationInterface, public NetEventsInterface { +class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface { private: CConnman* const connman; diff --git a/src/netbase.cpp b/src/netbase.cpp index 3ea3141d5e..92ac1c4c85 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -21,7 +21,7 @@ #include <boost/algorithm/string/case_conv.hpp> // for to_lower() #include <boost/algorithm/string/predicate.hpp> // for startswith() and endswith() -#if !defined(HAVE_MSG_NOSIGNAL) +#if !defined(MSG_NOSIGNAL) #define MSG_NOSIGNAL 0 #endif diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index bff58932b4..41f967c985 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -258,3 +258,8 @@ int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost) { return GetVirtualTransactionSize(GetTransactionWeight(tx), nSigOpCost); } + +int64_t GetVirtualTransactionInputSize(const CTxIn& txin, int64_t nSigOpCost) +{ + return GetVirtualTransactionSize(GetTransationInputWeight(txin), nSigOpCost); +} diff --git a/src/policy/policy.h b/src/policy/policy.h index 3d96406bbc..e4eda4b635 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -102,5 +102,6 @@ extern unsigned int nBytesPerSigOp; /** Compute the virtual transaction size (weight reinterpreted as bytes). */ int64_t GetVirtualTransactionSize(int64_t nWeight, int64_t nSigOpCost); int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost = 0); +int64_t GetVirtualTransactionInputSize(const CTxIn& tx, int64_t nSigOpCost = 0); #endif // BITCOIN_POLICY_POLICY_H diff --git a/src/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp index 517aa49e2b..f2ddbf259b 100644 --- a/src/qt/addressbookpage.cpp +++ b/src/qt/addressbookpage.cpp @@ -21,6 +21,41 @@ #include <QMessageBox> #include <QSortFilterProxyModel> +class AddressBookSortFilterProxyModel final : public QSortFilterProxyModel +{ + const QString m_type; + +public: + AddressBookSortFilterProxyModel(const QString& type, QObject* parent) + : QSortFilterProxyModel(parent) + , m_type(type) + { + setDynamicSortFilter(true); + setFilterCaseSensitivity(Qt::CaseInsensitive); + setSortCaseSensitivity(Qt::CaseInsensitive); + } + +protected: + bool filterAcceptsRow(int row, const QModelIndex& parent) const + { + auto model = sourceModel(); + auto label = model->index(row, AddressTableModel::Label, parent); + + if (model->data(label, AddressTableModel::TypeRole).toString() != m_type) { + return false; + } + + auto address = model->index(row, AddressTableModel::Address, parent); + + if (filterRegExp().indexIn(model->data(address).toString()) < 0 && + filterRegExp().indexIn(model->data(label).toString()) < 0) { + return false; + } + + return true; + } +}; + AddressBookPage::AddressBookPage(const PlatformStyle *platformStyle, Mode _mode, Tabs _tab, QWidget *parent) : QDialog(parent), ui(new Ui::AddressBookPage), @@ -69,10 +104,12 @@ AddressBookPage::AddressBookPage(const PlatformStyle *platformStyle, Mode _mode, case SendingTab: ui->labelExplanation->setText(tr("These are your Bitcoin addresses for sending payments. Always check the amount and the receiving address before sending coins.")); ui->deleteAddress->setVisible(true); + ui->newAddress->setVisible(true); break; case ReceivingTab: ui->labelExplanation->setText(tr("These are your Bitcoin addresses for receiving payments. It is recommended to use a new receiving address for each transaction.")); ui->deleteAddress->setVisible(false); + ui->newAddress->setVisible(false); break; } @@ -113,24 +150,12 @@ void AddressBookPage::setModel(AddressTableModel *_model) if(!_model) return; - proxyModel = new QSortFilterProxyModel(this); + auto type = tab == ReceivingTab ? AddressTableModel::Receive : AddressTableModel::Send; + proxyModel = new AddressBookSortFilterProxyModel(type, this); proxyModel->setSourceModel(_model); - proxyModel->setDynamicSortFilter(true); - proxyModel->setSortCaseSensitivity(Qt::CaseInsensitive); - proxyModel->setFilterCaseSensitivity(Qt::CaseInsensitive); - switch(tab) - { - case ReceivingTab: - // Receive filter - proxyModel->setFilterRole(AddressTableModel::TypeRole); - proxyModel->setFilterFixedString(AddressTableModel::Receive); - break; - case SendingTab: - // Send filter - proxyModel->setFilterRole(AddressTableModel::TypeRole); - proxyModel->setFilterFixedString(AddressTableModel::Send); - break; - } + + connect(ui->searchLineEdit, SIGNAL(textChanged(QString)), proxyModel, SLOT(setFilterWildcard(QString))); + ui->tableView->setModel(proxyModel); ui->tableView->sortByColumn(0, Qt::AscendingOrder); @@ -188,10 +213,11 @@ void AddressBookPage::on_newAddress_clicked() if(!model) return; - EditAddressDialog dlg( - tab == SendingTab ? - EditAddressDialog::NewSendingAddress : - EditAddressDialog::NewReceivingAddress, this); + if (tab == ReceivingTab) { + return; + } + + EditAddressDialog dlg(EditAddressDialog::NewSendingAddress, this); dlg.setModel(model); if(dlg.exec()) { diff --git a/src/qt/addressbookpage.h b/src/qt/addressbookpage.h index 54a43478d1..8877d07330 100644 --- a/src/qt/addressbookpage.h +++ b/src/qt/addressbookpage.h @@ -7,6 +7,7 @@ #include <QDialog> +class AddressBookSortFilterProxyModel; class AddressTableModel; class PlatformStyle; @@ -18,7 +19,6 @@ QT_BEGIN_NAMESPACE class QItemSelection; class QMenu; class QModelIndex; -class QSortFilterProxyModel; QT_END_NAMESPACE /** Widget that shows a list of sending or receiving addresses. @@ -53,7 +53,7 @@ private: Mode mode; Tabs tab; QString returnValue; - QSortFilterProxyModel *proxyModel; + AddressBookSortFilterProxyModel *proxyModel; QMenu *contextMenu; QAction *deleteAction; // to be able to explicitly disable it QString newAddressToSelect; diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index 4f9a79d654..801334483a 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -441,6 +441,8 @@ int AddressTableModel::lookupAddress(const QString &address) const } } +OutputType AddressTableModel::GetDefaultAddressType() const { return wallet->m_default_address_type; }; + void AddressTableModel::emitDataChanged(int idx) { Q_EMIT dataChanged(index(idx, 0, QModelIndex()), index(idx, columns.length()-1, QModelIndex())); diff --git a/src/qt/addresstablemodel.h b/src/qt/addresstablemodel.h index 11439e25d5..ed7a4e6f43 100644 --- a/src/qt/addresstablemodel.h +++ b/src/qt/addresstablemodel.h @@ -8,7 +8,7 @@ #include <QAbstractTableModel> #include <QStringList> -enum OutputType : int; +enum class OutputType; class AddressTablePriv; class WalletModel; @@ -76,6 +76,8 @@ public: EditStatus getEditStatus() const { return editStatus; } + OutputType GetDefaultAddressType() const; + private: WalletModel *walletModel; CWallet *wallet; diff --git a/src/qt/editaddressdialog.cpp b/src/qt/editaddressdialog.cpp index a945fc6aa0..38411c499f 100644 --- a/src/qt/editaddressdialog.cpp +++ b/src/qt/editaddressdialog.cpp @@ -11,7 +11,6 @@ #include <QDataWidgetMapper> #include <QMessageBox> -extern OutputType g_address_type; EditAddressDialog::EditAddressDialog(Mode _mode, QWidget *parent) : QDialog(parent), @@ -26,10 +25,6 @@ EditAddressDialog::EditAddressDialog(Mode _mode, QWidget *parent) : switch(mode) { - case NewReceivingAddress: - setWindowTitle(tr("New receiving address")); - ui->addressEdit->setEnabled(false); - break; case NewSendingAddress: setWindowTitle(tr("New sending address")); break; @@ -74,13 +69,12 @@ bool EditAddressDialog::saveCurrentRow() switch(mode) { - case NewReceivingAddress: case NewSendingAddress: address = model->addRow( - mode == NewSendingAddress ? AddressTableModel::Send : AddressTableModel::Receive, + AddressTableModel::Send, ui->labelEdit->text(), ui->addressEdit->text(), - g_address_type); + model->GetDefaultAddressType()); break; case EditReceivingAddress: case EditSendingAddress: diff --git a/src/qt/editaddressdialog.h b/src/qt/editaddressdialog.h index ddb67ece72..41c5d1708a 100644 --- a/src/qt/editaddressdialog.h +++ b/src/qt/editaddressdialog.h @@ -25,7 +25,6 @@ class EditAddressDialog : public QDialog public: enum Mode { - NewReceivingAddress, NewSendingAddress, EditReceivingAddress, EditSendingAddress diff --git a/src/qt/forms/addressbookpage.ui b/src/qt/forms/addressbookpage.ui index 264edeb720..7ac216286c 100644 --- a/src/qt/forms/addressbookpage.ui +++ b/src/qt/forms/addressbookpage.ui @@ -22,6 +22,13 @@ </widget> </item> <item> + <widget class="QLineEdit" name="searchLineEdit"> + <property name="placeholderText"> + <string>Enter address or label to search</string> + </property> + </widget> + </item> + <item> <widget class="QTableView" name="tableView"> <property name="contextMenuPolicy"> <enum>Qt::CustomContextMenu</enum> diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index 8ad4fa31f1..92d7d72935 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -643,7 +643,7 @@ void PaymentServer::fetchPaymentACK(CWallet* wallet, const SendCoinsRecipient& r // use for change. Despite an actual payment and not change, this is a close match: // it's the output type we use subject to privacy issues, but not restricted by what // other software supports. - const OutputType change_type = g_change_type != OUTPUT_TYPE_NONE ? g_change_type : g_address_type; + const OutputType change_type = wallet->m_default_change_type != OutputType::NONE ? wallet->m_default_change_type : wallet->m_default_address_type; wallet->LearnRelatedScripts(newKey, change_type); CTxDestination dest = GetDestinationForKey(newKey, change_type); wallet->SetAddressBook(dest, strAccount, "refund"); @@ -770,7 +770,7 @@ bool PaymentServer::verifyExpired(const payments::PaymentDetails& requestDetails { bool fVerified = (requestDetails.has_expires() && (int64_t)requestDetails.expires() < GetTime()); if (fVerified) { - const QString requestExpires = QString::fromStdString(DateTimeStrFormat("%Y-%m-%d %H:%M:%S", (int64_t)requestDetails.expires())); + const QString requestExpires = QString::fromStdString(FormatISO8601DateTime((int64_t)requestDetails.expires())); qWarning() << QString("PaymentServer::%1: Payment request expired \"%2\".") .arg(__func__) .arg(requestExpires); diff --git a/src/qt/receivecoinsdialog.cpp b/src/qt/receivecoinsdialog.cpp index 7fd5285467..132fb54d66 100644 --- a/src/qt/receivecoinsdialog.cpp +++ b/src/qt/receivecoinsdialog.cpp @@ -95,13 +95,13 @@ void ReceiveCoinsDialog::setModel(WalletModel *_model) columnResizingFixer = new GUIUtil::TableViewLastColumnResizingFixer(tableView, AMOUNT_MINIMUM_COLUMN_WIDTH, DATE_COLUMN_WIDTH, this); // configure bech32 checkbox, disable if launched with legacy as default: - if (model->getDefaultAddressType() == OUTPUT_TYPE_BECH32) { + if (model->getDefaultAddressType() == OutputType::BECH32) { ui->useBech32->setCheckState(Qt::Checked); } else { ui->useBech32->setCheckState(Qt::Unchecked); } - ui->useBech32->setVisible(model->getDefaultAddressType() != OUTPUT_TYPE_LEGACY); + ui->useBech32->setVisible(model->getDefaultAddressType() != OutputType::LEGACY); } } @@ -145,8 +145,8 @@ void ReceiveCoinsDialog::on_receiveButton_clicked() QString label = ui->reqLabel->text(); /* Generate new receiving address */ OutputType address_type = model->getDefaultAddressType(); - if (address_type != OUTPUT_TYPE_LEGACY) { - address_type = ui->useBech32->isChecked() ? OUTPUT_TYPE_BECH32 : OUTPUT_TYPE_P2SH_SEGWIT; + if (address_type != OutputType::LEGACY) { + address_type = ui->useBech32->isChecked() ? OutputType::BECH32 : OutputType::P2SH_SEGWIT; } address = model->getAddressTableModel()->addRow(AddressTableModel::Receive, label, "", address_type); SendCoinsRecipient info(address, label, diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 976aadc0af..c9898e52ca 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -150,9 +150,6 @@ void BumpFee(TransactionView& view, const uint256& txid, bool expectDisabled, st // src/qt/test/test_bitcoin-qt -platform cocoa # macOS void TestGUI() { - g_address_type = OUTPUT_TYPE_P2SH_SEGWIT; - g_change_type = OUTPUT_TYPE_P2SH_SEGWIT; - // Set up wallet and chain with 105 blocks (5 mature blocks for spending). TestChain100Setup test; for (int i = 0; i < 5; ++i) { @@ -163,7 +160,7 @@ void TestGUI() wallet.LoadWallet(firstRun); { LOCK(wallet.cs_wallet); - wallet.SetAddressBook(GetDestinationForKey(test.coinbaseKey.GetPubKey(), g_address_type), "", "receive"); + wallet.SetAddressBook(GetDestinationForKey(test.coinbaseKey.GetPubKey(), wallet.m_default_address_type), "", "receive"); wallet.AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey()); } { diff --git a/src/qt/transactionfilterproxy.cpp b/src/qt/transactionfilterproxy.cpp index a702461f7a..6301af7553 100644 --- a/src/qt/transactionfilterproxy.cpp +++ b/src/qt/transactionfilterproxy.cpp @@ -31,31 +31,35 @@ bool TransactionFilterProxy::filterAcceptsRow(int sourceRow, const QModelIndex & { QModelIndex index = sourceModel()->index(sourceRow, 0, sourceParent); - int type = index.data(TransactionTableModel::TypeRole).toInt(); - QDateTime datetime = index.data(TransactionTableModel::DateRole).toDateTime(); - bool involvesWatchAddress = index.data(TransactionTableModel::WatchonlyRole).toBool(); - QString address = index.data(TransactionTableModel::AddressRole).toString(); - QString label = index.data(TransactionTableModel::LabelRole).toString(); - QString txid = index.data(TransactionTableModel::TxHashRole).toString(); - qint64 amount = llabs(index.data(TransactionTableModel::AmountRole).toLongLong()); int status = index.data(TransactionTableModel::StatusRole).toInt(); - - if(!showInactive && status == TransactionStatus::Conflicted) + if (!showInactive && status == TransactionStatus::Conflicted) return false; - if(!(TYPE(type) & typeFilter)) + + int type = index.data(TransactionTableModel::TypeRole).toInt(); + if (!(TYPE(type) & typeFilter)) return false; + + bool involvesWatchAddress = index.data(TransactionTableModel::WatchonlyRole).toBool(); if (involvesWatchAddress && watchOnlyFilter == WatchOnlyFilter_No) return false; if (!involvesWatchAddress && watchOnlyFilter == WatchOnlyFilter_Yes) return false; - if(datetime < dateFrom || datetime > dateTo) + + QDateTime datetime = index.data(TransactionTableModel::DateRole).toDateTime(); + if (datetime < dateFrom || datetime > dateTo) return false; + + QString address = index.data(TransactionTableModel::AddressRole).toString(); + QString label = index.data(TransactionTableModel::LabelRole).toString(); + QString txid = index.data(TransactionTableModel::TxHashRole).toString(); if (!address.contains(m_search_string, Qt::CaseInsensitive) && ! label.contains(m_search_string, Qt::CaseInsensitive) && ! txid.contains(m_search_string, Qt::CaseInsensitive)) { return false; } - if(amount < minAmount) + + qint64 amount = llabs(index.data(TransactionTableModel::AmountRole).toLongLong()); + if (amount < minAmount) return false; return true; diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 19cdb0fdea..cc30cf747d 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -167,10 +167,7 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) // Determine transaction status // Find the block the tx is in - CBlockIndex* pindex = nullptr; - BlockMap::iterator mi = mapBlockIndex.find(wtx.hashBlock); - if (mi != mapBlockIndex.end()) - pindex = (*mi).second; + const CBlockIndex* pindex = LookupBlockIndex(wtx.hashBlock); // Sort order, unrecorded transactions sort to the top status.sortKey = strprintf("%010d-%01d-%010u-%03d", diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index e7d9d276d7..5e7aefb187 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -275,9 +275,9 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact int nChangePosRet = -1; std::string strFailReason; - CWalletTx *newTx = transaction.getTransaction(); + CTransactionRef& newTx = transaction.getTransaction(); CReserveKey *keyChange = transaction.getPossibleKeyChange(); - bool fCreated = wallet->CreateTransaction(vecSend, *newTx, *keyChange, nFeeRequired, nChangePosRet, strFailReason, coinControl); + bool fCreated = wallet->CreateTransaction(vecSend, newTx, *keyChange, nFeeRequired, nChangePosRet, strFailReason, coinControl); transaction.setTransactionFee(nFeeRequired); if (fSubtractFeeFromAmount && fCreated) transaction.reassignAmounts(nChangePosRet); @@ -309,8 +309,8 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran { LOCK2(cs_main, wallet->cs_wallet); - CWalletTx *newTx = transaction.getTransaction(); + std::vector<std::pair<std::string, std::string>> vOrderForm; for (const SendCoinsRecipient &rcp : transaction.getRecipients()) { if (rcp.paymentRequest.IsInitialized()) @@ -321,22 +321,22 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran } // Store PaymentRequests in wtx.vOrderForm in wallet. - std::string key("PaymentRequest"); std::string value; rcp.paymentRequest.SerializeToString(&value); - newTx->vOrderForm.push_back(make_pair(key, value)); + vOrderForm.emplace_back("PaymentRequest", std::move(value)); } else if (!rcp.message.isEmpty()) // Message from normal bitcoin:URI (bitcoin:123...?message=example) - newTx->vOrderForm.push_back(make_pair("Message", rcp.message.toStdString())); + vOrderForm.emplace_back("Message", rcp.message.toStdString()); } + CTransactionRef& newTx = transaction.getTransaction(); CReserveKey *keyChange = transaction.getPossibleKeyChange(); CValidationState state; - if(!wallet->CommitTransaction(*newTx, *keyChange, g_connman.get(), state)) + if (!wallet->CommitTransaction(newTx, {} /* mapValue */, std::move(vOrderForm), {} /* fromAccount */, *keyChange, g_connman.get(), state)) return SendCoinsReturn(TransactionCommitFailed, QString::fromStdString(state.GetRejectReason())); CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); - ssTx << *newTx->tx; + ssTx << newTx; transaction_array.append(&(ssTx[0]), ssTx.size()); } @@ -736,7 +736,7 @@ bool WalletModel::hdEnabled() const OutputType WalletModel::getDefaultAddressType() const { - return g_address_type; + return wallet->m_default_address_type; } int WalletModel::getDefaultConfirmTarget() const diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 811996b98f..64813e0f5a 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -20,7 +20,7 @@ #include <QObject> -enum OutputType : int; +enum class OutputType; class AddressTableModel; class OptionsModel; diff --git a/src/qt/walletmodeltransaction.cpp b/src/qt/walletmodeltransaction.cpp index 4b2bef2690..4df8a5687e 100644 --- a/src/qt/walletmodeltransaction.cpp +++ b/src/qt/walletmodeltransaction.cpp @@ -12,12 +12,6 @@ WalletModelTransaction::WalletModelTransaction(const QList<SendCoinsRecipient> & walletTransaction(0), fee(0) { - walletTransaction = new CWalletTx(); -} - -WalletModelTransaction::~WalletModelTransaction() -{ - delete walletTransaction; } QList<SendCoinsRecipient> WalletModelTransaction::getRecipients() const @@ -25,14 +19,14 @@ QList<SendCoinsRecipient> WalletModelTransaction::getRecipients() const return recipients; } -CWalletTx *WalletModelTransaction::getTransaction() const +CTransactionRef& WalletModelTransaction::getTransaction() { return walletTransaction; } unsigned int WalletModelTransaction::getTransactionSize() { - return (!walletTransaction ? 0 : ::GetVirtualTransactionSize(*walletTransaction->tx)); + return (!walletTransaction ? 0 : ::GetVirtualTransactionSize(*walletTransaction)); } CAmount WalletModelTransaction::getTransactionFee() const @@ -62,7 +56,7 @@ void WalletModelTransaction::reassignAmounts(int nChangePosRet) if (out.amount() <= 0) continue; if (i == nChangePosRet) i++; - subtotal += walletTransaction->tx->vout[i].nValue; + subtotal += walletTransaction->vout[i].nValue; i++; } rcp.amount = subtotal; @@ -71,7 +65,7 @@ void WalletModelTransaction::reassignAmounts(int nChangePosRet) { if (i == nChangePosRet) i++; - rcp.amount = walletTransaction->tx->vout[i].nValue; + rcp.amount = walletTransaction->vout[i].nValue; i++; } } diff --git a/src/qt/walletmodeltransaction.h b/src/qt/walletmodeltransaction.h index cd531dba4b..931e960d18 100644 --- a/src/qt/walletmodeltransaction.h +++ b/src/qt/walletmodeltransaction.h @@ -20,11 +20,10 @@ class WalletModelTransaction { public: explicit WalletModelTransaction(const QList<SendCoinsRecipient> &recipients); - ~WalletModelTransaction(); QList<SendCoinsRecipient> getRecipients() const; - CWalletTx *getTransaction() const; + CTransactionRef& getTransaction(); unsigned int getTransactionSize(); void setTransactionFee(const CAmount& newFee); @@ -39,7 +38,7 @@ public: private: QList<SendCoinsRecipient> recipients; - CWalletTx *walletTransaction; + CTransactionRef walletTransaction; std::unique_ptr<CReserveKey> keyChange; CAmount fee; }; diff --git a/src/rest.cpp b/src/rest.cpp index 8cba59dbbc..f47b40343b 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -147,8 +147,7 @@ static bool rest_headers(HTTPRequest* req, headers.reserve(count); { LOCK(cs_main); - BlockMap::const_iterator it = mapBlockIndex.find(hash); - const CBlockIndex *pindex = (it != mapBlockIndex.end()) ? it->second : nullptr; + const CBlockIndex* pindex = LookupBlockIndex(hash); while (pindex != nullptr && chainActive.Contains(pindex)) { headers.push_back(pindex); if (headers.size() == (unsigned long)count) @@ -212,10 +211,11 @@ static bool rest_block(HTTPRequest* req, CBlockIndex* pblockindex = nullptr; { LOCK(cs_main); - if (mapBlockIndex.count(hash) == 0) + pblockindex = LookupBlockIndex(hash); + if (!pblockindex) { return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); + } - pblockindex = mapBlockIndex[hash]; if (fHavePruned && !(pblockindex->nStatus & BLOCK_HAVE_DATA) && pblockindex->nTx > 0) return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not available (pruned data)"); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index f2a1fd048f..3f66c0c536 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -709,10 +709,10 @@ UniValue getblockheader(const JSONRPCRequest& request) if (!request.params[1].isNull()) fVerbose = request.params[1].get_bool(); - if (mapBlockIndex.count(hash) == 0) + const CBlockIndex* pblockindex = LookupBlockIndex(hash); + if (!pblockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); - - CBlockIndex* pblockindex = mapBlockIndex[hash]; + } if (!fVerbose) { @@ -788,12 +788,12 @@ UniValue getblock(const JSONRPCRequest& request) verbosity = request.params[1].get_bool() ? 1 : 0; } - if (mapBlockIndex.count(hash) == 0) + const CBlockIndex* pblockindex = LookupBlockIndex(hash); + if (!pblockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); + } CBlock block; - CBlockIndex* pblockindex = mapBlockIndex[hash]; - if (fHavePruned && !(pblockindex->nStatus & BLOCK_HAVE_DATA) && pblockindex->nTx > 0) throw JSONRPCError(RPC_MISC_ERROR, "Block not available (pruned data)"); @@ -834,18 +834,18 @@ static void ApplyStats(CCoinsStats &stats, CHashWriter& ss, const uint256& hash, { assert(!outputs.empty()); ss << hash; - ss << VARINT(outputs.begin()->second.nHeight * 2 + outputs.begin()->second.fCoinBase); + ss << VARINT(outputs.begin()->second.nHeight * 2 + outputs.begin()->second.fCoinBase, VarIntMode::NONNEGATIVE_SIGNED); stats.nTransactions++; for (const auto output : outputs) { ss << VARINT(output.first + 1); ss << output.second.out.scriptPubKey; - ss << VARINT(output.second.out.nValue); + ss << VARINT(output.second.out.nValue, VarIntMode::NONNEGATIVE_SIGNED); stats.nTransactionOutputs++; stats.nTotalAmount += output.second.out.nValue; stats.nBogoSize += 32 /* txid */ + 4 /* vout index */ + 4 /* height + coinbase */ + 8 /* amount */ + 2 /* scriptPubKey len */ + output.second.out.scriptPubKey.size() /* scriptPubKey */; } - ss << VARINT(0); + ss << VARINT(0u); } //! Calculate statistics about the unspent transaction output set @@ -858,7 +858,7 @@ static bool GetUTXOStats(CCoinsView *view, CCoinsStats &stats) stats.hashBlock = pcursor->GetBestBlock(); { LOCK(cs_main); - stats.nHeight = mapBlockIndex.find(stats.hashBlock)->second->nHeight; + stats.nHeight = LookupBlockIndex(stats.hashBlock)->nHeight; } ss << stats.hashBlock; uint256 prevkey; @@ -1041,8 +1041,7 @@ UniValue gettxout(const JSONRPCRequest& request) } } - BlockMap::iterator it = mapBlockIndex.find(pcoinsTip->GetBestBlock()); - CBlockIndex *pindex = it->second; + const CBlockIndex* pindex = LookupBlockIndex(pcoinsTip->GetBestBlock()); ret.pushKV("bestblock", pindex->GetBlockHash().GetHex()); if (coin.nHeight == MEMPOOL_HEIGHT) { ret.pushKV("confirmations", 0); @@ -1436,10 +1435,10 @@ UniValue preciousblock(const JSONRPCRequest& request) { LOCK(cs_main); - if (mapBlockIndex.count(hash) == 0) + pblockindex = LookupBlockIndex(hash); + if (!pblockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); - - pblockindex = mapBlockIndex[hash]; + } } CValidationState state; @@ -1472,10 +1471,11 @@ UniValue invalidateblock(const JSONRPCRequest& request) { LOCK(cs_main); - if (mapBlockIndex.count(hash) == 0) + CBlockIndex* pblockindex = LookupBlockIndex(hash); + if (!pblockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); + } - CBlockIndex* pblockindex = mapBlockIndex[hash]; InvalidateBlock(state, Params(), pblockindex); } @@ -1510,10 +1510,11 @@ UniValue reconsiderblock(const JSONRPCRequest& request) { LOCK(cs_main); - if (mapBlockIndex.count(hash) == 0) + CBlockIndex* pblockindex = LookupBlockIndex(hash); + if (!pblockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); + } - CBlockIndex* pblockindex = mapBlockIndex[hash]; ResetBlockFailureFlags(pblockindex); } @@ -1560,11 +1561,10 @@ UniValue getchaintxstats(const JSONRPCRequest& request) } else { uint256 hash = uint256S(request.params[1].get_str()); LOCK(cs_main); - auto it = mapBlockIndex.find(hash); - if (it == mapBlockIndex.end()) { + pindex = LookupBlockIndex(hash); + if (!pindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } - pindex = it->second; if (!chainActive.Contains(pindex)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Block is not in main chain"); } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 3073a49d0d..0537628763 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -396,9 +396,8 @@ UniValue getblocktemplate(const JSONRPCRequest& request) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Block decode failed"); uint256 hash = block.GetHash(); - BlockMap::iterator mi = mapBlockIndex.find(hash); - if (mi != mapBlockIndex.end()) { - CBlockIndex *pindex = mi->second; + const CBlockIndex* pindex = LookupBlockIndex(hash); + if (pindex) { if (pindex->IsValid(BLOCK_VALID_SCRIPTS)) return "duplicate"; if (pindex->nStatus & BLOCK_FAILED_MASK) @@ -727,9 +726,8 @@ UniValue submitblock(const JSONRPCRequest& request) bool fBlockPresent = false; { LOCK(cs_main); - BlockMap::iterator mi = mapBlockIndex.find(hash); - if (mi != mapBlockIndex.end()) { - CBlockIndex *pindex = mi->second; + const CBlockIndex* pindex = LookupBlockIndex(hash); + if (pindex) { if (pindex->IsValid(BLOCK_VALID_SCRIPTS)) { return "duplicate"; } @@ -743,9 +741,9 @@ UniValue submitblock(const JSONRPCRequest& request) { LOCK(cs_main); - BlockMap::iterator mi = mapBlockIndex.find(block.hashPrevBlock); - if (mi != mapBlockIndex.end()) { - UpdateUncommittedBlockStructures(block, mi->second, Params().GetConsensus()); + const CBlockIndex* pindex = LookupBlockIndex(block.hashPrevBlock); + if (pindex) { + UpdateUncommittedBlockStructures(block, pindex, Params().GetConsensus()); } } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 8dcfb48e9a..20bfd3f355 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -48,9 +48,8 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry) if (!hashBlock.IsNull()) { entry.pushKV("blockhash", hashBlock.GetHex()); - BlockMap::iterator mi = mapBlockIndex.find(hashBlock); - if (mi != mapBlockIndex.end() && (*mi).second) { - CBlockIndex* pindex = (*mi).second; + CBlockIndex* pindex = LookupBlockIndex(hashBlock); + if (pindex) { if (chainActive.Contains(pindex)) { entry.pushKV("confirmations", 1 + chainActive.Height() - pindex->nHeight); entry.pushKV("time", pindex->GetBlockTime()); @@ -160,11 +159,10 @@ UniValue getrawtransaction(const JSONRPCRequest& request) if (!request.params[2].isNull()) { uint256 blockhash = ParseHashV(request.params[2], "parameter 3"); - BlockMap::iterator it = mapBlockIndex.find(blockhash); - if (it == mapBlockIndex.end()) { + blockindex = LookupBlockIndex(blockhash); + if (!blockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block hash not found"); } - blockindex = it->second; in_active_chain = chainActive.Contains(blockindex); } @@ -238,9 +236,10 @@ UniValue gettxoutproof(const JSONRPCRequest& request) if (!request.params[1].isNull()) { hashBlock = uint256S(request.params[1].get_str()); - if (!mapBlockIndex.count(hashBlock)) + pblockindex = LookupBlockIndex(hashBlock); + if (!pblockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); - pblockindex = mapBlockIndex[hashBlock]; + } } else { // Loop through txids and try to find which block they're in. Exit loop once a block is found. for (const auto& tx : setTxids) { @@ -257,9 +256,10 @@ UniValue gettxoutproof(const JSONRPCRequest& request) CTransactionRef tx; if (!GetTransaction(oneTxid, tx, Params().GetConsensus(), hashBlock, false) || hashBlock.IsNull()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not yet in block"); - if (!mapBlockIndex.count(hashBlock)) + pblockindex = LookupBlockIndex(hashBlock); + if (!pblockindex) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Transaction index corrupt"); - pblockindex = mapBlockIndex[hashBlock]; + } } CBlock block; @@ -306,8 +306,10 @@ UniValue verifytxoutproof(const JSONRPCRequest& request) LOCK(cs_main); - if (!mapBlockIndex.count(merkleBlock.header.GetHash()) || !chainActive.Contains(mapBlockIndex[merkleBlock.header.GetHash()])) + const CBlockIndex* pindex = LookupBlockIndex(merkleBlock.header.GetHash()); + if (!pindex || !chainActive.Contains(pindex)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain"); + } for (const uint256& hash : vMatch) res.push_back(hash.GetHex()); @@ -316,9 +318,10 @@ UniValue verifytxoutproof(const JSONRPCRequest& request) UniValue createrawtransaction(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() < 2 || request.params.size() > 4) + if (request.fHelp || request.params.size() < 2 || request.params.size() > 4) { throw std::runtime_error( - "createrawtransaction [{\"txid\":\"id\",\"vout\":n},...] {\"address\":amount,\"data\":\"hex\",...} ( locktime ) ( replaceable )\n" + // clang-format off + "createrawtransaction [{\"txid\":\"id\",\"vout\":n},...] [{\"address\":amount},{\"data\":\"hex\"},...] ( locktime ) ( replaceable )\n" "\nCreate a transaction spending the given inputs and creating new outputs.\n" "Outputs can be addresses or data.\n" "Returns hex-encoded raw transaction.\n" @@ -329,18 +332,23 @@ UniValue createrawtransaction(const JSONRPCRequest& request) "1. \"inputs\" (array, required) A json array of json objects\n" " [\n" " {\n" - " \"txid\":\"id\", (string, required) The transaction id\n" + " \"txid\":\"id\", (string, required) The transaction id\n" " \"vout\":n, (numeric, required) The output number\n" " \"sequence\":n (numeric, optional) The sequence number\n" " } \n" " ,...\n" " ]\n" - "2. \"outputs\" (object, required) a json object with outputs\n" + "2. \"outputs\" (array, required) a json array with outputs (key-value pairs)\n" + " [\n" " {\n" - " \"address\": x.xxx, (numeric or string, required) The key is the bitcoin address, the numeric value (can be string) is the " + CURRENCY_UNIT + " amount\n" - " \"data\": \"hex\" (string, required) The key is \"data\", the value is hex encoded data\n" - " ,...\n" + " \"address\": x.xxx, (obj, optional) A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in " + CURRENCY_UNIT + "\n" + " },\n" + " {\n" + " \"data\": \"hex\" (obj, optional) A key-value pair. The key must be \"data\", the value is hex encoded data\n" " }\n" + " ,... More key-value pairs of the above form. For compatibility reasons, a dictionary, which holds the key-value pairs directly, is also\n" + " accepted as second parameter.\n" + " ]\n" "3. locktime (numeric, optional, default=0) Raw locktime. Non-0 value also locktime-activates inputs\n" "4. replaceable (boolean, optional, default=false) Marks this transaction as BIP125 replaceable.\n" " Allows this transaction to be replaced by a transaction with higher fees. If provided, it is an error if explicit sequence numbers are incompatible.\n" @@ -348,18 +356,29 @@ UniValue createrawtransaction(const JSONRPCRequest& request) "\"transaction\" (string) hex string of the transaction\n" "\nExamples:\n" - + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\" \"{\\\"address\\\":0.01}\"") - + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\" \"{\\\"data\\\":\\\"00010203\\\"}\"") - + HelpExampleRpc("createrawtransaction", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\", \"{\\\"address\\\":0.01}\"") - + HelpExampleRpc("createrawtransaction", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\", \"{\\\"data\\\":\\\"00010203\\\"}\"") + + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\" \"[{\\\"address\\\":0.01}]\"") + + HelpExampleCli("createrawtransaction", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\" \"[{\\\"data\\\":\\\"00010203\\\"}]\"") + + HelpExampleRpc("createrawtransaction", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\", \"[{\\\"address\\\":0.01}]\"") + + HelpExampleRpc("createrawtransaction", "\"[{\\\"txid\\\":\\\"myid\\\",\\\"vout\\\":0}]\", \"[{\\\"data\\\":\\\"00010203\\\"}]\"") + // clang-format on ); + } - RPCTypeCheck(request.params, {UniValue::VARR, UniValue::VOBJ, UniValue::VNUM, UniValue::VBOOL}, true); + RPCTypeCheck(request.params, { + UniValue::VARR, + UniValueType(), // ARR or OBJ, checked later + UniValue::VNUM, + UniValue::VBOOL + }, true + ); if (request.params[0].isNull() || request.params[1].isNull()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, arguments 1 and 2 must be non-null"); UniValue inputs = request.params[0].get_array(); - UniValue sendTo = request.params[1].get_obj(); + const bool outputs_is_obj = request.params[1].isObject(); + UniValue outputs = outputs_is_obj ? + request.params[1].get_obj() : + request.params[1].get_array(); CMutableTransaction rawTx; @@ -411,11 +430,24 @@ UniValue createrawtransaction(const JSONRPCRequest& request) } std::set<CTxDestination> destinations; - std::vector<std::string> addrList = sendTo.getKeys(); - for (const std::string& name_ : addrList) { - + if (!outputs_is_obj) { + // Translate array of key-value pairs into dict + UniValue outputs_dict = UniValue(UniValue::VOBJ); + for (size_t i = 0; i < outputs.size(); ++i) { + const UniValue& output = outputs[i]; + if (!output.isObject()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, key-value pair not an object as expected"); + } + if (output.size() != 1) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, key-value pair must contain exactly one key"); + } + outputs_dict.pushKVs(output); + } + outputs = std::move(outputs_dict); + } + for (const std::string& name_ : outputs.getKeys()) { if (name_ == "data") { - std::vector<unsigned char> data = ParseHexV(sendTo[name_].getValStr(),"Data"); + std::vector<unsigned char> data = ParseHexV(outputs[name_].getValStr(), "Data"); CTxOut out(0, CScript() << OP_RETURN << data); rawTx.vout.push_back(out); @@ -430,7 +462,7 @@ UniValue createrawtransaction(const JSONRPCRequest& request) } CScript scriptPubKey = GetScriptForDestination(destination); - CAmount nAmount = AmountFromValue(sendTo[name_]); + CAmount nAmount = AmountFromValue(outputs[name_]); CTxOut out(nAmount, scriptPubKey); rawTx.vout.push_back(out); diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 35401bf876..54995ef000 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -50,12 +50,11 @@ void RPCServer::OnStopped(std::function<void ()> slot) } void RPCTypeCheck(const UniValue& params, - const std::list<UniValue::VType>& typesExpected, + const std::list<UniValueType>& typesExpected, bool fAllowNull) { unsigned int i = 0; - for (UniValue::VType t : typesExpected) - { + for (const UniValueType& t : typesExpected) { if (params.size() <= i) break; @@ -67,10 +66,10 @@ void RPCTypeCheck(const UniValue& params, } } -void RPCTypeCheckArgument(const UniValue& value, UniValue::VType typeExpected) +void RPCTypeCheckArgument(const UniValue& value, const UniValueType& typeExpected) { - if (value.type() != typeExpected) { - throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Expected type %s, got %s", uvTypeName(typeExpected), uvTypeName(value.type()))); + if (!typeExpected.typeAny && value.type() != typeExpected.type) { + throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Expected type %s, got %s", uvTypeName(typeExpected.type), uvTypeName(value.type()))); } } diff --git a/src/rpc/server.h b/src/rpc/server.h index 075940cb90..d25268a8ab 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -28,9 +28,9 @@ namespace RPCServer } /** Wrapper for UniValue::VType, which includes typeAny: - * Used to denote don't care type. Only used by RPCTypeCheckObj */ + * Used to denote don't care type. */ struct UniValueType { - explicit UniValueType(UniValue::VType _type) : typeAny(false), type(_type) {} + UniValueType(UniValue::VType _type) : typeAny(false), type(_type) {} UniValueType() : typeAny(true) {} bool typeAny; UniValue::VType type; @@ -69,12 +69,12 @@ bool RPCIsInWarmup(std::string *outStatus); * the right number of arguments are passed, just that any passed are the correct type. */ void RPCTypeCheck(const UniValue& params, - const std::list<UniValue::VType>& typesExpected, bool fAllowNull=false); + const std::list<UniValueType>& typesExpected, bool fAllowNull=false); /** * Type-check one argument; throws JSONRPCError if wrong type given. */ -void RPCTypeCheckArgument(const UniValue& value, UniValue::VType typeExpected); +void RPCTypeCheckArgument(const UniValue& value, const UniValueType& typeExpected); /* Check for expected keys/value types in an Object. @@ -165,8 +165,17 @@ public: /** * Appends a CRPCCommand to the dispatch table. + * * Returns false if RPC server is already running (dump concurrency protection). + * * Commands cannot be overwritten (returns false). + * + * Commands with different method names but the same callback function will + * be considered aliases, and only the first registered method name will + * show up in the help text command listing. Aliased commands do not have + * to have the same behavior. Server and client code can distinguish + * between calls based on method name, and aliased commands can also + * register different names, types, and numbers of parameters. */ bool appendCommand(const std::string& name, const CRPCCommand* pcmd); }; diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 593962e710..e72b1c4840 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -4,7 +4,6 @@ #include <key_io.h> #include <keystore.h> -#include <pubkey.h> #include <rpc/protocol.h> #include <rpc/util.h> #include <tinyformat.h> diff --git a/src/rpc/util.h b/src/rpc/util.h index 5380d45a83..c6a79d5cf9 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -8,7 +8,6 @@ #include <pubkey.h> #include <script/standard.h> #include <univalue.h> -#include <utilstrencodings.h> #include <boost/variant/static_visitor.hpp> diff --git a/src/script/bitcoinconsensus.cpp b/src/script/bitcoinconsensus.cpp index 7d3587e2c2..8cc44b675f 100644 --- a/src/script/bitcoinconsensus.cpp +++ b/src/script/bitcoinconsensus.cpp @@ -40,7 +40,7 @@ public: } template<typename T> - TxInputStream& operator>>(T& obj) + TxInputStream& operator>>(T&& obj) { ::Unserialize(*this, obj); return *this; diff --git a/src/script/sign.cpp b/src/script/sign.cpp index aaba5e5926..baa712dc2d 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -194,11 +194,16 @@ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nI return data; } +void UpdateInput(CTxIn& input, const SignatureData& data) +{ + input.scriptSig = data.scriptSig; + input.scriptWitness = data.scriptWitness; +} + void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data) { assert(tx.vin.size() > nIn); - tx.vin[nIn].scriptSig = data.scriptSig; - tx.vin[nIn].scriptWitness = data.scriptWitness; + UpdateInput(tx.vin[nIn], data); } bool SignSignature(const CKeyStore &keystore, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType) diff --git a/src/script/sign.h b/src/script/sign.h index 97c0014cd0..2c749521cd 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -80,6 +80,7 @@ SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignature /** Extract signature data from a transaction, and insert it. */ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn); void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data); +void UpdateInput(CTxIn& input, const SignatureData& data); /* Check whether we know how to sign for an output like this, assuming we * have all private keys. While this function does not need private keys, the passed diff --git a/src/serialize.h b/src/serialize.h index dcc8d8691e..91da6b0f80 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -148,8 +148,7 @@ enum SER_GETHASH = (1 << 2), }; -#define READWRITE(obj) (::SerReadWrite(s, (obj), ser_action)) -#define READWRITEMANY(...) (::SerReadWriteMany(s, ser_action, __VA_ARGS__)) +#define READWRITE(...) (::SerReadWriteMany(s, ser_action, __VA_ARGS__)) /** * Implement three methods for serializable objects. These are actually wrappers over @@ -297,9 +296,31 @@ uint64_t ReadCompactSize(Stream& is) * 2^32: [0x8E 0xFE 0xFE 0xFF 0x00] */ -template<typename I> +/** + * Mode for encoding VarInts. + * + * Currently there is no support for signed encodings. The default mode will not + * compile with signed values, and the legacy "nonnegative signed" mode will + * accept signed values, but improperly encode and decode them if they are + * negative. In the future, the DEFAULT mode could be extended to support + * negative numbers in a backwards compatible way, and additional modes could be + * added to support different varint formats (e.g. zigzag encoding). + */ +enum class VarIntMode { DEFAULT, NONNEGATIVE_SIGNED }; + +template <VarIntMode Mode, typename I> +struct CheckVarIntMode { + constexpr CheckVarIntMode() + { + static_assert(Mode != VarIntMode::DEFAULT || std::is_unsigned<I>::value, "Unsigned type required with mode DEFAULT."); + static_assert(Mode != VarIntMode::NONNEGATIVE_SIGNED || std::is_signed<I>::value, "Signed type required with mode NONNEGATIVE_SIGNED."); + } +}; + +template<VarIntMode Mode, typename I> inline unsigned int GetSizeOfVarInt(I n) { + CheckVarIntMode<Mode, I>(); int nRet = 0; while(true) { nRet++; @@ -313,9 +334,10 @@ inline unsigned int GetSizeOfVarInt(I n) template<typename I> inline void WriteVarInt(CSizeComputer& os, I n); -template<typename Stream, typename I> +template<typename Stream, VarIntMode Mode, typename I> void WriteVarInt(Stream& os, I n) { + CheckVarIntMode<Mode, I>(); unsigned char tmp[(sizeof(n)*8+6)/7]; int len=0; while(true) { @@ -330,9 +352,10 @@ void WriteVarInt(Stream& os, I n) } while(len--); } -template<typename Stream, typename I> +template<typename Stream, VarIntMode Mode, typename I> I ReadVarInt(Stream& is) { + CheckVarIntMode<Mode, I>(); I n = 0; while(true) { unsigned char chData = ser_readdata8(is); @@ -351,10 +374,10 @@ I ReadVarInt(Stream& is) } } -#define FLATDATA(obj) REF(CFlatData((char*)&(obj), (char*)&(obj) + sizeof(obj))) -#define VARINT(obj) REF(WrapVarInt(REF(obj))) -#define COMPACTSIZE(obj) REF(CCompactSize(REF(obj))) -#define LIMITED_STRING(obj,n) REF(LimitedString< n >(REF(obj))) +#define FLATDATA(obj) CFlatData((char*)&(obj), (char*)&(obj) + sizeof(obj)) +#define VARINT(obj, ...) WrapVarInt<__VA_ARGS__>(REF(obj)) +#define COMPACTSIZE(obj) CCompactSize(REF(obj)) +#define LIMITED_STRING(obj,n) LimitedString< n >(REF(obj)) /** * Wrapper for serializing arrays and POD. @@ -396,7 +419,7 @@ public: } }; -template<typename I> +template<VarIntMode Mode, typename I> class CVarInt { protected: @@ -406,12 +429,12 @@ public: template<typename Stream> void Serialize(Stream &s) const { - WriteVarInt<Stream,I>(s, n); + WriteVarInt<Stream,Mode,I>(s, n); } template<typename Stream> void Unserialize(Stream& s) { - n = ReadVarInt<Stream,I>(s); + n = ReadVarInt<Stream,Mode,I>(s); } }; @@ -462,8 +485,8 @@ public: } }; -template<typename I> -CVarInt<I> WrapVarInt(I& n) { return CVarInt<I>(n); } +template<VarIntMode Mode=VarIntMode::DEFAULT, typename I> +CVarInt<Mode, I> WrapVarInt(I& n) { return CVarInt<Mode, I>{n}; } /** * Forward declarations @@ -539,7 +562,7 @@ inline void Serialize(Stream& os, const T& a) } template<typename Stream, typename T> -inline void Unserialize(Stream& is, T& a) +inline void Unserialize(Stream& is, T&& a) { a.Unserialize(is); } @@ -825,19 +848,6 @@ struct CSerActionUnserialize constexpr bool ForRead() const { return true; } }; -template<typename Stream, typename T> -inline void SerReadWrite(Stream& s, const T& obj, CSerActionSerialize ser_action) -{ - ::Serialize(s, obj); -} - -template<typename Stream, typename T> -inline void SerReadWrite(Stream& s, T& obj, CSerActionUnserialize ser_action) -{ - ::Unserialize(s, obj); -} - - @@ -897,17 +907,11 @@ void SerializeMany(Stream& s) { } -template<typename Stream, typename Arg> -void SerializeMany(Stream& s, Arg&& arg) -{ - ::Serialize(s, std::forward<Arg>(arg)); -} - template<typename Stream, typename Arg, typename... Args> -void SerializeMany(Stream& s, Arg&& arg, Args&&... args) +void SerializeMany(Stream& s, const Arg& arg, const Args&... args) { - ::Serialize(s, std::forward<Arg>(arg)); - ::SerializeMany(s, std::forward<Args>(args)...); + ::Serialize(s, arg); + ::SerializeMany(s, args...); } template<typename Stream> @@ -915,27 +919,21 @@ inline void UnserializeMany(Stream& s) { } -template<typename Stream, typename Arg> -inline void UnserializeMany(Stream& s, Arg& arg) -{ - ::Unserialize(s, arg); -} - template<typename Stream, typename Arg, typename... Args> -inline void UnserializeMany(Stream& s, Arg& arg, Args&... args) +inline void UnserializeMany(Stream& s, Arg&& arg, Args&&... args) { ::Unserialize(s, arg); ::UnserializeMany(s, args...); } template<typename Stream, typename... Args> -inline void SerReadWriteMany(Stream& s, CSerActionSerialize ser_action, Args&&... args) +inline void SerReadWriteMany(Stream& s, CSerActionSerialize ser_action, const Args&... args) { - ::SerializeMany(s, std::forward<Args>(args)...); + ::SerializeMany(s, args...); } template<typename Stream, typename... Args> -inline void SerReadWriteMany(Stream& s, CSerActionUnserialize ser_action, Args&... args) +inline void SerReadWriteMany(Stream& s, CSerActionUnserialize ser_action, Args&&... args) { ::UnserializeMany(s, args...); } diff --git a/src/streams.h b/src/streams.h index 9f86c4a163..6ba4f103da 100644 --- a/src/streams.h +++ b/src/streams.h @@ -42,7 +42,7 @@ public: } template<typename T> - OverrideStream<Stream>& operator>>(T& obj) + OverrideStream<Stream>& operator>>(T&& obj) { // Unserialize from this stream ::Unserialize(*this, obj); @@ -399,7 +399,7 @@ public: } template<typename T> - CDataStream& operator>>(T& obj) + CDataStream& operator>>(T&& obj) { // Unserialize from this stream ::Unserialize(*this, obj); @@ -543,7 +543,7 @@ public: } template<typename T> - CAutoFile& operator>>(T& obj) + CAutoFile& operator>>(T&& obj) { // Unserialize from this stream if (!file) @@ -686,7 +686,7 @@ public: } template<typename T> - CBufferedFile& operator>>(T& obj) { + CBufferedFile& operator>>(T&& obj) { // Unserialize from this stream ::Unserialize(*this, obj); return (*this); diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 892e4f2dac..8d9f80ada0 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -52,7 +52,6 @@ BOOST_AUTO_TEST_CASE(rpc_rawparams) BOOST_CHECK_THROW(CallRPC("createrawtransaction"), std::runtime_error); BOOST_CHECK_THROW(CallRPC("createrawtransaction null null"), std::runtime_error); BOOST_CHECK_THROW(CallRPC("createrawtransaction not_array"), std::runtime_error); - BOOST_CHECK_THROW(CallRPC("createrawtransaction [] []"), std::runtime_error); BOOST_CHECK_THROW(CallRPC("createrawtransaction {} {}"), std::runtime_error); BOOST_CHECK_NO_THROW(CallRPC("createrawtransaction [] {}")); BOOST_CHECK_THROW(CallRPC("createrawtransaction [] {} extra"), std::runtime_error); diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 4595519435..7a79a77e8b 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -53,7 +53,7 @@ public: template <typename Stream, typename Operation> inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITEMANY(intval, boolval, stringval, FLATDATA(charstrval), txval); + READWRITE(intval, boolval, stringval, FLATDATA(charstrval), txval); } }; @@ -177,8 +177,8 @@ BOOST_AUTO_TEST_CASE(varints) CDataStream ss(SER_DISK, 0); CDataStream::size_type size = 0; for (int i = 0; i < 100000; i++) { - ss << VARINT(i); - size += ::GetSerializeSize(VARINT(i), 0, 0); + ss << VARINT(i, VarIntMode::NONNEGATIVE_SIGNED); + size += ::GetSerializeSize(VARINT(i, VarIntMode::NONNEGATIVE_SIGNED), 0, 0); BOOST_CHECK(size == ss.size()); } @@ -191,7 +191,7 @@ BOOST_AUTO_TEST_CASE(varints) // decode for (int i = 0; i < 100000; i++) { int j = -1; - ss >> VARINT(j); + ss >> VARINT(j, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_MESSAGE(i == j, "decoded:" << j << " expected:" << i); } @@ -205,21 +205,21 @@ BOOST_AUTO_TEST_CASE(varints) BOOST_AUTO_TEST_CASE(varints_bitpatterns) { CDataStream ss(SER_DISK, 0); - ss << VARINT(0); BOOST_CHECK_EQUAL(HexStr(ss), "00"); ss.clear(); - ss << VARINT(0x7f); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear(); - ss << VARINT((int8_t)0x7f); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear(); - ss << VARINT(0x80); BOOST_CHECK_EQUAL(HexStr(ss), "8000"); ss.clear(); + ss << VARINT(0, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "00"); ss.clear(); + ss << VARINT(0x7f, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear(); + ss << VARINT((int8_t)0x7f, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear(); + ss << VARINT(0x80, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "8000"); ss.clear(); ss << VARINT((uint8_t)0x80); BOOST_CHECK_EQUAL(HexStr(ss), "8000"); ss.clear(); - ss << VARINT(0x1234); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear(); - ss << VARINT((int16_t)0x1234); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear(); - ss << VARINT(0xffff); BOOST_CHECK_EQUAL(HexStr(ss), "82fe7f"); ss.clear(); + ss << VARINT(0x1234, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear(); + ss << VARINT((int16_t)0x1234, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear(); + ss << VARINT(0xffff, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "82fe7f"); ss.clear(); ss << VARINT((uint16_t)0xffff); BOOST_CHECK_EQUAL(HexStr(ss), "82fe7f"); ss.clear(); - ss << VARINT(0x123456); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear(); - ss << VARINT((int32_t)0x123456); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear(); + ss << VARINT(0x123456, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear(); + ss << VARINT((int32_t)0x123456, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear(); ss << VARINT(0x80123456U); BOOST_CHECK_EQUAL(HexStr(ss), "86ffc7e756"); ss.clear(); ss << VARINT((uint32_t)0x80123456U); BOOST_CHECK_EQUAL(HexStr(ss), "86ffc7e756"); ss.clear(); ss << VARINT(0xffffffff); BOOST_CHECK_EQUAL(HexStr(ss), "8efefefe7f"); ss.clear(); - ss << VARINT(0x7fffffffffffffffLL); BOOST_CHECK_EQUAL(HexStr(ss), "fefefefefefefefe7f"); ss.clear(); + ss << VARINT(0x7fffffffffffffffLL, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "fefefefefefefefe7f"); ss.clear(); ss << VARINT(0xffffffffffffffffULL); BOOST_CHECK_EQUAL(HexStr(ss), "80fefefefefefefefe7f"); ss.clear(); } diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 58f033cd89..84b61bea86 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -164,10 +164,27 @@ BOOST_AUTO_TEST_CASE(util_DateTimeStrFormat) BOOST_CHECK_EQUAL(DateTimeStrFormat("%Y-%m-%d %H:%M:%S", 0), "1970-01-01 00:00:00"); BOOST_CHECK_EQUAL(DateTimeStrFormat("%Y-%m-%d %H:%M:%S", 0x7FFFFFFF), "2038-01-19 03:14:07"); BOOST_CHECK_EQUAL(DateTimeStrFormat("%Y-%m-%d %H:%M:%S", 1317425777), "2011-09-30 23:36:17"); + BOOST_CHECK_EQUAL(DateTimeStrFormat("%Y-%m-%dT%H:%M:%SZ", 1317425777), "2011-09-30T23:36:17Z"); + BOOST_CHECK_EQUAL(DateTimeStrFormat("%H:%M:%SZ", 1317425777), "23:36:17Z"); BOOST_CHECK_EQUAL(DateTimeStrFormat("%Y-%m-%d %H:%M", 1317425777), "2011-09-30 23:36"); BOOST_CHECK_EQUAL(DateTimeStrFormat("%a, %d %b %Y %H:%M:%S +0000", 1317425777), "Fri, 30 Sep 2011 23:36:17 +0000"); } +BOOST_AUTO_TEST_CASE(util_FormatISO8601DateTime) +{ + BOOST_CHECK_EQUAL(FormatISO8601DateTime(1317425777), "2011-09-30T23:36:17Z"); +} + +BOOST_AUTO_TEST_CASE(util_FormatISO8601Date) +{ + BOOST_CHECK_EQUAL(FormatISO8601Date(1317425777), "2011-09-30"); +} + +BOOST_AUTO_TEST_CASE(util_FormatISO8601Time) +{ + BOOST_CHECK_EQUAL(FormatISO8601Time(1317425777), "23:36:17Z"); +} + class TestArgsManager : public ArgsManager { public: diff --git a/src/txdb.cpp b/src/txdb.cpp index 293d43c7b3..91d6c98430 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -324,7 +324,7 @@ public: void Unserialize(Stream &s) { unsigned int nCode = 0; // version - int nVersionDummy; + unsigned int nVersionDummy; ::Unserialize(s, VARINT(nVersionDummy)); // header code ::Unserialize(s, VARINT(nCode)); @@ -348,10 +348,10 @@ public: vout.assign(vAvail.size(), CTxOut()); for (unsigned int i = 0; i < vAvail.size(); i++) { if (vAvail[i]) - ::Unserialize(s, REF(CTxOutCompressor(vout[i]))); + ::Unserialize(s, CTxOutCompressor(vout[i])); } // coinbase height - ::Unserialize(s, VARINT(nHeight)); + ::Unserialize(s, VARINT(nHeight, VarIntMode::NONNEGATIVE_SIGNED)); } }; diff --git a/src/undo.h b/src/undo.h index 1f10c6652c..6fc25b9853 100644 --- a/src/undo.h +++ b/src/undo.h @@ -25,7 +25,7 @@ class TxInUndoSerializer public: template<typename Stream> void Serialize(Stream &s) const { - ::Serialize(s, VARINT(txout->nHeight * 2 + (txout->fCoinBase ? 1 : 0))); + ::Serialize(s, VARINT(txout->nHeight * 2 + (txout->fCoinBase ? 1 : 0), VarIntMode::NONNEGATIVE_SIGNED)); if (txout->nHeight > 0) { // Required to maintain compatibility with older undo format. ::Serialize(s, (unsigned char)0); @@ -51,10 +51,10 @@ public: // Old versions stored the version number for the last spend of // a transaction's outputs. Non-final spends were indicated with // height = 0. - int nVersionDummy; + unsigned int nVersionDummy; ::Unserialize(s, VARINT(nVersionDummy)); } - ::Unserialize(s, REF(CTxOutCompressor(REF(txout->out)))); + ::Unserialize(s, CTxOutCompressor(REF(txout->out))); } explicit TxInUndoDeserializer(Coin* coin) : txout(coin) {} @@ -76,7 +76,7 @@ public: uint64_t count = vprevout.size(); ::Serialize(s, COMPACTSIZE(REF(count))); for (const auto& prevout : vprevout) { - ::Serialize(s, REF(TxInUndoSerializer(&prevout))); + ::Serialize(s, TxInUndoSerializer(&prevout)); } } @@ -90,7 +90,7 @@ public: } vprevout.resize(count); for (auto& prevout : vprevout) { - ::Unserialize(s, REF(TxInUndoDeserializer(&prevout))); + ::Unserialize(s, TxInUndoDeserializer(&prevout)); } } }; diff --git a/src/util.cpp b/src/util.cpp index 82c99a3c2f..94f829ad32 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -4,7 +4,6 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <util.h> -#include <fs.h> #include <chainparamsbase.h> #include <random.h> @@ -315,12 +314,14 @@ static std::string LogTimestampStr(const std::string &str, std::atomic_bool *fSt if (*fStartedNewLine) { int64_t nTimeMicros = GetTimeMicros(); - strStamped = DateTimeStrFormat("%Y-%m-%d %H:%M:%S", nTimeMicros/1000000); - if (fLogTimeMicros) - strStamped += strprintf(".%06d", nTimeMicros%1000000); + strStamped = FormatISO8601DateTime(nTimeMicros/1000000); + if (fLogTimeMicros) { + strStamped.pop_back(); + strStamped += strprintf(".%06dZ", nTimeMicros%1000000); + } int64_t mocktime = GetMockTime(); if (mocktime) { - strStamped += " (mocktime: " + DateTimeStrFormat("%Y-%m-%d %H:%M:%S", mocktime) + ")"; + strStamped += " (mocktime: " + FormatISO8601DateTime(mocktime) + ")"; } strStamped += ' ' + str; } else diff --git a/src/utiltime.cpp b/src/utiltime.cpp index e908173135..8a861039b3 100644 --- a/src/utiltime.cpp +++ b/src/utiltime.cpp @@ -85,3 +85,15 @@ std::string DateTimeStrFormat(const char* pszFormat, int64_t nTime) ss << boost::posix_time::from_time_t(nTime); return ss.str(); } + +std::string FormatISO8601DateTime(int64_t nTime) { + return DateTimeStrFormat("%Y-%m-%dT%H:%M:%SZ", nTime); +} + +std::string FormatISO8601Date(int64_t nTime) { + return DateTimeStrFormat("%Y-%m-%d", nTime); +} + +std::string FormatISO8601Time(int64_t nTime) { + return DateTimeStrFormat("%H:%M:%SZ", nTime); +} diff --git a/src/utiltime.h b/src/utiltime.h index 56cc31da67..807c52ffaf 100644 --- a/src/utiltime.h +++ b/src/utiltime.h @@ -27,6 +27,14 @@ void SetMockTime(int64_t nMockTimeIn); int64_t GetMockTime(); void MilliSleep(int64_t n); +/** + * ISO 8601 formatting is preferred. Use the FormatISO8601{DateTime,Date,Time} + * helper functions if possible. + */ std::string DateTimeStrFormat(const char* pszFormat, int64_t nTime); +std::string FormatISO8601DateTime(int64_t nTime); +std::string FormatISO8601Date(int64_t nTime); +std::string FormatISO8601Time(int64_t nTime); + #endif // BITCOIN_UTILTIME_H diff --git a/src/validation.cpp b/src/validation.cpp index 51e40c17b5..bee890437e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -260,12 +260,12 @@ namespace { CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator) { + AssertLockHeld(cs_main); + // Find the first block the caller has in the main chain for (const uint256& hash : locator.vHave) { - BlockMap::iterator mi = mapBlockIndex.find(hash); - if (mi != mapBlockIndex.end()) - { - CBlockIndex* pindex = (*mi).second; + CBlockIndex* pindex = LookupBlockIndex(hash); + if (pindex) { if (chain.Contains(pindex)) return pindex; if (pindex->GetAncestor(chain.Height()) == chain.Tip()) { @@ -1267,13 +1267,12 @@ void static InvalidChainFound(CBlockIndex* pindexNew) LogPrintf("%s: invalid block=%s height=%d log2_work=%.8g date=%s\n", __func__, pindexNew->GetBlockHash().ToString(), pindexNew->nHeight, - log(pindexNew->nChainWork.getdouble())/log(2.0), DateTimeStrFormat("%Y-%m-%d %H:%M:%S", - pindexNew->GetBlockTime())); + log(pindexNew->nChainWork.getdouble())/log(2.0), FormatISO8601DateTime(pindexNew->GetBlockTime())); CBlockIndex *tip = chainActive.Tip(); assert (tip); LogPrintf("%s: current best=%s height=%d log2_work=%.8g date=%s\n", __func__, tip->GetBlockHash().ToString(), chainActive.Height(), log(tip->nChainWork.getdouble())/log(2.0), - DateTimeStrFormat("%Y-%m-%d %H:%M:%S", tip->GetBlockTime())); + FormatISO8601DateTime(tip->GetBlockTime())); CheckForkWarningConditions(); } @@ -1317,7 +1316,7 @@ bool CScriptCheck::operator()() { int GetSpendHeight(const CCoinsViewCache& inputs) { LOCK(cs_main); - CBlockIndex* pindexPrev = mapBlockIndex.find(inputs.GetBestBlock())->second; + CBlockIndex* pindexPrev = LookupBlockIndex(inputs.GetBestBlock()); return pindexPrev->nHeight + 1; } @@ -2229,7 +2228,7 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%.8g tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)", __func__, pindexNew->GetBlockHash().ToString(), pindexNew->nHeight, pindexNew->nVersion, log(pindexNew->nChainWork.getdouble())/log(2.0), (unsigned long)pindexNew->nChainTx, - DateTimeStrFormat("%Y-%m-%d %H:%M:%S", pindexNew->GetBlockTime()), + FormatISO8601DateTime(pindexNew->GetBlockTime()), GuessVerificationProgress(chainParams.TxData(), pindexNew), pcoinsTip->DynamicMemoryUsage() * (1.0 / (1<<20)), pcoinsTip->GetCacheSize()); if (!warningMessages.empty()) LogPrintf(" warning='%s'", boost::algorithm::join(warningMessages, ", ")); @@ -2781,7 +2780,11 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c } InvalidChainFound(pindex); - uiInterface.NotifyBlockTip(IsInitialBlockDownload(), pindex->pprev); + + // Only notify about a new block tip if the active chain was modified. + if (pindex_was_in_chain) { + uiInterface.NotifyBlockTip(IsInitialBlockDownload(), pindex->pprev); + } return true; } bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex *pindex) { @@ -2827,6 +2830,8 @@ bool ResetBlockFailureFlags(CBlockIndex *pindex) { CBlockIndex* CChainState::AddToBlockIndex(const CBlockHeader& block) { + AssertLockHeld(cs_main); + // Check for duplicate uint256 hash = block.GetHash(); BlockMap::iterator it = mapBlockIndex.find(hash); @@ -3273,7 +3278,6 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState& BlockMap::iterator miSelf = mapBlockIndex.find(hash); CBlockIndex *pindex = nullptr; if (hash != chainparams.GetConsensus().hashGenesisBlock) { - if (miSelf != mapBlockIndex.end()) { // Block header is already known. pindex = miSelf->second; @@ -3707,6 +3711,8 @@ fs::path GetBlockPosFilename(const CDiskBlockPos &pos, const char *prefix) CBlockIndex * CChainState::InsertBlockIndex(const uint256& hash) { + AssertLockHeld(cs_main); + if (hash.IsNull()) return nullptr; @@ -3834,6 +3840,8 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams) bool LoadChainTip(const CChainParams& chainparams) { + AssertLockHeld(cs_main); + if (chainActive.Tip() && chainActive.Tip()->GetBlockHash() == pcoinsTip->GetBestBlock()) return true; if (pcoinsTip->GetBestBlock().IsNull() && mapBlockIndex.size() == 1) { @@ -3847,16 +3855,17 @@ bool LoadChainTip(const CChainParams& chainparams) } // Load pointer to end of best chain - BlockMap::iterator it = mapBlockIndex.find(pcoinsTip->GetBestBlock()); - if (it == mapBlockIndex.end()) + CBlockIndex* pindex = LookupBlockIndex(pcoinsTip->GetBestBlock()); + if (!pindex) { return false; - chainActive.SetTip(it->second); + } + chainActive.SetTip(pindex); g_chainstate.PruneBlockIndexCandidates(); LogPrintf("Loaded best chain: hashBestChain=%s height=%d date=%s progress=%f\n", chainActive.Tip()->GetBlockHash().ToString(), chainActive.Height(), - DateTimeStrFormat("%Y-%m-%d %H:%M:%S", chainActive.Tip()->GetBlockTime()), + FormatISO8601DateTime(chainActive.Tip()->GetBlockTime()), GuessVerificationProgress(chainparams.TxData(), chainActive.Tip())); return true; } @@ -4297,26 +4306,31 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, CDiskB blkdat >> block; nRewind = blkdat.GetPos(); - // detect out of order blocks, and store them for later uint256 hash = block.GetHash(); - if (hash != chainparams.GetConsensus().hashGenesisBlock && mapBlockIndex.find(block.hashPrevBlock) == mapBlockIndex.end()) { - LogPrint(BCLog::REINDEX, "%s: Out of order block %s, parent %s not known\n", __func__, hash.ToString(), - block.hashPrevBlock.ToString()); - if (dbp) - mapBlocksUnknownParent.insert(std::make_pair(block.hashPrevBlock, *dbp)); - continue; - } - - // process in case the block isn't known yet - if (mapBlockIndex.count(hash) == 0 || (mapBlockIndex[hash]->nStatus & BLOCK_HAVE_DATA) == 0) { + { LOCK(cs_main); - CValidationState state; - if (g_chainstate.AcceptBlock(pblock, state, chainparams, nullptr, true, dbp, nullptr)) - nLoaded++; - if (state.IsError()) - break; - } else if (hash != chainparams.GetConsensus().hashGenesisBlock && mapBlockIndex[hash]->nHeight % 1000 == 0) { - LogPrint(BCLog::REINDEX, "Block Import: already had block %s at height %d\n", hash.ToString(), mapBlockIndex[hash]->nHeight); + // detect out of order blocks, and store them for later + if (hash != chainparams.GetConsensus().hashGenesisBlock && !LookupBlockIndex(block.hashPrevBlock)) { + LogPrint(BCLog::REINDEX, "%s: Out of order block %s, parent %s not known\n", __func__, hash.ToString(), + block.hashPrevBlock.ToString()); + if (dbp) + mapBlocksUnknownParent.insert(std::make_pair(block.hashPrevBlock, *dbp)); + continue; + } + + // process in case the block isn't known yet + CBlockIndex* pindex = LookupBlockIndex(hash); + if (!pindex || (pindex->nStatus & BLOCK_HAVE_DATA) == 0) { + CValidationState state; + if (g_chainstate.AcceptBlock(pblock, state, chainparams, nullptr, true, dbp, nullptr)) { + nLoaded++; + } + if (state.IsError()) { + break; + } + } else if (hash != chainparams.GetConsensus().hashGenesisBlock && pindex->nHeight % 1000 == 0) { + LogPrint(BCLog::REINDEX, "Block Import: already had block %s at height %d\n", hash.ToString(), pindex->nHeight); + } } // Activate the genesis block so normal node progress can continue @@ -4554,7 +4568,7 @@ void CChainState::CheckBlockIndex(const Consensus::Params& consensusParams) std::string CBlockFileInfo::ToString() const { - return strprintf("CBlockFileInfo(blocks=%u, size=%u, heights=%u...%u, time=%s...%s)", nBlocks, nSize, nHeightFirst, nHeightLast, DateTimeStrFormat("%Y-%m-%d", nTimeFirst), DateTimeStrFormat("%Y-%m-%d", nTimeLast)); + return strprintf("CBlockFileInfo(blocks=%u, size=%u, heights=%u...%u, time=%s...%s)", nBlocks, nSize, nHeightFirst, nHeightLast, FormatISO8601Date(nTimeFirst), FormatISO8601Date(nTimeLast)); } CBlockFileInfo* GetBlockFileInfo(size_t n) diff --git a/src/validation.h b/src/validation.h index 99cbfdf1ee..e780f453b2 100644 --- a/src/validation.h +++ b/src/validation.h @@ -428,6 +428,13 @@ public: /** Replay blocks that aren't fully applied to the database. */ bool ReplayBlocks(const CChainParams& params, CCoinsView* view); +inline CBlockIndex* LookupBlockIndex(const uint256& hash) +{ + AssertLockHeld(cs_main); + BlockMap::const_iterator it = mapBlockIndex.find(hash); + return it == mapBlockIndex.end() ? nullptr : it->second; +} + /** Find the last common block between the parameter chain and a locator. */ CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator); diff --git a/src/validationinterface.h b/src/validationinterface.h index 56ea698a2e..63097166af 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -56,6 +56,11 @@ void SyncWithValidationInterfaceQueue(); class CValidationInterface { protected: /** + * Protected destructor so that instances can only be deleted by derived classes. + * If that restriction is no longer desired, this should be made public and virtual. + */ + ~CValidationInterface() = default; + /** * Notifies listeners of updated block chain tip * * Called on a background thread. diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 458e770e03..52d6a291c9 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -18,8 +18,8 @@ class CCoinControl public: //! Custom change destination, if not set an address is generated CTxDestination destChange; - //! Custom change type, ignored if destChange is set, defaults to g_change_type - OutputType change_type; + //! Override the default change type if set, ignored if destChange is set + boost::optional<OutputType> m_change_type; //! If false, allows unselected inputs, but requires all selected inputs be used bool fAllowOtherInputs; //! Includes watch only addresses which match the ISMINE_WATCH_SOLVABLE criteria @@ -43,7 +43,7 @@ public: void SetNull() { destChange = CNoDestination(); - change_type = g_change_type; + m_change_type.reset(); fAllowOtherInputs = false; fAllowWatchOnly = false; setSelected.clear(); diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp new file mode 100644 index 0000000000..8596ad2adc --- /dev/null +++ b/src/wallet/coinselection.cpp @@ -0,0 +1,300 @@ +// Copyright (c) 2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <wallet/coinselection.h> +#include <util.h> +#include <utilmoneystr.h> + +// Descending order comparator +struct { + bool operator()(const CInputCoin& a, const CInputCoin& b) const + { + return a.effective_value > b.effective_value; + } +} descending; + +/* + * This is the Branch and Bound Coin Selection algorithm designed by Murch. It searches for an input + * set that can pay for the spending target and does not exceed the spending target by more than the + * cost of creating and spending a change output. The algorithm uses a depth-first search on a binary + * tree. In the binary tree, each node corresponds to the inclusion or the omission of a UTXO. UTXOs + * are sorted by their effective values and the trees is explored deterministically per the inclusion + * branch first. At each node, the algorithm checks whether the selection is within the target range. + * While the selection has not reached the target range, more UTXOs are included. When a selection's + * value exceeds the target range, the complete subtree deriving from this selection can be omitted. + * At that point, the last included UTXO is deselected and the corresponding omission branch explored + * instead. The search ends after the complete tree has been searched or after a limited number of tries. + * + * The search continues to search for better solutions after one solution has been found. The best + * solution is chosen by minimizing the waste metric. The waste metric is defined as the cost to + * spend the current inputs at the given fee rate minus the long term expected cost to spend the + * inputs, plus the amount the selection exceeds the spending target: + * + * waste = selectionTotal - target + inputs × (currentFeeRate - longTermFeeRate) + * + * The algorithm uses two additional optimizations. A lookahead keeps track of the total value of + * the unexplored UTXOs. A subtree is not explored if the lookahead indicates that the target range + * cannot be reached. Further, it is unnecessary to test equivalent combinations. This allows us + * to skip testing the inclusion of UTXOs that match the effective value and waste of an omitted + * predecessor. + * + * The Branch and Bound algorithm is described in detail in Murch's Master Thesis: + * https://murch.one/wp-content/uploads/2016/11/erhardt2016coinselection.pdf + * + * @param const std::vector<CInputCoin>& utxo_pool The set of UTXOs that we are choosing from. + * These UTXOs will be sorted in descending order by effective value and the CInputCoins' + * values are their effective values. + * @param const CAmount& target_value This is the value that we want to select. It is the lower + * bound of the range. + * @param const CAmount& cost_of_change This is the cost of creating and spending a change output. + * This plus target_value is the upper bound of the range. + * @param std::set<CInputCoin>& out_set -> This is an output parameter for the set of CInputCoins + * that have been selected. + * @param CAmount& value_ret -> This is an output parameter for the total value of the CInputCoins + * that were selected. + * @param CAmount not_input_fees -> The fees that need to be paid for the outputs and fixed size + * overhead (version, locktime, marker and flag) + */ + +static const size_t TOTAL_TRIES = 100000; + +bool SelectCoinsBnB(std::vector<CInputCoin>& utxo_pool, const CAmount& target_value, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret, CAmount not_input_fees) +{ + out_set.clear(); + CAmount curr_value = 0; + + std::vector<bool> curr_selection; // select the utxo at this index + curr_selection.reserve(utxo_pool.size()); + CAmount actual_target = not_input_fees + target_value; + + // Calculate curr_available_value + CAmount curr_available_value = 0; + for (const CInputCoin& utxo : utxo_pool) { + // Assert that this utxo is not negative. It should never be negative, effective value calculation should have removed it + assert(utxo.effective_value > 0); + curr_available_value += utxo.effective_value; + } + if (curr_available_value < actual_target) { + return false; + } + + // Sort the utxo_pool + std::sort(utxo_pool.begin(), utxo_pool.end(), descending); + + CAmount curr_waste = 0; + std::vector<bool> best_selection; + CAmount best_waste = MAX_MONEY; + + // Depth First search loop for choosing the UTXOs + for (size_t i = 0; i < TOTAL_TRIES; ++i) { + // Conditions for starting a backtrack + bool backtrack = false; + if (curr_value + curr_available_value < actual_target || // Cannot possibly reach target with the amount remaining in the curr_available_value. + curr_value > actual_target + cost_of_change || // Selected value is out of range, go back and try other branch + (curr_waste > best_waste && (utxo_pool.at(0).fee - utxo_pool.at(0).long_term_fee) > 0)) { // Don't select things which we know will be more wasteful if the waste is increasing + backtrack = true; + } else if (curr_value >= actual_target) { // Selected value is within range + curr_waste += (curr_value - actual_target); // This is the excess value which is added to the waste for the below comparison + // Adding another UTXO after this check could bring the waste down if the long term fee is higher than the current fee. + // However we are not going to explore that because this optimization for the waste is only done when we have hit our target + // value. Adding any more UTXOs will be just burning the UTXO; it will go entirely to fees. Thus we aren't going to + // explore any more UTXOs to avoid burning money like that. + if (curr_waste <= best_waste) { + best_selection = curr_selection; + best_selection.resize(utxo_pool.size()); + best_waste = curr_waste; + } + curr_waste -= (curr_value - actual_target); // Remove the excess value as we will be selecting different coins now + backtrack = true; + } + + // Backtracking, moving backwards + if (backtrack) { + // Walk backwards to find the last included UTXO that still needs to have its omission branch traversed. + while (!curr_selection.empty() && !curr_selection.back()) { + curr_selection.pop_back(); + curr_available_value += utxo_pool.at(curr_selection.size()).effective_value; + }; + + if (curr_selection.empty()) { // We have walked back to the first utxo and no branch is untraversed. All solutions searched + break; + } + + // Output was included on previous iterations, try excluding now. + curr_selection.back() = false; + CInputCoin& utxo = utxo_pool.at(curr_selection.size() - 1); + curr_value -= utxo.effective_value; + curr_waste -= utxo.fee - utxo.long_term_fee; + } else { // Moving forwards, continuing down this branch + CInputCoin& utxo = utxo_pool.at(curr_selection.size()); + + // Remove this utxo from the curr_available_value utxo amount + curr_available_value -= utxo.effective_value; + + // Avoid searching a branch if the previous UTXO has the same value and same waste and was excluded. Since the ratio of fee to + // long term fee is the same, we only need to check if one of those values match in order to know that the waste is the same. + if (!curr_selection.empty() && !curr_selection.back() && + utxo.effective_value == utxo_pool.at(curr_selection.size() - 1).effective_value && + utxo.fee == utxo_pool.at(curr_selection.size() - 1).fee) { + curr_selection.push_back(false); + } else { + // Inclusion branch first (Largest First Exploration) + curr_selection.push_back(true); + curr_value += utxo.effective_value; + curr_waste += utxo.fee - utxo.long_term_fee; + } + } + } + + // Check for solution + if (best_selection.empty()) { + return false; + } + + // Set output set + value_ret = 0; + for (size_t i = 0; i < best_selection.size(); ++i) { + if (best_selection.at(i)) { + out_set.insert(utxo_pool.at(i)); + value_ret += utxo_pool.at(i).txout.nValue; + } + } + + return true; +} + +static void ApproximateBestSubset(const std::vector<CInputCoin>& vValue, const CAmount& nTotalLower, const CAmount& nTargetValue, + std::vector<char>& vfBest, CAmount& nBest, int iterations = 1000) +{ + std::vector<char> vfIncluded; + + vfBest.assign(vValue.size(), true); + nBest = nTotalLower; + + FastRandomContext insecure_rand; + + for (int nRep = 0; nRep < iterations && nBest != nTargetValue; nRep++) + { + vfIncluded.assign(vValue.size(), false); + CAmount nTotal = 0; + bool fReachedTarget = false; + for (int nPass = 0; nPass < 2 && !fReachedTarget; nPass++) + { + for (unsigned int i = 0; i < vValue.size(); i++) + { + //The solver here uses a randomized algorithm, + //the randomness serves no real security purpose but is just + //needed to prevent degenerate behavior and it is important + //that the rng is fast. We do not use a constant random sequence, + //because there may be some privacy improvement by making + //the selection random. + if (nPass == 0 ? insecure_rand.randbool() : !vfIncluded[i]) + { + nTotal += vValue[i].txout.nValue; + vfIncluded[i] = true; + if (nTotal >= nTargetValue) + { + fReachedTarget = true; + if (nTotal < nBest) + { + nBest = nTotal; + vfBest = vfIncluded; + } + nTotal -= vValue[i].txout.nValue; + vfIncluded[i] = false; + } + } + } + } + } +} + +bool KnapsackSolver(const CAmount& nTargetValue, std::vector<CInputCoin>& vCoins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet) +{ + setCoinsRet.clear(); + nValueRet = 0; + + // List of values less than target + boost::optional<CInputCoin> coinLowestLarger; + std::vector<CInputCoin> vValue; + CAmount nTotalLower = 0; + + random_shuffle(vCoins.begin(), vCoins.end(), GetRandInt); + + for (const CInputCoin &coin : vCoins) + { + if (coin.txout.nValue == nTargetValue) + { + setCoinsRet.insert(coin); + nValueRet += coin.txout.nValue; + return true; + } + else if (coin.txout.nValue < nTargetValue + MIN_CHANGE) + { + vValue.push_back(coin); + nTotalLower += coin.txout.nValue; + } + else if (!coinLowestLarger || coin.txout.nValue < coinLowestLarger->txout.nValue) + { + coinLowestLarger = coin; + } + } + + if (nTotalLower == nTargetValue) + { + for (const auto& input : vValue) + { + setCoinsRet.insert(input); + nValueRet += input.txout.nValue; + } + return true; + } + + if (nTotalLower < nTargetValue) + { + if (!coinLowestLarger) + return false; + setCoinsRet.insert(coinLowestLarger.get()); + nValueRet += coinLowestLarger->txout.nValue; + return true; + } + + // Solve subset sum by stochastic approximation + std::sort(vValue.begin(), vValue.end(), descending); + std::vector<char> vfBest; + CAmount nBest; + + ApproximateBestSubset(vValue, nTotalLower, nTargetValue, vfBest, nBest); + if (nBest != nTargetValue && nTotalLower >= nTargetValue + MIN_CHANGE) + ApproximateBestSubset(vValue, nTotalLower, nTargetValue + MIN_CHANGE, vfBest, nBest); + + // If we have a bigger coin and (either the stochastic approximation didn't find a good solution, + // or the next bigger coin is closer), return the bigger coin + if (coinLowestLarger && + ((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || coinLowestLarger->txout.nValue <= nBest)) + { + setCoinsRet.insert(coinLowestLarger.get()); + nValueRet += coinLowestLarger->txout.nValue; + } + else { + for (unsigned int i = 0; i < vValue.size(); i++) + if (vfBest[i]) + { + setCoinsRet.insert(vValue[i]); + nValueRet += vValue[i].txout.nValue; + } + + if (LogAcceptCategory(BCLog::SELECTCOINS)) { + LogPrint(BCLog::SELECTCOINS, "SelectCoins() best subset: "); + for (unsigned int i = 0; i < vValue.size(); i++) { + if (vfBest[i]) { + LogPrint(BCLog::SELECTCOINS, "%s ", FormatMoney(vValue[i].txout.nValue)); + } + } + LogPrint(BCLog::SELECTCOINS, "total %s\n", FormatMoney(nBest)); + } + } + + return true; +} diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h new file mode 100644 index 0000000000..4d1a43bc17 --- /dev/null +++ b/src/wallet/coinselection.h @@ -0,0 +1,54 @@ +// Copyright (c) 2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_COINSELECTION_H +#define BITCOIN_COINSELECTION_H + +#include <amount.h> +#include <primitives/transaction.h> +#include <random.h> + +//! target minimum change amount +static const CAmount MIN_CHANGE = CENT; +//! final minimum change amount after paying for fees +static const CAmount MIN_FINAL_CHANGE = MIN_CHANGE/2; + +class CInputCoin { +public: + CInputCoin(const CTransactionRef& tx, unsigned int i) + { + if (!tx) + throw std::invalid_argument("tx should not be null"); + if (i >= tx->vout.size()) + throw std::out_of_range("The output index is out of range"); + + outpoint = COutPoint(tx->GetHash(), i); + txout = tx->vout[i]; + effective_value = txout.nValue; + } + + COutPoint outpoint; + CTxOut txout; + CAmount effective_value; + CAmount fee = 0; + CAmount long_term_fee = 0; + + bool operator<(const CInputCoin& rhs) const { + return outpoint < rhs.outpoint; + } + + bool operator!=(const CInputCoin& rhs) const { + return outpoint != rhs.outpoint; + } + + bool operator==(const CInputCoin& rhs) const { + return outpoint == rhs.outpoint; + } +}; + +bool SelectCoinsBnB(std::vector<CInputCoin>& utxo_pool, const CAmount& target_value, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret, CAmount not_input_fees); + +// Original coin selection algorithm as a fallback +bool KnapsackSolver(const CAmount& nTargetValue, std::vector<CInputCoin>& vCoins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet); +#endif // BITCOIN_COINSELECTION_H diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 9cae660c60..82a5017de0 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -16,33 +16,6 @@ #include <util.h> #include <net.h> -// Calculate the size of the transaction assuming all signatures are max size -// Use DummySignatureCreator, which inserts 72 byte signatures everywhere. -// TODO: re-use this in CWallet::CreateTransaction (right now -// CreateTransaction uses the constructed dummy-signed tx to do a priority -// calculation, but we should be able to refactor after priority is removed). -// NOTE: this requires that all inputs must be in mapWallet (eg the tx should -// be IsAllFromMe). -static int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet) -{ - CMutableTransaction txNew(tx); - std::vector<CInputCoin> vCoins; - // Look up the inputs. We should have already checked that this transaction - // IsAllFromMe(ISMINE_SPENDABLE), so every input should already be in our - // wallet, with a valid index into the vout array. - for (auto& input : tx.vin) { - const auto mi = wallet->mapWallet.find(input.prevout.hash); - assert(mi != wallet->mapWallet.end() && input.prevout.n < mi->second.tx->vout.size()); - vCoins.emplace_back(CInputCoin(&(mi->second), input.prevout.n)); - } - if (!wallet->DummySignTx(txNew, vCoins)) { - // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) - // implies that we can sign for every input. - return -1; - } - return GetVirtualTransactionSize(txNew); -} - //! Check whether transaction has descendant in wallet or mempool, or has been //! mined, or conflicts with a mined transaction. Return a feebumper::Result. static feebumper::Result PreconditionChecks(const CWallet* wallet, const CWalletTx& wtx, std::vector<std::string>& errors) @@ -262,23 +235,20 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti return result; } - CWalletTx wtxBumped(wallet, MakeTransactionRef(std::move(mtx))); // commit/broadcast the tx + CTransactionRef tx = MakeTransactionRef(std::move(mtx)); + mapValue_t mapValue = oldWtx.mapValue; + mapValue["replaces_txid"] = oldWtx.GetHash().ToString(); + CReserveKey reservekey(wallet); - wtxBumped.mapValue = oldWtx.mapValue; - wtxBumped.mapValue["replaces_txid"] = oldWtx.GetHash().ToString(); - wtxBumped.vOrderForm = oldWtx.vOrderForm; - wtxBumped.strFromAccount = oldWtx.strFromAccount; - wtxBumped.fTimeReceivedIsTxTime = true; - wtxBumped.fFromMe = true; CValidationState state; - if (!wallet->CommitTransaction(wtxBumped, reservekey, g_connman.get(), state)) { + if (!wallet->CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, oldWtx.strFromAccount, reservekey, g_connman.get(), state)) { // NOTE: CommitTransaction never returns false, so this should never happen. errors.push_back(strprintf("The transaction was rejected: %s", FormatStateMessage(state))); return Result::WALLET_ERROR; } - bumped_txid = wtxBumped.GetHash(); + bumped_txid = tx->GetHash(); if (state.IsInvalid()) { // This can happen if the mempool rejected the transaction. Report // what happened in the "errors" response. @@ -286,7 +256,7 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti } // mark the original tx as bumped - if (!wallet->MarkReplaced(oldWtx.GetHash(), wtxBumped.GetHash())) { + if (!wallet->MarkReplaced(oldWtx.GetHash(), bumped_txid)) { // TODO: see if JSON-RPC has a standard way of returning a response // along with an exception. It would be good to return information about // wtxBumped to the caller even if marking the original transaction diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index 385fdc963a..03c32d3b97 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -21,6 +21,22 @@ CAmount GetRequiredFee(unsigned int nTxBytes) CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc) { + CAmount fee_needed = GetMinimumFeeRate(coin_control, pool, estimator, feeCalc).GetFee(nTxBytes); + // Always obey the maximum + if (fee_needed > maxTxFee) { + fee_needed = maxTxFee; + if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE; + } + return fee_needed; +} + +CFeeRate GetRequiredFeeRate() +{ + return std::max(CWallet::minTxFee, ::minRelayTxFee); +} + +CFeeRate GetMinimumFeeRate(const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc) +{ /* User control of how to calculate fee uses the following parameter precedence: 1. coin_control.m_feerate 2. coin_control.m_confirm_target @@ -28,15 +44,15 @@ CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, c 4. nTxConfirmTarget (user-set global variable) The first parameter that is set is used. */ - CAmount fee_needed; + CFeeRate feerate_needed ; if (coin_control.m_feerate) { // 1. - fee_needed = coin_control.m_feerate->GetFee(nTxBytes); + feerate_needed = *(coin_control.m_feerate); if (feeCalc) feeCalc->reason = FeeReason::PAYTXFEE; // Allow to override automatic min/max check over coin control instance - if (coin_control.fOverrideFeeRate) return fee_needed; + if (coin_control.fOverrideFeeRate) return feerate_needed; } else if (!coin_control.m_confirm_target && ::payTxFee != CFeeRate(0)) { // 3. TODO: remove magic value of 0 for global payTxFee - fee_needed = ::payTxFee.GetFee(nTxBytes); + feerate_needed = ::payTxFee; if (feeCalc) feeCalc->reason = FeeReason::PAYTXFEE; } else { // 2. or 4. @@ -48,38 +64,32 @@ CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, c if (coin_control.m_fee_mode == FeeEstimateMode::CONSERVATIVE) conservative_estimate = true; else if (coin_control.m_fee_mode == FeeEstimateMode::ECONOMICAL) conservative_estimate = false; - fee_needed = estimator.estimateSmartFee(target, feeCalc, conservative_estimate).GetFee(nTxBytes); - if (fee_needed == 0) { + feerate_needed = estimator.estimateSmartFee(target, feeCalc, conservative_estimate); + if (feerate_needed == CFeeRate(0)) { // if we don't have enough data for estimateSmartFee, then use fallbackFee - fee_needed = CWallet::fallbackFee.GetFee(nTxBytes); + feerate_needed = CWallet::fallbackFee; if (feeCalc) feeCalc->reason = FeeReason::FALLBACK; // directly return if fallback fee is disabled (feerate 0 == disabled) - if (CWallet::fallbackFee.GetFee(1000) == 0) return fee_needed; + if (CWallet::fallbackFee == CFeeRate(0)) return feerate_needed; } // Obey mempool min fee when using smart fee estimation - CAmount min_mempool_fee = pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(nTxBytes); - if (fee_needed < min_mempool_fee) { - fee_needed = min_mempool_fee; + CFeeRate min_mempool_feerate = pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); + if (feerate_needed < min_mempool_feerate) { + feerate_needed = min_mempool_feerate; if (feeCalc) feeCalc->reason = FeeReason::MEMPOOL_MIN; } } // prevent user from paying a fee below minRelayTxFee or minTxFee - CAmount required_fee = GetRequiredFee(nTxBytes); - if (required_fee > fee_needed) { - fee_needed = required_fee; + CFeeRate required_feerate = GetRequiredFeeRate(); + if (required_feerate > feerate_needed) { + feerate_needed = required_feerate; if (feeCalc) feeCalc->reason = FeeReason::REQUIRED; } - // But always obey the maximum - if (fee_needed > maxTxFee) { - fee_needed = maxTxFee; - if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE; - } - return fee_needed; + return feerate_needed; } - CFeeRate GetDiscardRate(const CBlockPolicyEstimator& estimator) { unsigned int highest_target = estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); diff --git a/src/wallet/fees.h b/src/wallet/fees.h index 225aff08ad..a627af70b0 100644 --- a/src/wallet/fees.h +++ b/src/wallet/fees.h @@ -27,6 +27,18 @@ CAmount GetRequiredFee(unsigned int nTxBytes); CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc); /** + * Return the minimum required feerate taking into account the + * floating relay feerate and user set minimum transaction feerate + */ +CFeeRate GetRequiredFeeRate(); + +/** + * Estimate the minimum fee rate considering user set parameters + * and the required fee + */ +CFeeRate GetMinimumFeeRate(const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc); + +/** * Return the maximum feerate for discarding change. */ CFeeRate GetDiscardRate(const CBlockPolicyEstimator& estimator); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index e028cf4210..61481e01b6 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -17,7 +17,7 @@ std::string GetWalletHelpString(bool showDebug) { std::string strUsage = HelpMessageGroup(_("Wallet options:")); - strUsage += HelpMessageOpt("-addresstype", strprintf("What type of addresses to use (\"legacy\", \"p2sh-segwit\", or \"bech32\", default: \"%s\")", FormatOutputType(OUTPUT_TYPE_DEFAULT))); + strUsage += HelpMessageOpt("-addresstype", strprintf("What type of addresses to use (\"legacy\", \"p2sh-segwit\", or \"bech32\", default: \"%s\")", FormatOutputType(DEFAULT_ADDRESS_TYPE))); strUsage += HelpMessageOpt("-changetype", "What type of change to use (\"legacy\", \"p2sh-segwit\", or \"bech32\"). Default is same as -addresstype, except when -addresstype=p2sh-segwit a native segwit output is used when sending to a native segwit address)"); strUsage += HelpMessageOpt("-disablewallet", _("Do not load the wallet and disable wallet RPC calls")); strUsage += HelpMessageOpt("-discardfee=<amt>", strprintf(_("The fee rate (in %s/kB) that indicates your tolerance for discarding change by adding it to the fee (default: %s). " @@ -181,18 +181,6 @@ bool WalletParameterInteraction() bSpendZeroConfChange = gArgs.GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE); fWalletRbf = gArgs.GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF); - g_address_type = ParseOutputType(gArgs.GetArg("-addresstype", "")); - if (g_address_type == OUTPUT_TYPE_NONE) { - return InitError(strprintf("Unknown address type '%s'", gArgs.GetArg("-addresstype", ""))); - } - - // If changetype is set in config file or parameter, check that it's valid. - // Default to OUTPUT_TYPE_NONE if not set. - g_change_type = ParseOutputType(gArgs.GetArg("-changetype", ""), OUTPUT_TYPE_NONE); - if (g_change_type == OUTPUT_TYPE_NONE && !gArgs.GetArg("-changetype", "").empty()) { - return InitError(strprintf("Unknown change type '%s'", gArgs.GetArg("-changetype", ""))); - } - return true; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 0edc8d8d66..1721bc6df6 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -28,10 +28,6 @@ #include <univalue.h> -std::string static EncodeDumpTime(int64_t nTime) { - return DateTimeStrFormat("%Y-%m-%dT%H:%M:%SZ", nTime); -} - int64_t static DecodeDumpTime(const std::string &str) { static const boost::posix_time::ptime epoch = boost::posix_time::from_time_t(0); static const std::locale loc(std::locale::classic(), @@ -87,7 +83,7 @@ bool GetWalletAddressesForKey(CWallet * const pwallet, const CKeyID &keyid, std: } } if (!fLabelFound) { - strAddr = EncodeDestination(GetDestinationForKey(key.GetPubKey(), g_address_type)); + strAddr = EncodeDestination(GetDestinationForKey(key.GetPubKey(), pwallet->m_default_address_type)); } return fLabelFound; } @@ -354,9 +350,10 @@ UniValue importprunedfunds(const JSONRPCRequest& request) if (merkleBlock.txn.ExtractMatches(vMatch, vIndex) == merkleBlock.header.hashMerkleRoot) { LOCK(cs_main); - - if (!mapBlockIndex.count(merkleBlock.header.GetHash()) || !chainActive.Contains(mapBlockIndex[merkleBlock.header.GetHash()])) + const CBlockIndex* pindex = LookupBlockIndex(merkleBlock.header.GetHash()); + if (!pindex || !chainActive.Contains(pindex)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain"); + } std::vector<uint256>::const_iterator it; if ((it = std::find(vMatch.begin(), vMatch.end(), hashTx))==vMatch.end()) { @@ -722,9 +719,9 @@ UniValue dumpwallet(const JSONRPCRequest& request) // produce output file << strprintf("# Wallet dump created by Bitcoin %s\n", CLIENT_BUILD); - file << strprintf("# * Created on %s\n", EncodeDumpTime(GetTime())); + file << strprintf("# * Created on %s\n", FormatISO8601DateTime(GetTime())); file << strprintf("# * Best block at time of backup was %i (%s),\n", chainActive.Height(), chainActive.Tip()->GetBlockHash().ToString()); - file << strprintf("# mined on %s\n", EncodeDumpTime(chainActive.Tip()->GetBlockTime())); + file << strprintf("# mined on %s\n", FormatISO8601DateTime(chainActive.Tip()->GetBlockTime())); file << "\n"; // add the base58check encoded extended master if the wallet uses HD @@ -741,7 +738,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) } for (std::vector<std::pair<int64_t, CKeyID> >::const_iterator it = vKeyBirth.begin(); it != vKeyBirth.end(); it++) { const CKeyID &keyid = it->second; - std::string strTime = EncodeDumpTime(it->first); + std::string strTime = FormatISO8601DateTime(it->first); std::string strAddr; std::string strLabel; CKey key; @@ -769,7 +766,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) // get birth times for scripts with metadata auto it = pwallet->m_script_metadata.find(scriptid); if (it != pwallet->m_script_metadata.end()) { - create_time = EncodeDumpTime(it->second.nCreateTime); + create_time = FormatISO8601DateTime(it->second.nCreateTime); } if(pwallet->GetCScript(scriptid, script)) { file << strprintf("%s %s script=1", HexStr(script.begin(), script.end()), create_time); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 457abec1bc..fd13d8c542 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -95,7 +95,7 @@ void WalletTxToJSON(const CWalletTx& wtx, UniValue& entry) { entry.pushKV("blockhash", wtx.hashBlock.GetHex()); entry.pushKV("blockindex", wtx.nIndex); - entry.pushKV("blocktime", mapBlockIndex[wtx.hashBlock]->GetBlockTime()); + entry.pushKV("blocktime", LookupBlockIndex(wtx.hashBlock)->GetBlockTime()); } else { entry.pushKV("trusted", wtx.IsTrusted()); } @@ -162,10 +162,10 @@ UniValue getnewaddress(const JSONRPCRequest& request) if (!request.params[0].isNull()) strAccount = AccountFromValue(request.params[0]); - OutputType output_type = g_address_type; + OutputType output_type = pwallet->m_default_address_type; if (!request.params[1].isNull()) { - output_type = ParseOutputType(request.params[1].get_str(), g_address_type); - if (output_type == OUTPUT_TYPE_NONE) { + output_type = ParseOutputType(request.params[1].get_str(), pwallet->m_default_address_type); + if (output_type == OutputType::NONE) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[1].get_str())); } } @@ -259,10 +259,10 @@ UniValue getrawchangeaddress(const JSONRPCRequest& request) pwallet->TopUpKeyPool(); } - OutputType output_type = g_change_type != OUTPUT_TYPE_NONE ? g_change_type : g_address_type; + OutputType output_type = pwallet->m_default_change_type != OutputType::NONE ? pwallet->m_default_change_type : pwallet->m_default_address_type; if (!request.params[0].isNull()) { output_type = ParseOutputType(request.params[0].get_str(), output_type); - if (output_type == OUTPUT_TYPE_NONE) { + if (output_type == OutputType::NONE) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[0].get_str())); } } @@ -404,7 +404,7 @@ UniValue getaddressesbyaccount(const JSONRPCRequest& request) return ret; } -static void SendMoney(CWallet * const pwallet, const CTxDestination &address, CAmount nValue, bool fSubtractFeeFromAmount, CWalletTx& wtxNew, const CCoinControl& coin_control) +static CTransactionRef SendMoney(CWallet * const pwallet, const CTxDestination &address, CAmount nValue, bool fSubtractFeeFromAmount, const CCoinControl& coin_control, mapValue_t mapValue, std::string fromAccount) { CAmount curBalance = pwallet->GetBalance(); @@ -430,16 +430,18 @@ static void SendMoney(CWallet * const pwallet, const CTxDestination &address, CA int nChangePosRet = -1; CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount}; vecSend.push_back(recipient); - if (!pwallet->CreateTransaction(vecSend, wtxNew, reservekey, nFeeRequired, nChangePosRet, strError, coin_control)) { + CTransactionRef tx; + if (!pwallet->CreateTransaction(vecSend, tx, reservekey, nFeeRequired, nChangePosRet, strError, coin_control)) { if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance) strError = strprintf("Error: This transaction requires a transaction fee of at least %s", FormatMoney(nFeeRequired)); throw JSONRPCError(RPC_WALLET_ERROR, strError); } CValidationState state; - if (!pwallet->CommitTransaction(wtxNew, reservekey, g_connman.get(), state)) { + if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, std::move(fromAccount), reservekey, g_connman.get(), state)) { strError = strprintf("Error: The transaction was rejected! Reason given: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strError); } + return tx; } UniValue sendtoaddress(const JSONRPCRequest& request) @@ -498,11 +500,11 @@ UniValue sendtoaddress(const JSONRPCRequest& request) throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount for send"); // Wallet comments - CWalletTx wtx; + mapValue_t mapValue; if (!request.params[2].isNull() && !request.params[2].get_str().empty()) - wtx.mapValue["comment"] = request.params[2].get_str(); + mapValue["comment"] = request.params[2].get_str(); if (!request.params[3].isNull() && !request.params[3].get_str().empty()) - wtx.mapValue["to"] = request.params[3].get_str(); + mapValue["to"] = request.params[3].get_str(); bool fSubtractFeeFromAmount = false; if (!request.params[4].isNull()) { @@ -527,9 +529,8 @@ UniValue sendtoaddress(const JSONRPCRequest& request) EnsureWalletIsUnlocked(pwallet); - SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, wtx, coin_control); - - return wtx.GetHash().GetHex(); + CTransactionRef tx = SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, coin_control, std::move(mapValue), {} /* fromAccount */); + return tx->GetHash().GetHex(); } UniValue listaddressgroupings(const JSONRPCRequest& request) @@ -995,12 +996,11 @@ UniValue sendfrom(const JSONRPCRequest& request) if (!request.params[3].isNull()) nMinDepth = request.params[3].get_int(); - CWalletTx wtx; - wtx.strFromAccount = strAccount; + mapValue_t mapValue; if (!request.params[4].isNull() && !request.params[4].get_str().empty()) - wtx.mapValue["comment"] = request.params[4].get_str(); + mapValue["comment"] = request.params[4].get_str(); if (!request.params[5].isNull() && !request.params[5].get_str().empty()) - wtx.mapValue["to"] = request.params[5].get_str(); + mapValue["to"] = request.params[5].get_str(); EnsureWalletIsUnlocked(pwallet); @@ -1010,9 +1010,8 @@ UniValue sendfrom(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Account has insufficient funds"); CCoinControl no_coin_control; // This is a deprecated API - SendMoney(pwallet, dest, nAmount, false, wtx, no_coin_control); - - return wtx.GetHash().GetHex(); + CTransactionRef tx = SendMoney(pwallet, dest, nAmount, false, no_coin_control, std::move(mapValue), std::move(strAccount)); + return tx->GetHash().GetHex(); } @@ -1083,10 +1082,9 @@ UniValue sendmany(const JSONRPCRequest& request) if (!request.params[2].isNull()) nMinDepth = request.params[2].get_int(); - CWalletTx wtx; - wtx.strFromAccount = strAccount; + mapValue_t mapValue; if (!request.params[3].isNull() && !request.params[3].get_str().empty()) - wtx.mapValue["comment"] = request.params[3].get_str(); + mapValue["comment"] = request.params[3].get_str(); UniValue subtractFeeFromAmount(UniValue::VARR); if (!request.params[4].isNull()) @@ -1152,16 +1150,17 @@ UniValue sendmany(const JSONRPCRequest& request) CAmount nFeeRequired = 0; int nChangePosRet = -1; std::string strFailReason; - bool fCreated = pwallet->CreateTransaction(vecSend, wtx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coin_control); + CTransactionRef tx; + bool fCreated = pwallet->CreateTransaction(vecSend, tx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coin_control); if (!fCreated) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); CValidationState state; - if (!pwallet->CommitTransaction(wtx, keyChange, g_connman.get(), state)) { + if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, std::move(strAccount), keyChange, g_connman.get(), state)) { strFailReason = strprintf("Transaction commit failed:: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strFailReason); } - return wtx.GetHash().GetHex(); + return tx->GetHash().GetHex(); } UniValue addmultisigaddress(const JSONRPCRequest& request) @@ -1222,10 +1221,10 @@ UniValue addmultisigaddress(const JSONRPCRequest& request) } } - OutputType output_type = g_address_type; + OutputType output_type = pwallet->m_default_address_type; if (!request.params[3].isNull()) { output_type = ParseOutputType(request.params[3].get_str(), output_type); - if (output_type == OUTPUT_TYPE_NONE) { + if (output_type == OutputType::NONE) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[3].get_str())); } } @@ -2043,11 +2042,10 @@ UniValue listsinceblock(const JSONRPCRequest& request) uint256 blockId; blockId.SetHex(request.params[0].get_str()); - BlockMap::iterator it = mapBlockIndex.find(blockId); - if (it == mapBlockIndex.end()) { + paltindex = pindex = LookupBlockIndex(blockId); + if (!pindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); } - paltindex = pindex = it->second; if (chainActive[pindex->nHeight] != pindex) { // the block being asked for is a part of a deactivated chain; // we don't want to depend on its perceived height in the block @@ -2365,8 +2363,6 @@ UniValue walletpassphrase(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (request.fHelp) - return true; if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called."); } @@ -2431,8 +2427,6 @@ UniValue walletpassphrasechange(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (request.fHelp) - return true; if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrasechange was called."); } @@ -2487,8 +2481,6 @@ UniValue walletlock(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (request.fHelp) - return true; if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletlock was called."); } @@ -2534,8 +2526,6 @@ UniValue encryptwallet(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (request.fHelp) - return true; if (pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an encrypted wallet, but encryptwallet was called."); } @@ -3185,8 +3175,8 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) if (options.exists("changeAddress")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both changeAddress and address_type options"); } - coinControl.change_type = ParseOutputType(options["change_type"].get_str(), coinControl.change_type); - if (coinControl.change_type == OUTPUT_TYPE_NONE) { + coinControl.m_change_type = ParseOutputType(options["change_type"].get_str(), pwallet->m_default_change_type); + if (coinControl.m_change_type == OutputType::NONE) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown change type '%s'", options["change_type"].get_str())); } } diff --git a/src/wallet/test/accounting_tests.cpp b/src/wallet/test/accounting_tests.cpp index 7b20bd7b02..aae328d81f 100644 --- a/src/wallet/test/accounting_tests.cpp +++ b/src/wallet/test/accounting_tests.cpp @@ -29,7 +29,7 @@ GetResults(CWallet& wallet, std::map<CAmount, CAccountingEntry>& results) BOOST_AUTO_TEST_CASE(acc_orderupgrade) { std::vector<CWalletTx*> vpwtx; - CWalletTx wtx; + CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef()); CAccountingEntry ae; std::map<CAmount, CAccountingEntry> results; @@ -44,7 +44,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.mapValue["comment"] = "z"; m_wallet.AddToWallet(wtx); - vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); + vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetHash())); vpwtx[0]->nTimeReceived = (unsigned int)1333333335; vpwtx[0]->nOrderPos = -1; @@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.SetTx(MakeTransactionRef(std::move(tx))); } m_wallet.AddToWallet(wtx); - vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); + vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetHash())); vpwtx[1]->nTimeReceived = (unsigned int)1333333336; wtx.mapValue["comment"] = "x"; @@ -96,7 +96,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.SetTx(MakeTransactionRef(std::move(tx))); } m_wallet.AddToWallet(wtx); - vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); + vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetHash())); vpwtx[2]->nTimeReceived = (unsigned int)1333333329; vpwtx[2]->nOrderPos = -1; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp new file mode 100644 index 0000000000..f05c81cd4c --- /dev/null +++ b/src/wallet/test/coinselector_tests.cpp @@ -0,0 +1,561 @@ +// Copyright (c) 2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "wallet/wallet.h" +#include "wallet/coinselection.h" +#include "amount.h" +#include "primitives/transaction.h" +#include "random.h" +#include "test/test_bitcoin.h" +#include "wallet/test/wallet_test_fixture.h" + +#include <boost/test/unit_test.hpp> +#include <random> + +BOOST_FIXTURE_TEST_SUITE(coin_selection_tests, WalletTestingSetup) + +// how many times to run all the tests to have a chance to catch errors that only show up with particular random shuffles +#define RUN_TESTS 100 + +// some tests fail 1% of the time due to bad luck. +// we repeat those tests this many times and only complain if all iterations of the test fail +#define RANDOM_REPEATS 5 + +std::vector<std::unique_ptr<CWalletTx>> wtxn; + +typedef std::set<CInputCoin> CoinSet; + +static std::vector<COutput> vCoins; +static const CWallet testWallet("dummy", CWalletDBWrapper::CreateDummy()); +static CAmount balance = 0; + +CoinEligibilityFilter filter_standard(1, 6, 0); +CoinEligibilityFilter filter_confirmed(1, 1, 0); +CoinEligibilityFilter filter_standard_extra(6, 6, 0); +CoinSelectionParams coin_selection_params(false, 0, 0, CFeeRate(0), 0); + +static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set) +{ + CMutableTransaction tx; + tx.vout.resize(nInput + 1); + tx.vout[nInput].nValue = nValue; + set.emplace_back(MakeTransactionRef(tx), nInput); +} + +static void add_coin(const CAmount& nValue, int nInput, CoinSet& set) +{ + CMutableTransaction tx; + tx.vout.resize(nInput + 1); + tx.vout[nInput].nValue = nValue; + set.emplace(MakeTransactionRef(tx), nInput); +} + +static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) +{ + balance += nValue; + static int nextLockTime = 0; + CMutableTransaction tx; + tx.nLockTime = nextLockTime++; // so all transactions get different hashes + tx.vout.resize(nInput + 1); + tx.vout[nInput].nValue = nValue; + if (fIsFromMe) { + // IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(), + // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() + tx.vin.resize(1); + } + std::unique_ptr<CWalletTx> wtx(new CWalletTx(&testWallet, MakeTransactionRef(std::move(tx)))); + if (fIsFromMe) + { + wtx->fDebitCached = true; + wtx->nDebitCached = 1; + } + COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); + vCoins.push_back(output); + wtxn.emplace_back(std::move(wtx)); +} + +static void empty_wallet(void) +{ + vCoins.clear(); + wtxn.clear(); + balance = 0; +} + +static bool equal_sets(CoinSet a, CoinSet b) +{ + std::pair<CoinSet::iterator, CoinSet::iterator> ret = mismatch(a.begin(), a.end(), b.begin()); + return ret.first == a.end() && ret.second == b.end(); +} + +static CAmount make_hard_case(int utxos, std::vector<CInputCoin>& utxo_pool) +{ + utxo_pool.clear(); + CAmount target = 0; + for (int i = 0; i < utxos; ++i) { + target += (CAmount)1 << (utxos+i); + add_coin((CAmount)1 << (utxos+i), 2*i, utxo_pool); + add_coin(((CAmount)1 << (utxos+i)) + ((CAmount)1 << (utxos-1-i)), 2*i + 1, utxo_pool); + } + return target; +} + +// Branch and bound coin selection tests +BOOST_AUTO_TEST_CASE(bnb_search_test) +{ + + LOCK(testWallet.cs_wallet); + + // Setup + std::vector<CInputCoin> utxo_pool; + CoinSet selection; + CoinSet actual_selection; + CAmount value_ret = 0; + CAmount not_input_fees = 0; + + ///////////////////////// + // Known Outcome tests // + ///////////////////////// + BOOST_TEST_MESSAGE("Testing known outcomes"); + + // Empty utxo pool + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, 1 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + selection.clear(); + + // Add utxos + add_coin(1 * CENT, 1, utxo_pool); + add_coin(2 * CENT, 2, utxo_pool); + add_coin(3 * CENT, 3, utxo_pool); + add_coin(4 * CENT, 4, utxo_pool); + + // Select 1 Cent + add_coin(1 * CENT, 1, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 1 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + BOOST_CHECK(equal_sets(selection, actual_selection)); + actual_selection.clear(); + selection.clear(); + + // Select 2 Cent + add_coin(2 * CENT, 2, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 2 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + BOOST_CHECK(equal_sets(selection, actual_selection)); + actual_selection.clear(); + selection.clear(); + + // Select 5 Cent + add_coin(3 * CENT, 3, actual_selection); + add_coin(2 * CENT, 2, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 5 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + BOOST_CHECK(equal_sets(selection, actual_selection)); + actual_selection.clear(); + selection.clear(); + + // Select 11 Cent, not possible + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, 11 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + actual_selection.clear(); + selection.clear(); + + // Select 10 Cent + add_coin(5 * CENT, 5, utxo_pool); + add_coin(4 * CENT, 4, actual_selection); + add_coin(3 * CENT, 3, actual_selection); + add_coin(2 * CENT, 2, actual_selection); + add_coin(1 * CENT, 1, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 10 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + BOOST_CHECK(equal_sets(selection, actual_selection)); + actual_selection.clear(); + selection.clear(); + + // Negative effective value + // Select 10 Cent but have 1 Cent not be possible because too small + add_coin(5 * CENT, 5, actual_selection); + add_coin(3 * CENT, 3, actual_selection); + add_coin(2 * CENT, 2, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 10 * CENT, 5000, selection, value_ret, not_input_fees)); + + // Select 0.25 Cent, not possible + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, 0.25 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + actual_selection.clear(); + selection.clear(); + + // Iteration exhaustion test + CAmount target = make_hard_case(17, utxo_pool); + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, target, 0, selection, value_ret, not_input_fees)); // Should exhaust + target = make_hard_case(14, utxo_pool); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, target, 0, selection, value_ret, not_input_fees)); // Should not exhaust + + // Test same value early bailout optimization + add_coin(7 * CENT, 7, actual_selection); + add_coin(7 * CENT, 7, actual_selection); + add_coin(7 * CENT, 7, actual_selection); + add_coin(7 * CENT, 7, actual_selection); + add_coin(2 * CENT, 7, actual_selection); + add_coin(7 * CENT, 7, utxo_pool); + add_coin(7 * CENT, 7, utxo_pool); + add_coin(7 * CENT, 7, utxo_pool); + add_coin(7 * CENT, 7, utxo_pool); + add_coin(2 * CENT, 7, utxo_pool); + for (int i = 0; i < 50000; ++i) { + add_coin(5 * CENT, 7, utxo_pool); + } + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 30 * CENT, 5000, selection, value_ret, not_input_fees)); + + //////////////////// + // Behavior tests // + //////////////////// + // Select 1 Cent with pool of only greater than 5 Cent + utxo_pool.clear(); + for (int i = 5; i <= 20; ++i) { + add_coin(i * CENT, i, utxo_pool); + } + // Run 100 times, to make sure it is never finding a solution + for (int i = 0; i < 100; ++i) { + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, 1 * CENT, 2 * CENT, selection, value_ret, not_input_fees)); + } + + // Make sure that effective value is working in SelectCoinsMinConf when BnB is used + CoinSelectionParams coin_selection_params_bnb(true, 0, 0, CFeeRate(3000), 0); + CoinSet setCoinsRet; + CAmount nValueRet; + bool bnb_used; + empty_wallet(); + add_coin(1); + vCoins.at(0).nInputBytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); +} + +BOOST_AUTO_TEST_CASE(knapsack_solver_test) +{ + CoinSet setCoinsRet, setCoinsRet2; + CAmount nValueRet; + bool bnb_used; + + LOCK(testWallet.cs_wallet); + + // test multiple times to allow for differences in the shuffle order + for (int i = 0; i < RUN_TESTS; i++) + { + empty_wallet(); + + // with an empty wallet we can't even pay one cent + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + + add_coin(1*CENT, 4); // add a new 1 cent coin + + // with a new 1 cent coin, we still can't find a mature 1 cent + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + + // but we can find a new 1 cent + BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); + + add_coin(2*CENT); // add a mature 2 cent coin + + // we can't make 3 cents of mature coins + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + + // we can make 3 cents of new coins + BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); + + add_coin(5*CENT); // add a mature 5 cent coin, + add_coin(10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses + add_coin(20*CENT); // and a mature 20 cent coin + + // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38 + + // we can't make 38 cents only if we disallow new coins: + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + // we can't even make 37 cents if we don't allow new coins even if they're from us + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard_extra, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + // but we can make 37 cents if we accept new coins from ourself + BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); + // and we can make 38 cents if we accept all new coins + BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 38 * CENT); + + // try making 34 cents from 1,2,5,10,20 - we can't do it exactly + BOOST_CHECK( testWallet.SelectCoinsMinConf(34 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 35 * CENT); // but 35 cents is closest + BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) + + // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 + BOOST_CHECK( testWallet.SelectCoinsMinConf( 7 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. + BOOST_CHECK( testWallet.SelectCoinsMinConf( 8 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(nValueRet == 8 * CENT); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); + + // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) + BOOST_CHECK( testWallet.SelectCoinsMinConf( 9 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin + empty_wallet(); + + add_coin( 6*CENT); + add_coin( 7*CENT); + add_coin( 8*CENT); + add_coin(20*CENT); + add_coin(30*CENT); // now we have 6+7+8+20+30 = 71 cents total + + // check that we have 71 and not 72 + BOOST_CHECK( testWallet.SelectCoinsMinConf(71 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + + // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + add_coin( 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total + + // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins + BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); + + add_coin( 18*CENT); // now we have 5+6+7+8+18+20+30 + + // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // because in the event of a tie, the biggest coin wins + + // now try making 11 cents. we should get 5+6 + BOOST_CHECK( testWallet.SelectCoinsMinConf(11 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + // check that the smallest bigger coin is used + add_coin( 1*COIN); + add_coin( 2*COIN); + add_coin( 3*COIN); + add_coin( 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents + BOOST_CHECK( testWallet.SelectCoinsMinConf(95 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + BOOST_CHECK( testWallet.SelectCoinsMinConf(195 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + // empty the wallet and start again, now with fractions of a cent, to test small change avoidance + + empty_wallet(); + add_coin(MIN_CHANGE * 1 / 10); + add_coin(MIN_CHANGE * 2 / 10); + add_coin(MIN_CHANGE * 3 / 10); + add_coin(MIN_CHANGE * 4 / 10); + add_coin(MIN_CHANGE * 5 / 10); + + // try making 1 * MIN_CHANGE from the 1.5 * MIN_CHANGE + // we'll get change smaller than MIN_CHANGE whatever happens, so can expect MIN_CHANGE exactly + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); + + // but if we add a bigger coin, small change is avoided + add_coin(1111*MIN_CHANGE); + + // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount + + // if we add more small coins: + add_coin(MIN_CHANGE * 6 / 10); + add_coin(MIN_CHANGE * 7 / 10); + + // and try again to make 1.0 * MIN_CHANGE + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount + + // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) + // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change + empty_wallet(); + for (int j = 0; j < 20; j++) + add_coin(50000 * COIN); + + BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount + BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins + + // if there's not enough in the smaller coins to make at least 1 * MIN_CHANGE change (0.5+0.6+0.7 < 1.0+1.0), + // we need to try finding an exact subset anyway + + // sometimes it will fail, and so we use the next biggest coin: + empty_wallet(); + add_coin(MIN_CHANGE * 5 / 10); + add_coin(MIN_CHANGE * 6 / 10); + add_coin(MIN_CHANGE * 7 / 10); + add_coin(1111 * MIN_CHANGE); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) + empty_wallet(); + add_coin(MIN_CHANGE * 4 / 10); + add_coin(MIN_CHANGE * 6 / 10); + add_coin(MIN_CHANGE * 8 / 10); + add_coin(1111 * MIN_CHANGE); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // we should get the exact amount + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two coins 0.4+0.6 + + // test avoiding small change + empty_wallet(); + add_coin(MIN_CHANGE * 5 / 100); + add_coin(MIN_CHANGE * 1); + add_coin(MIN_CHANGE * 100); + + // trying to make 100.01 from these three coins + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE * 10105 / 100); // we should get all coins + BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); + + // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + // test with many inputs + for (CAmount amt=1500; amt < COIN; amt*=10) { + empty_wallet(); + // Create 676 inputs (= (old MAX_STANDARD_TX_SIZE == 100000) / 148 bytes per input) + for (uint16_t j = 0; j < 676; j++) + add_coin(amt); + BOOST_CHECK(testWallet.SelectCoinsMinConf(2000, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + if (amt - 2000 < MIN_CHANGE) { + // needs more than one input: + uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); + CAmount returnValue = amt * returnSize; + BOOST_CHECK_EQUAL(nValueRet, returnValue); + BOOST_CHECK_EQUAL(setCoinsRet.size(), returnSize); + } else { + // one input is sufficient: + BOOST_CHECK_EQUAL(nValueRet, amt); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + } + } + + // test randomness + { + empty_wallet(); + for (int i2 = 0; i2 < 100; i2++) + add_coin(COIN); + + // picking 50 from 100 coins doesn't depend on the shuffle, + // but does depend on randomness in the stochastic approximation code + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2)); + + int fails = 0; + for (int j = 0; j < RANDOM_REPEATS; j++) + { + // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time + // run the test RANDOM_REPEATS times and only complain if all of them fail + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + if (equal_sets(setCoinsRet, setCoinsRet2)) + fails++; + } + BOOST_CHECK_NE(fails, RANDOM_REPEATS); + + // add 75 cents in small change. not enough to make 90 cents, + // then try making 90 cents. there are multiple competing "smallest bigger" coins, + // one of which should be picked at random + add_coin(5 * CENT); + add_coin(10 * CENT); + add_coin(15 * CENT); + add_coin(20 * CENT); + add_coin(25 * CENT); + + fails = 0; + for (int j = 0; j < RANDOM_REPEATS; j++) + { + // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time + // run the test RANDOM_REPEATS times and only complain if all of them fail + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + if (equal_sets(setCoinsRet, setCoinsRet2)) + fails++; + } + BOOST_CHECK_NE(fails, RANDOM_REPEATS); + } + } + empty_wallet(); +} + +BOOST_AUTO_TEST_CASE(ApproximateBestSubset) +{ + CoinSet setCoinsRet; + CAmount nValueRet; + bool bnb_used; + + LOCK(testWallet.cs_wallet); + + empty_wallet(); + + // Test vValue sort order + for (int i = 0; i < 1000; i++) + add_coin(1000 * COIN); + add_coin(3 * COIN); + + BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + empty_wallet(); +} + +// Tests that with the ideal conditions, the coin selector will always be able to find a solution that can pay the target value +BOOST_AUTO_TEST_CASE(SelectCoins_test) +{ + // Random generator stuff + std::default_random_engine generator; + std::exponential_distribution<double> distribution (100); + FastRandomContext rand; + + // Output stuff + CAmount out_value = 0; + CoinSet out_set; + CAmount target = 0; + bool bnb_used; + + // Run this test 100 times + for (int i = 0; i < 100; ++i) + { + // Reset + out_value = 0; + target = 0; + out_set.clear(); + empty_wallet(); + + // Make a wallet with 1000 exponentially distributed random inputs + for (int j = 0; j < 1000; ++j) + { + add_coin((CAmount)(distribution(generator)*10000000)); + } + + // Generate a random fee rate in the range of 100 - 400 + CFeeRate rate(rand.randrange(300) + 100); + + // Generate a random target value between 1000 and wallet balance + target = rand.randrange(balance - 1000) + 1000; + + // Perform selection + CoinSelectionParams coin_selection_params_knapsack(false, 34, 148, CFeeRate(0), 0); + CoinSelectionParams coin_selection_params_bnb(true, 34, 148, CFeeRate(0), 0); + BOOST_CHECK(testWallet.SelectCoinsMinConf(target, filter_standard, vCoins, out_set, out_value, coin_selection_params_bnb, bnb_used) || + testWallet.SelectCoinsMinConf(target, filter_standard, vCoins, out_set, out_value, coin_selection_params_knapsack, bnb_used)); + BOOST_CHECK_GE(out_value, target); + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 77ccd0b8d8..5c550742c8 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -12,8 +12,6 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName): TestingSetup(chainName), m_wallet("mock", CWalletDBWrapper::CreateMock()) { bool fFirstRun; - g_address_type = OUTPUT_TYPE_DEFAULT; - g_change_type = OUTPUT_TYPE_DEFAULT; m_wallet.LoadWallet(fFirstRun); RegisterValidationInterface(&m_wallet); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 41348b50a4..808f8b8838 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -23,345 +23,8 @@ extern UniValue importmulti(const JSONRPCRequest& request); extern UniValue dumpwallet(const JSONRPCRequest& request); extern UniValue importwallet(const JSONRPCRequest& request); -// how many times to run all the tests to have a chance to catch errors that only show up with particular random shuffles -#define RUN_TESTS 100 - -// some tests fail 1% of the time due to bad luck. -// we repeat those tests this many times and only complain if all iterations of the test fail -#define RANDOM_REPEATS 5 - -std::vector<std::unique_ptr<CWalletTx>> wtxn; - -typedef std::set<CInputCoin> CoinSet; - BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) -static const CWallet testWallet("dummy", CWalletDBWrapper::CreateDummy()); -static std::vector<COutput> vCoins; - -static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) -{ - static int nextLockTime = 0; - CMutableTransaction tx; - tx.nLockTime = nextLockTime++; // so all transactions get different hashes - tx.vout.resize(nInput+1); - tx.vout[nInput].nValue = nValue; - if (fIsFromMe) { - // IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(), - // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() - tx.vin.resize(1); - } - std::unique_ptr<CWalletTx> wtx(new CWalletTx(&testWallet, MakeTransactionRef(std::move(tx)))); - if (fIsFromMe) - { - wtx->fDebitCached = true; - wtx->nDebitCached = 1; - } - COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); - vCoins.push_back(output); - wtxn.emplace_back(std::move(wtx)); -} - -static void empty_wallet(void) -{ - vCoins.clear(); - wtxn.clear(); -} - -static bool equal_sets(CoinSet a, CoinSet b) -{ - std::pair<CoinSet::iterator, CoinSet::iterator> ret = mismatch(a.begin(), a.end(), b.begin()); - return ret.first == a.end() && ret.second == b.end(); -} - -BOOST_AUTO_TEST_CASE(coin_selection_tests) -{ - CoinSet setCoinsRet, setCoinsRet2; - CAmount nValueRet; - - LOCK(testWallet.cs_wallet); - - // test multiple times to allow for differences in the shuffle order - for (int i = 0; i < RUN_TESTS; i++) - { - empty_wallet(); - - // with an empty wallet we can't even pay one cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - - add_coin(1*CENT, 4); // add a new 1 cent coin - - // with a new 1 cent coin, we still can't find a mature 1 cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - - // but we can find a new 1 cent - BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); - - add_coin(2*CENT); // add a mature 2 cent coin - - // we can't make 3 cents of mature coins - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - - // we can make 3 cents of new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); - - add_coin(5*CENT); // add a mature 5 cent coin, - add_coin(10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses - add_coin(20*CENT); // and a mature 20 cent coin - - // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38 - - // we can't make 38 cents only if we disallow new coins: - BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, 6, 6, 0, vCoins, setCoinsRet, nValueRet)); - // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); - // and we can make 38 cents if we accept all new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 38 * CENT); - - // try making 34 cents from 1,2,5,10,20 - we can't do it exactly - BOOST_CHECK( testWallet.SelectCoinsMinConf(34 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 35 * CENT); // but 35 cents is closest - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) - - // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 - BOOST_CHECK( testWallet.SelectCoinsMinConf( 7 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. - BOOST_CHECK( testWallet.SelectCoinsMinConf( 8 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK(nValueRet == 8 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); - - // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) - BOOST_CHECK( testWallet.SelectCoinsMinConf( 9 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin - empty_wallet(); - - add_coin( 6*CENT); - add_coin( 7*CENT); - add_coin( 8*CENT); - add_coin(20*CENT); - add_coin(30*CENT); // now we have 6+7+8+20+30 = 71 cents total - - // check that we have 71 and not 72 - BOOST_CHECK( testWallet.SelectCoinsMinConf(71 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - - // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - add_coin( 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total - - // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); - - add_coin( 18*CENT); // now we have 5+6+7+8+18+20+30 - - // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // because in the event of a tie, the biggest coin wins - - // now try making 11 cents. we should get 5+6 - BOOST_CHECK( testWallet.SelectCoinsMinConf(11 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - // check that the smallest bigger coin is used - add_coin( 1*COIN); - add_coin( 2*COIN); - add_coin( 3*COIN); - add_coin( 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents - BOOST_CHECK( testWallet.SelectCoinsMinConf(95 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - BOOST_CHECK( testWallet.SelectCoinsMinConf(195 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - // empty the wallet and start again, now with fractions of a cent, to test small change avoidance - - empty_wallet(); - add_coin(MIN_CHANGE * 1 / 10); - add_coin(MIN_CHANGE * 2 / 10); - add_coin(MIN_CHANGE * 3 / 10); - add_coin(MIN_CHANGE * 4 / 10); - add_coin(MIN_CHANGE * 5 / 10); - - // try making 1 * MIN_CHANGE from the 1.5 * MIN_CHANGE - // we'll get change smaller than MIN_CHANGE whatever happens, so can expect MIN_CHANGE exactly - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); - - // but if we add a bigger coin, small change is avoided - add_coin(1111*MIN_CHANGE); - - // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount - - // if we add more small coins: - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 7 / 10); - - // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount - - // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) - // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change - empty_wallet(); - for (int j = 0; j < 20; j++) - add_coin(50000 * COIN); - - BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount - BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins - - // if there's not enough in the smaller coins to make at least 1 * MIN_CHANGE change (0.5+0.6+0.7 < 1.0+1.0), - // we need to try finding an exact subset anyway - - // sometimes it will fail, and so we use the next biggest coin: - empty_wallet(); - add_coin(MIN_CHANGE * 5 / 10); - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 7 / 10); - add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) - empty_wallet(); - add_coin(MIN_CHANGE * 4 / 10); - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 8 / 10); - add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // we should get the exact amount - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two coins 0.4+0.6 - - // test avoiding small change - empty_wallet(); - add_coin(MIN_CHANGE * 5 / 100); - add_coin(MIN_CHANGE * 1); - add_coin(MIN_CHANGE * 100); - - // trying to make 100.01 from these three coins - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE * 10105 / 100); // we should get all coins - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); - - // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - // test with many inputs - for (CAmount amt=1500; amt < COIN; amt*=10) { - empty_wallet(); - // Create 676 inputs (= (old MAX_STANDARD_TX_SIZE == 100000) / 148 bytes per input) - for (uint16_t j = 0; j < 676; j++) - add_coin(amt); - BOOST_CHECK(testWallet.SelectCoinsMinConf(2000, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - if (amt - 2000 < MIN_CHANGE) { - // needs more than one input: - uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); - CAmount returnValue = amt * returnSize; - BOOST_CHECK_EQUAL(nValueRet, returnValue); - BOOST_CHECK_EQUAL(setCoinsRet.size(), returnSize); - } else { - // one input is sufficient: - BOOST_CHECK_EQUAL(nValueRet, amt); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - } - } - - // test randomness - { - empty_wallet(); - for (int i2 = 0; i2 < 100; i2++) - add_coin(COIN); - - // picking 50 from 100 coins doesn't depend on the shuffle, - // but does depend on randomness in the stochastic approximation code - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); - BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2)); - - int fails = 0; - for (int j = 0; j < RANDOM_REPEATS; j++) - { - // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time - // run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); - if (equal_sets(setCoinsRet, setCoinsRet2)) - fails++; - } - BOOST_CHECK_NE(fails, RANDOM_REPEATS); - - // add 75 cents in small change. not enough to make 90 cents, - // then try making 90 cents. there are multiple competing "smallest bigger" coins, - // one of which should be picked at random - add_coin(5 * CENT); - add_coin(10 * CENT); - add_coin(15 * CENT); - add_coin(20 * CENT); - add_coin(25 * CENT); - - fails = 0; - for (int j = 0; j < RANDOM_REPEATS; j++) - { - // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time - // run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); - if (equal_sets(setCoinsRet, setCoinsRet2)) - fails++; - } - BOOST_CHECK_NE(fails, RANDOM_REPEATS); - } - } - empty_wallet(); -} - -BOOST_AUTO_TEST_CASE(ApproximateBestSubset) -{ - CoinSet setCoinsRet; - CAmount nValueRet; - - LOCK(testWallet.cs_wallet); - - empty_wallet(); - - // Test vValue sort order - for (int i = 0; i < 1000; i++) - add_coin(1000 * COIN); - add_coin(3 * COIN); - - BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - empty_wallet(); -} - static void AddKey(CWallet& wallet, const CKey& key) { LOCK(wallet.cs_wallet); @@ -451,9 +114,6 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // than or equal to key birthday. BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) { - g_address_type = OUTPUT_TYPE_DEFAULT; - g_change_type = OUTPUT_TYPE_DEFAULT; - // Create two blocks with same timestamp to verify that importwallet rescan // will pick up both blocks, not just the first. const int64_t BLOCK_TIME = chainActive.Tip()->GetBlockTimeMax() + 5; @@ -553,7 +213,10 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 if (block) { wtx.SetMerkleBranch(block, 0); } - wallet.AddToWallet(wtx); + { + LOCK(cs_main); + wallet.AddToWallet(wtx); + } LOCK(wallet.cs_wallet); return wallet.mapWallet.at(wtx.GetHash()).nTimeSmart; } @@ -606,8 +269,6 @@ public: ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - g_address_type = OUTPUT_TYPE_DEFAULT; - g_change_type = OUTPUT_TYPE_DEFAULT; wallet = MakeUnique<CWallet>("mock", CWalletDBWrapper::CreateMock()); bool firstRun; wallet->LoadWallet(firstRun); @@ -624,23 +285,23 @@ public: CWalletTx& AddTx(CRecipient recipient) { - CWalletTx wtx; + CTransactionRef tx; CReserveKey reservekey(wallet.get()); CAmount fee; int changePos = -1; std::string error; CCoinControl dummy; - BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, changePos, error, dummy)); + BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, reservekey, fee, changePos, error, dummy)); CValidationState state; - BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state)); + BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, {}, reservekey, nullptr, state)); CMutableTransaction blocktx; { LOCK(wallet->cs_wallet); - blocktx = CMutableTransaction(*wallet->mapWallet.at(wtx.GetHash()).tx); + blocktx = CMutableTransaction(*wallet->mapWallet.at(tx->GetHash()).tx); } CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); LOCK(wallet->cs_wallet); - auto it = wallet->mapWallet.find(wtx.GetHash()); + auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); it->second.SetMerkleBranch(chainActive.Tip(), 1); return it->second; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0c468878e1..3f1a2c1990 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -8,6 +8,7 @@ #include <checkpoints.h> #include <chain.h> #include <wallet/coincontrol.h> +#include <wallet/coinselection.h> #include <consensus/consensus.h> #include <consensus/validation.h> #include <fs.h> @@ -41,8 +42,6 @@ CFeeRate payTxFee(DEFAULT_TRANSACTION_FEE); unsigned int nTxConfirmTarget = DEFAULT_TX_CONFIRM_TARGET; bool bSpendZeroConfChange = DEFAULT_SPEND_ZEROCONF_CHANGE; bool fWalletRbf = DEFAULT_WALLET_RBF; -OutputType g_address_type = OUTPUT_TYPE_NONE; -OutputType g_change_type = OUTPUT_TYPE_NONE; bool g_wallet_allow_fallback_fee = true; //<! will be defined via chainparams const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; @@ -68,15 +67,6 @@ const uint256 CMerkleTx::ABANDON_HASH(uint256S("00000000000000000000000000000000 * @{ */ -struct CompareValueOnly -{ - bool operator()(const CInputCoin& t1, - const CInputCoin& t2) const - { - return t1.txout.nValue < t2.txout.nValue; - } -}; - std::string COutput::ToString() const { return strprintf("COutput(%s, %d, %d) [%s]", tx->GetHash().ToString(), i, nDepth, FormatMoney(tx->tx->vout[i].nValue)); @@ -531,7 +521,7 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran int nMinOrderPos = std::numeric_limits<int>::max(); const CWalletTx* copyFrom = nullptr; for (TxSpends::iterator it = range.first; it != range.second; ++it) { - const CWalletTx* wtx = &mapWallet[it->second]; + const CWalletTx* wtx = &mapWallet.at(it->second); if (wtx->nOrderPos < nMinOrderPos) { nMinOrderPos = wtx->nOrderPos;; copyFrom = wtx; @@ -544,7 +534,7 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran for (TxSpends::iterator it = range.first; it != range.second; ++it) { const uint256& hash = it->second; - CWalletTx* copyTo = &mapWallet[hash]; + CWalletTx* copyTo = &mapWallet.at(hash); if (copyFrom == copyTo) continue; assert(copyFrom && "Oldest wallet transaction in range assumed to have been found."); if (!copyFrom->IsEquivalentTo(*copyTo)) continue; @@ -831,7 +821,7 @@ bool CWallet::GetAccountDestination(CTxDestination &dest, std::string strAccount bForceNew = true; else { // Check if the current key has been used (TODO: check other addresses with the same key) - CScript scriptPubKey = GetScriptForDestination(GetDestinationForKey(account.vchPubKey, g_address_type)); + CScript scriptPubKey = GetScriptForDestination(GetDestinationForKey(account.vchPubKey, m_default_address_type)); for (std::map<uint256, CWalletTx>::iterator it = mapWallet.begin(); it != mapWallet.end() && account.vchPubKey.IsValid(); ++it) @@ -848,12 +838,12 @@ bool CWallet::GetAccountDestination(CTxDestination &dest, std::string strAccount if (!GetKeyFromPool(account.vchPubKey, false)) return false; - LearnRelatedScripts(account.vchPubKey, g_address_type); - dest = GetDestinationForKey(account.vchPubKey, g_address_type); + LearnRelatedScripts(account.vchPubKey, m_default_address_type); + dest = GetDestinationForKey(account.vchPubKey, m_default_address_type); SetAddressBook(dest, strAccount, "receive"); walletdb.WriteAccount(strAccount, account); } else { - dest = GetDestinationForKey(account.vchPubKey, g_address_type); + dest = GetDestinationForKey(account.vchPubKey, m_default_address_type); } return true; @@ -1147,11 +1137,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) LOCK2(cs_main, cs_wallet); int conflictconfirms = 0; - if (mapBlockIndex.count(hashBlock)) { - CBlockIndex* pindex = mapBlockIndex[hashBlock]; - if (chainActive.Contains(pindex)) { - conflictconfirms = -(chainActive.Height() - pindex->nHeight + 1); - } + CBlockIndex* pindex = LookupBlockIndex(hashBlock); + if (pindex && chainActive.Contains(pindex)) { + conflictconfirms = -(chainActive.Height() - pindex->nHeight + 1); } // If number of conflict confirms cannot be determined, this means // that the block is still unknown or not yet part of the main chain, @@ -1543,6 +1531,79 @@ int CWalletTx::GetRequestCount() const return nRequests; } +// Helper for producing a max-sized low-S signature (eg 72 bytes) +bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout) const +{ + // Fill in dummy signatures for fee calculation. + const CScript& scriptPubKey = txout.scriptPubKey; + SignatureData sigdata; + + if (!ProduceSignature(DummySignatureCreator(this), scriptPubKey, sigdata)) + { + return false; + } else { + UpdateInput(tx_in, sigdata); + } + return true; +} + +// Helper for producing a bunch of max-sized low-S signatures (eg 72 bytes) +bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts) const +{ + // Fill in dummy signatures for fee calculation. + int nIn = 0; + for (const auto& txout : txouts) + { + if (!DummySignInput(txNew.vin[nIn], txout)) { + return false; + } + + nIn++; + } + return true; +} + +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet) +{ + std::vector<CTxOut> txouts; + // Look up the inputs. We should have already checked that this transaction + // IsAllFromMe(ISMINE_SPENDABLE), so every input should already be in our + // wallet, with a valid index into the vout array, and the ability to sign. + for (auto& input : tx.vin) { + const auto mi = wallet->mapWallet.find(input.prevout.hash); + if (mi == wallet->mapWallet.end()) { + return -1; + } + assert(input.prevout.n < mi->second.tx->vout.size()); + txouts.emplace_back(mi->second.tx->vout[input.prevout.n]); + } + return CalculateMaximumSignedTxSize(tx, wallet, txouts); +} + +// txouts needs to be in the order of tx.vin +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts) +{ + CMutableTransaction txNew(tx); + if (!wallet->DummySignTx(txNew, txouts)) { + // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) + // implies that we can sign for every input. + return -1; + } + return GetVirtualTransactionSize(txNew); +} + +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet) +{ + CMutableTransaction txn; + txn.vin.push_back(CTxIn(COutPoint())); + if (!wallet->DummySignInput(txn.vin[0], txout)) { + // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) + // implies that we can sign for every input. + return -1; + } + return GetVirtualTransactionInputSize(txn.vin[0]); +} + void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived, std::list<COutputEntry>& listSent, CAmount& nFee, std::string& strSentAccount, const isminefilter& filter) const { @@ -2364,171 +2425,88 @@ const CTxOut& CWallet::FindNonChangeParentOutput(const CTransaction& tx, int out return ptx->vout[n]; } -static void ApproximateBestSubset(const std::vector<CInputCoin>& vValue, const CAmount& nTotalLower, const CAmount& nTargetValue, - std::vector<char>& vfBest, CAmount& nBest, int iterations = 1000) +bool CWallet::OutputEligibleForSpending(const COutput& output, const CoinEligibilityFilter& eligibilty_filter) const { - std::vector<char> vfIncluded; + if (!output.fSpendable) + return false; - vfBest.assign(vValue.size(), true); - nBest = nTotalLower; + if (output.nDepth < (output.tx->IsFromMe(ISMINE_ALL) ? eligibilty_filter.conf_mine : eligibilty_filter.conf_theirs)) + return false; - FastRandomContext insecure_rand; + if (!mempool.TransactionWithinChainLimit(output.tx->GetHash(), eligibilty_filter.max_ancestors)) + return false; - for (int nRep = 0; nRep < iterations && nBest != nTargetValue; nRep++) - { - vfIncluded.assign(vValue.size(), false); - CAmount nTotal = 0; - bool fReachedTarget = false; - for (int nPass = 0; nPass < 2 && !fReachedTarget; nPass++) - { - for (unsigned int i = 0; i < vValue.size(); i++) - { - //The solver here uses a randomized algorithm, - //the randomness serves no real security purpose but is just - //needed to prevent degenerate behavior and it is important - //that the rng is fast. We do not use a constant random sequence, - //because there may be some privacy improvement by making - //the selection random. - if (nPass == 0 ? insecure_rand.randbool() : !vfIncluded[i]) - { - nTotal += vValue[i].txout.nValue; - vfIncluded[i] = true; - if (nTotal >= nTargetValue) - { - fReachedTarget = true; - if (nTotal < nBest) - { - nBest = nTotal; - vfBest = vfIncluded; - } - nTotal -= vValue[i].txout.nValue; - vfIncluded[i] = false; - } - } - } - } - } + return true; } -bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMine, const int nConfTheirs, const uint64_t nMaxAncestors, std::vector<COutput> vCoins, - std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet) const +bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibilty_filter, std::vector<COutput> vCoins, + std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const { setCoinsRet.clear(); nValueRet = 0; - // List of values less than target - boost::optional<CInputCoin> coinLowestLarger; - std::vector<CInputCoin> vValue; - CAmount nTotalLower = 0; + std::vector<CInputCoin> utxo_pool; + if (coin_selection_params.use_bnb) { - random_shuffle(vCoins.begin(), vCoins.end(), GetRandInt); + // Get long term estimate + FeeCalculation feeCalc; + CCoinControl temp; + temp.m_confirm_target = 1008; + CFeeRate long_term_feerate = GetMinimumFeeRate(temp, ::mempool, ::feeEstimator, &feeCalc); - for (const COutput &output : vCoins) - { - if (!output.fSpendable) - continue; - - const CWalletTx *pcoin = output.tx; - - if (output.nDepth < (pcoin->IsFromMe(ISMINE_ALL) ? nConfMine : nConfTheirs)) - continue; - - if (!mempool.TransactionWithinChainLimit(pcoin->GetHash(), nMaxAncestors)) - continue; + // Calculate cost of change + CAmount cost_of_change = GetDiscardRate(::feeEstimator).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size); - int i = output.i; - - CInputCoin coin = CInputCoin(pcoin, i); - - if (coin.txout.nValue == nTargetValue) - { - setCoinsRet.insert(coin); - nValueRet += coin.txout.nValue; - return true; - } - else if (coin.txout.nValue < nTargetValue + MIN_CHANGE) + // Filter by the min conf specs and add to utxo_pool and calculate effective value + for (const COutput &output : vCoins) { - vValue.push_back(coin); - nTotalLower += coin.txout.nValue; + if (!OutputEligibleForSpending(output, eligibilty_filter)) + continue; + + CInputCoin coin(output.tx->tx, output.i); + coin.effective_value = coin.txout.nValue - (output.nInputBytes < 0 ? 0 : coin_selection_params.effective_fee.GetFee(output.nInputBytes)); + // Only include outputs that are positive effective value (i.e. not dust) + if (coin.effective_value > 0) { + coin.fee = output.nInputBytes < 0 ? 0 : coin_selection_params.effective_fee.GetFee(output.nInputBytes); + coin.long_term_fee = output.nInputBytes < 0 ? 0 : long_term_feerate.GetFee(output.nInputBytes); + utxo_pool.push_back(coin); + } } - else if (!coinLowestLarger || coin.txout.nValue < coinLowestLarger->txout.nValue) + // Calculate the fees for things that aren't inputs + CAmount not_input_fees = coin_selection_params.effective_fee.GetFee(coin_selection_params.tx_noinputs_size); + bnb_used = true; + return SelectCoinsBnB(utxo_pool, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); + } else { + // Filter by the min conf specs and add to utxo_pool + for (const COutput &output : vCoins) { - coinLowestLarger = coin; - } - } + if (!OutputEligibleForSpending(output, eligibilty_filter)) + continue; - if (nTotalLower == nTargetValue) - { - for (const auto& input : vValue) - { - setCoinsRet.insert(input); - nValueRet += input.txout.nValue; + CInputCoin coin = CInputCoin(output.tx->tx, output.i); + utxo_pool.push_back(coin); } - return true; + bnb_used = false; + return KnapsackSolver(nTargetValue, utxo_pool, setCoinsRet, nValueRet); } - - if (nTotalLower < nTargetValue) - { - if (!coinLowestLarger) - return false; - setCoinsRet.insert(coinLowestLarger.get()); - nValueRet += coinLowestLarger->txout.nValue; - return true; - } - - // Solve subset sum by stochastic approximation - std::sort(vValue.begin(), vValue.end(), CompareValueOnly()); - std::reverse(vValue.begin(), vValue.end()); - std::vector<char> vfBest; - CAmount nBest; - - ApproximateBestSubset(vValue, nTotalLower, nTargetValue, vfBest, nBest); - if (nBest != nTargetValue && nTotalLower >= nTargetValue + MIN_CHANGE) - ApproximateBestSubset(vValue, nTotalLower, nTargetValue + MIN_CHANGE, vfBest, nBest); - - // If we have a bigger coin and (either the stochastic approximation didn't find a good solution, - // or the next bigger coin is closer), return the bigger coin - if (coinLowestLarger && - ((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || coinLowestLarger->txout.nValue <= nBest)) - { - setCoinsRet.insert(coinLowestLarger.get()); - nValueRet += coinLowestLarger->txout.nValue; - } - else { - for (unsigned int i = 0; i < vValue.size(); i++) - if (vfBest[i]) - { - setCoinsRet.insert(vValue[i]); - nValueRet += vValue[i].txout.nValue; - } - - if (LogAcceptCategory(BCLog::SELECTCOINS)) { - LogPrint(BCLog::SELECTCOINS, "SelectCoins() best subset: "); - for (unsigned int i = 0; i < vValue.size(); i++) { - if (vfBest[i]) { - LogPrint(BCLog::SELECTCOINS, "%s ", FormatMoney(vValue[i].txout.nValue)); - } - } - LogPrint(BCLog::SELECTCOINS, "total %s\n", FormatMoney(nBest)); - } - } - - return true; } -bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CCoinControl* coinControl) const +bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const { std::vector<COutput> vCoins(vAvailableCoins); // coin control -> return all selected outputs (we want all selected to go into the transaction for sure) - if (coinControl && coinControl->HasSelected() && !coinControl->fAllowOtherInputs) + if (coin_control.HasSelected() && !coin_control.fAllowOtherInputs) { + // We didn't use BnB here, so set it to false. + bnb_used = false; + for (const COutput& out : vCoins) { if (!out.fSpendable) continue; nValueRet += out.tx->tx->vout[out.i].nValue; - setCoinsRet.insert(CInputCoin(out.tx, out.i)); + setCoinsRet.insert(CInputCoin(out.tx->tx, out.i)); } return (nValueRet >= nTargetValue); } @@ -2538,10 +2516,12 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm CAmount nValueFromPresetInputs = 0; std::vector<COutPoint> vPresetInputs; - if (coinControl) - coinControl->ListSelected(vPresetInputs); + coin_control.ListSelected(vPresetInputs); for (const COutPoint& outpoint : vPresetInputs) { + // For now, don't use BnB if preset inputs are selected. TODO: Enable this later + bnb_used = false; + std::map<uint256, CWalletTx>::const_iterator it = mapWallet.find(outpoint.hash); if (it != mapWallet.end()) { @@ -2549,16 +2529,17 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm // Clearly invalid input, fail if (pcoin->tx->vout.size() <= outpoint.n) return false; + // Just to calculate the marginal byte size nValueFromPresetInputs += pcoin->tx->vout[outpoint.n].nValue; - setPresetCoins.insert(CInputCoin(pcoin, outpoint.n)); + setPresetCoins.insert(CInputCoin(pcoin->tx, outpoint.n)); } else return false; // TODO: Allow non-wallet inputs } // remove preset inputs from vCoins - for (std::vector<COutput>::iterator it = vCoins.begin(); it != vCoins.end() && coinControl && coinControl->HasSelected();) + for (std::vector<COutput>::iterator it = vCoins.begin(); it != vCoins.end() && coin_control.HasSelected();) { - if (setPresetCoins.count(CInputCoin(it->tx, it->i))) + if (setPresetCoins.count(CInputCoin(it->tx->tx, it->i))) it = vCoins.erase(it); else ++it; @@ -2568,13 +2549,13 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm bool fRejectLongChains = gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS); bool res = nTargetValue <= nValueFromPresetInputs || - SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 1, 6, 0, vCoins, setCoinsRet, nValueRet) || - SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 1, 1, 0, vCoins, setCoinsRet, nValueRet) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, 2, vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, std::min((size_t)4, nMaxChainLength/3), vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, nMaxChainLength/2, vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, nMaxChainLength, vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && !fRejectLongChains && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, std::numeric_limits<uint64_t>::max(), vCoins, setCoinsRet, nValueRet)); + SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(1, 6, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || + SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(1, 1, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, 2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, std::min((size_t)4, nMaxChainLength/3)), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, nMaxChainLength/2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, nMaxChainLength), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (bSpendZeroConfChange && !fRejectLongChains && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max()), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // because SelectCoinsMinConf clears the setCoinsRet, we now add the possible inputs to the coinset setCoinsRet.insert(setPresetCoins.begin(), setPresetCoins.end()); @@ -2631,13 +2612,13 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC LOCK2(cs_main, cs_wallet); CReserveKey reservekey(this); - CWalletTx wtx; - if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) { + CTransactionRef tx_new; + if (!CreateTransaction(vecSend, tx_new, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) { return false; } if (nChangePosInOut != -1) { - tx.vout.insert(tx.vout.begin() + nChangePosInOut, wtx.tx->vout[nChangePosInOut]); + tx.vout.insert(tx.vout.begin() + nChangePosInOut, tx_new->vout[nChangePosInOut]); // We don't have the normal Create/Commit cycle, and don't want to risk // reusing change, so just remove the key from the keypool here. reservekey.KeepKey(); @@ -2646,11 +2627,11 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC // Copy output sizes from new transaction; they may have had the fee // subtracted from them. for (unsigned int idx = 0; idx < tx.vout.size(); idx++) { - tx.vout[idx].nValue = wtx.tx->vout[idx].nValue; + tx.vout[idx].nValue = tx_new->vout[idx].nValue; } // Add new txins while keeping original txin scriptSig/order. - for (const CTxIn& txin : wtx.tx->vin) { + for (const CTxIn& txin : tx_new->vin) { if (!coinControl.IsSelected(txin.prevout)) { tx.vin.push_back(txin); @@ -2666,14 +2647,14 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vector<CRecipient>& vecSend) { // If -changetype is specified, always use that change type. - if (change_type != OUTPUT_TYPE_NONE) { + if (change_type != OutputType::NONE) { return change_type; } - // if g_address_type is legacy, use legacy address as change (even + // if m_default_address_type is legacy, use legacy address as change (even // if some of the outputs are P2WPKH or P2WSH). - if (g_address_type == OUTPUT_TYPE_LEGACY) { - return OUTPUT_TYPE_LEGACY; + if (m_default_address_type == OutputType::LEGACY) { + return OutputType::LEGACY; } // if any destination is P2WPKH or P2WSH, use P2WPKH for the change @@ -2683,15 +2664,15 @@ OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vec int witnessversion = 0; std::vector<unsigned char> witnessprogram; if (recipient.scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) { - return OUTPUT_TYPE_BECH32; + return OutputType::BECH32; } } - // else use g_address_type for change - return g_address_type; + // else use m_default_address_type for change + return m_default_address_type; } -bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, +bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign) { CAmount nValue = 0; @@ -2715,8 +2696,6 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT return false; } - wtxNew.fTimeReceivedIsTxTime = true; - wtxNew.BindWallet(this); CMutableTransaction txNew; // Discourage fee sniping. @@ -2752,13 +2731,14 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT assert(txNew.nLockTime < LOCKTIME_THRESHOLD); FeeCalculation feeCalc; CAmount nFeeNeeded; - unsigned int nBytes; + int nBytes; { std::set<CInputCoin> setCoins; LOCK2(cs_main, cs_wallet); { std::vector<COutput> vAvailableCoins; AvailableCoins(vAvailableCoins, true, &coin_control); + CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy // Create change script that will be used if we need change // TODO: pass in scriptChange instead of reservekey so @@ -2786,31 +2766,40 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT return false; } - const OutputType change_type = TransactionChangeType(coin_control.change_type, vecSend); + const OutputType change_type = TransactionChangeType(coin_control.m_change_type ? *coin_control.m_change_type : m_default_change_type, vecSend); LearnRelatedScripts(vchPubKey, change_type); scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, change_type)); } CTxOut change_prototype_txout(0, scriptChange); - size_t change_prototype_size = GetSerializeSize(change_prototype_txout, SER_DISK, 0); + coin_selection_params.change_output_size = GetSerializeSize(change_prototype_txout, SER_DISK, 0); CFeeRate discard_rate = GetDiscardRate(::feeEstimator); + + // Get the fee rate to use effective values in coin selection + CFeeRate nFeeRateNeeded = GetMinimumFeeRate(coin_control, ::mempool, ::feeEstimator, &feeCalc); + nFeeRet = 0; bool pick_new_inputs = true; CAmount nValueIn = 0; + + // BnB selector is the only selector used when this is true. + // That should only happen on the first pass through the loop. + coin_selection_params.use_bnb = nSubtractFeeFromAmount == 0; // If we are doing subtract fee from recipient, then don't use BnB // Start with no fee and loop until there is enough fee while (true) { nChangePosInOut = nChangePosRequest; txNew.vin.clear(); txNew.vout.clear(); - wtxNew.fFromMe = true; bool fFirst = true; CAmount nValueToSelect = nValue; if (nSubtractFeeFromAmount == 0) nValueToSelect += nFeeRet; + // vouts to the payees + coin_selection_params.tx_noinputs_size = 11; // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 output count, 1 witness overhead (dummy, flag, stack size) for (const auto& recipient : vecSend) { CTxOut txout(recipient.nAmount, recipient.scriptPubKey); @@ -2826,6 +2815,8 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT txout.nValue -= nFeeRet % nSubtractFeeFromAmount; } } + // Include the fee cost for outputs. Note this is only used for BnB right now + coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout, SER_NETWORK, PROTOCOL_VERSION); if (IsDust(txout, ::dustRelayFee)) { @@ -2844,18 +2835,27 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT } // Choose coins to use + bool bnb_used; if (pick_new_inputs) { nValueIn = 0; setCoins.clear(); - if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, &coin_control)) + coin_selection_params.change_spend_size = CalculateMaximumSignedInputSize(change_prototype_txout, this); + coin_selection_params.effective_fee = nFeeRateNeeded; + if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coin_control, coin_selection_params, bnb_used)) { - strFailReason = _("Insufficient funds"); - return false; + // If BnB was used, it was the first pass. No longer the first pass and continue loop with knapsack. + if (bnb_used) { + coin_selection_params.use_bnb = false; + continue; + } + else { + strFailReason = _("Insufficient funds"); + return false; + } } } const CAmount nChange = nValueIn - nValueToSelect; - if (nChange > 0) { // Fill a vout to ourself @@ -2863,7 +2863,8 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT // Never create dust outputs; if we would, just // add the dust to the fee. - if (IsDust(newTxOut, discard_rate)) + // The nChange when BnB is used is always going to go to fees. + if (IsDust(newTxOut, discard_rate) || bnb_used) { nChangePosInOut = -1; nFeeRet += nChange; @@ -2903,20 +2904,12 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT txNew.vin.push_back(CTxIn(coin.outpoint,CScript(), nSequence)); - // Fill in dummy signatures for fee calculation. - if (!DummySignTx(txNew, setCoins)) { + nBytes = CalculateMaximumSignedTxSize(txNew, this); + if (nBytes < 0) { strFailReason = _("Signing transaction failed"); return false; } - nBytes = GetVirtualTransactionSize(txNew); - - // Remove scriptSigs to eliminate the fee calculation dummy signatures - for (auto& vin : txNew.vin) { - vin.scriptSig = CScript(); - vin.scriptWitness.SetNull(); - } - nFeeNeeded = GetMinimumFee(nBytes, coin_control, ::mempool, ::feeEstimator, &feeCalc); if (feeCalc.reason == FeeReason::FALLBACK && !g_wallet_allow_fallback_fee) { // eventually allow a fallback fee @@ -2944,7 +2937,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT // (because of reduced tx size) and so we should add a // change output. Only try this once. if (nChangePosInOut == -1 && nSubtractFeeFromAmount == 0 && pick_new_inputs) { - unsigned int tx_size_with_change = nBytes + change_prototype_size + 2; // Add 2 as a buffer in case increasing # of outputs changes compact size + unsigned int tx_size_with_change = nBytes + coin_selection_params.change_output_size + 2; // Add 2 as a buffer in case increasing # of outputs changes compact size CAmount fee_needed_with_change = GetMinimumFee(tx_size_with_change, coin_control, ::mempool, ::feeEstimator, nullptr); CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, discard_rate); if (nFeeRet >= fee_needed_with_change + minimum_value_for_change) { @@ -2992,6 +2985,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT // Include more fee and try again. nFeeRet = nFeeNeeded; + coin_selection_params.use_bnb = false; continue; } } @@ -3019,11 +3013,11 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT } } - // Embed the constructed transaction data in wtxNew. - wtxNew.SetTx(MakeTransactionRef(std::move(txNew))); + // Return the constructed transaction data. + tx = MakeTransactionRef(std::move(txNew)); // Limit size - if (GetTransactionWeight(*wtxNew.tx) >= MAX_STANDARD_TX_WEIGHT) + if (GetTransactionWeight(*tx) >= MAX_STANDARD_TX_WEIGHT) { strFailReason = _("Transaction too large"); return false; @@ -3033,7 +3027,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits LockPoints lp; - CTxMemPoolEntry entry(wtxNew.tx, 0, 0, 0, false, 0, lp); + CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp); CTxMemPool::setEntries setAncestors; size_t nLimitAncestors = gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); size_t nLimitAncestorSize = gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000; @@ -3060,10 +3054,18 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT /** * Call after CreateTransaction unless you want to abort */ -bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CConnman* connman, CValidationState& state) +bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, std::string fromAccount, CReserveKey& reservekey, CConnman* connman, CValidationState& state) { { LOCK2(cs_main, cs_wallet); + + CWalletTx wtxNew(this, std::move(tx)); + wtxNew.mapValue = std::move(mapValue); + wtxNew.vOrderForm = std::move(orderForm); + wtxNew.strFromAccount = std::move(fromAccount); + wtxNew.fTimeReceivedIsTxTime = true; + wtxNew.fFromMe = true; + LogPrintf("CommitTransaction:\n%s", wtxNew.tx->ToString()); { // Take key pair from key pool so it won't be used again @@ -3076,7 +3078,7 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CCon // Notify that old coins are spent for (const CTxIn& txin : wtxNew.tx->vin) { - CWalletTx &coin = mapWallet[txin.prevout.hash]; + CWalletTx &coin = mapWallet.at(txin.prevout.hash); coin.BindWallet(this); NotifyTransactionChanged(this, coin.GetHash(), CT_UPDATED); } @@ -3087,7 +3089,7 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CCon // Get the inserted-CWalletTx from mapWallet so that the // fInMempool flag is cached properly - CWalletTx& wtx = mapWallet[wtxNew.GetHash()]; + CWalletTx& wtx = mapWallet.at(wtxNew.GetHash()); if (fBroadcastTransactions) { @@ -3543,7 +3545,7 @@ std::set< std::set<CTxDestination> > CWallet::GetAddressGroupings() CTxDestination address; if(!IsMine(txin)) /* If this input isn't mine, ignore it */ continue; - if(!ExtractDestination(mapWallet[txin.prevout.hash].tx->vout[txin.prevout.n].scriptPubKey, address)) + if(!ExtractDestination(mapWallet.at(txin.prevout.hash).tx->vout[txin.prevout.n].scriptPubKey, address)) continue; grouping.insert(address); any_mine = true; @@ -3767,10 +3769,10 @@ void CWallet::GetKeyBirthTimes(std::map<CTxDestination, int64_t> &mapKeyBirth) c for (const auto& entry : mapWallet) { // iterate over all wallet transactions... const CWalletTx &wtx = entry.second; - BlockMap::const_iterator blit = mapBlockIndex.find(wtx.hashBlock); - if (blit != mapBlockIndex.end() && chainActive.Contains(blit->second)) { + CBlockIndex* pindex = LookupBlockIndex(wtx.hashBlock); + if (pindex && chainActive.Contains(pindex)) { // ... which are already in a block - int nHeight = blit->second->nHeight; + int nHeight = pindex->nHeight; for (const CTxOut &txout : wtx.tx->vout) { // iterate over all their outputs CAffectedKeysVisitor(*this, vAffected).Process(txout.scriptPubKey); @@ -3778,7 +3780,7 @@ void CWallet::GetKeyBirthTimes(std::map<CTxDestination, int64_t> &mapKeyBirth) c // ... and all their affected keys std::map<CKeyID, CBlockIndex*>::iterator rit = mapKeyFirstBlock.find(keyid); if (rit != mapKeyFirstBlock.end() && nHeight < rit->second->nHeight) - rit->second = blit->second; + rit->second = pindex; } vAffected.clear(); } @@ -3815,7 +3817,7 @@ unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const { unsigned int nTimeSmart = wtx.nTimeReceived; if (!wtx.hashUnset()) { - if (mapBlockIndex.count(wtx.hashBlock)) { + if (const CBlockIndex* pindex = LookupBlockIndex(wtx.hashBlock)) { int64_t latestNow = wtx.nTimeReceived; int64_t latestEntry = 0; @@ -3846,7 +3848,7 @@ unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const } } - int64_t blocktime = mapBlockIndex[wtx.hashBlock]->GetBlockTime(); + int64_t blocktime = pindex->GetBlockTime(); nTimeSmart = std::max(latestEntry, std::min(blocktime, latestNow)); } else { LogPrintf("%s: found %s in block %s not in index\n", __func__, wtx.GetHash().ToString(), wtx.hashBlock.ToString()); @@ -3998,8 +4000,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& } walletInstance->SetBestChain(chainActive.GetLocator()); - } - else if (gArgs.IsArgSet("-usehd")) { + } else if (gArgs.IsArgSet("-usehd")) { bool useHD = gArgs.GetBoolArg("-usehd", true); if (walletInstance->IsHDEnabled() && !useHD) { InitError(strprintf(_("Error loading %s: You can't disable HD on an already existing HD wallet"), walletFile)); @@ -4011,11 +4012,27 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& } } + walletInstance->m_default_address_type = ParseOutputType(gArgs.GetArg("-addresstype", ""), DEFAULT_ADDRESS_TYPE); + if (walletInstance->m_default_address_type == OutputType::NONE) { + InitError(strprintf("Unknown address type '%s'", gArgs.GetArg("-addresstype", ""))); + return nullptr; + } + + // If changetype is set in config file or parameter, check that it's valid. + // Default to OutputType::NONE if not set. + walletInstance->m_default_change_type = ParseOutputType(gArgs.GetArg("-changetype", ""), OutputType::NONE); + if (walletInstance->m_default_change_type == OutputType::NONE && !gArgs.GetArg("-changetype", "").empty()) { + InitError(strprintf("Unknown change type '%s'", gArgs.GetArg("-changetype", ""))); + return nullptr; + } + LogPrintf(" wallet %15dms\n", GetTimeMillis() - nStart); // Try to top up keypool. No-op if the wallet is locked. walletInstance->TopUpKeyPool(); + LOCK(cs_main); + CBlockIndex *pindexRescan = chainActive.Genesis(); if (!gArgs.GetBoolArg("-rescan", false)) { @@ -4159,10 +4176,7 @@ int CMerkleTx::GetDepthInMainChain(const CBlockIndex* &pindexRet) const AssertLockHeld(cs_main); // Find the block it claims to be in - BlockMap::iterator mi = mapBlockIndex.find(hashBlock); - if (mi == mapBlockIndex.end()) - return 0; - CBlockIndex* pindex = (*mi).second; + CBlockIndex* pindex = LookupBlockIndex(hashBlock); if (!pindex || !chainActive.Contains(pindex)) return 0; @@ -4200,29 +4214,29 @@ OutputType ParseOutputType(const std::string& type, OutputType default_type) if (type.empty()) { return default_type; } else if (type == OUTPUT_TYPE_STRING_LEGACY) { - return OUTPUT_TYPE_LEGACY; + return OutputType::LEGACY; } else if (type == OUTPUT_TYPE_STRING_P2SH_SEGWIT) { - return OUTPUT_TYPE_P2SH_SEGWIT; + return OutputType::P2SH_SEGWIT; } else if (type == OUTPUT_TYPE_STRING_BECH32) { - return OUTPUT_TYPE_BECH32; + return OutputType::BECH32; } else { - return OUTPUT_TYPE_NONE; + return OutputType::NONE; } } const std::string& FormatOutputType(OutputType type) { switch (type) { - case OUTPUT_TYPE_LEGACY: return OUTPUT_TYPE_STRING_LEGACY; - case OUTPUT_TYPE_P2SH_SEGWIT: return OUTPUT_TYPE_STRING_P2SH_SEGWIT; - case OUTPUT_TYPE_BECH32: return OUTPUT_TYPE_STRING_BECH32; + case OutputType::LEGACY: return OUTPUT_TYPE_STRING_LEGACY; + case OutputType::P2SH_SEGWIT: return OUTPUT_TYPE_STRING_P2SH_SEGWIT; + case OutputType::BECH32: return OUTPUT_TYPE_STRING_BECH32; default: assert(false); } } void CWallet::LearnRelatedScripts(const CPubKey& key, OutputType type) { - if (key.IsCompressed() && (type == OUTPUT_TYPE_P2SH_SEGWIT || type == OUTPUT_TYPE_BECH32)) { + if (key.IsCompressed() && (type == OutputType::P2SH_SEGWIT || type == OutputType::BECH32)) { CTxDestination witdest = WitnessV0KeyHash(key.GetID()); CScript witprog = GetScriptForDestination(witdest); // Make sure the resulting program is solvable. @@ -4233,20 +4247,20 @@ void CWallet::LearnRelatedScripts(const CPubKey& key, OutputType type) void CWallet::LearnAllRelatedScripts(const CPubKey& key) { - // OUTPUT_TYPE_P2SH_SEGWIT always adds all necessary scripts for all types. - LearnRelatedScripts(key, OUTPUT_TYPE_P2SH_SEGWIT); + // OutputType::P2SH_SEGWIT always adds all necessary scripts for all types. + LearnRelatedScripts(key, OutputType::P2SH_SEGWIT); } CTxDestination GetDestinationForKey(const CPubKey& key, OutputType type) { switch (type) { - case OUTPUT_TYPE_LEGACY: return key.GetID(); - case OUTPUT_TYPE_P2SH_SEGWIT: - case OUTPUT_TYPE_BECH32: { + case OutputType::LEGACY: return key.GetID(); + case OutputType::P2SH_SEGWIT: + case OutputType::BECH32: { if (!key.IsCompressed()) return key.GetID(); CTxDestination witdest = WitnessV0KeyHash(key.GetID()); CScript witprog = GetScriptForDestination(witdest); - if (type == OUTPUT_TYPE_P2SH_SEGWIT) { + if (type == OutputType::P2SH_SEGWIT) { return CScriptID(witprog); } else { return witdest; @@ -4272,10 +4286,10 @@ CTxDestination CWallet::AddAndGetDestinationForScript(const CScript& script, Out { // Note that scripts over 520 bytes are not yet supported. switch (type) { - case OUTPUT_TYPE_LEGACY: + case OutputType::LEGACY: return CScriptID(script); - case OUTPUT_TYPE_P2SH_SEGWIT: - case OUTPUT_TYPE_BECH32: { + case OutputType::P2SH_SEGWIT: + case OutputType::BECH32: { WitnessV0ScriptHash hash; CSHA256().Write(script.data(), script.size()).Finalize(hash.begin()); CTxDestination witdest = hash; @@ -4284,7 +4298,7 @@ CTxDestination CWallet::AddAndGetDestinationForScript(const CScript& script, Out if (!IsSolvable(*this, witprog)) return CScriptID(script); // Add the redeemscript, so that P2WSH and P2SH-P2WSH outputs are recognized as ours. AddCScript(witprog); - if (type == OUTPUT_TYPE_BECH32) { + if (type == OutputType::BECH32) { return witdest; } else { return CScriptID(witprog); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3e2d1794d8..5ac8457eb4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -17,6 +17,7 @@ #include <script/sign.h> #include <util.h> #include <wallet/crypter.h> +#include <wallet/coinselection.h> #include <wallet/walletdb.h> #include <wallet/rpcwallet.h> @@ -42,6 +43,7 @@ extern bool bSpendZeroConfChange; extern bool fWalletRbf; extern bool g_wallet_allow_fallback_fee; +//! Default for -keypool static const unsigned int DEFAULT_KEYPOOL_SIZE = 1000; //! -paytxfee default static const CAmount DEFAULT_TRANSACTION_FEE = 0; @@ -53,10 +55,6 @@ static const CAmount DEFAULT_DISCARD_FEE = 10000; static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000; //! minimum recommended increment for BIP 125 replacement txs static const CAmount WALLET_INCREMENTAL_RELAY_FEE = 5000; -//! target minimum change amount -static const CAmount MIN_CHANGE = CENT; -//! final minimum change amount after paying for fees -static const CAmount MIN_FINAL_CHANGE = MIN_CHANGE/2; //! Default for -spendzeroconfchange static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true; //! Default for -walletrejectlongchains @@ -99,18 +97,15 @@ enum WalletFeature FEATURE_LATEST = FEATURE_COMPRPUBKEY // HD is optional, use FEATURE_COMPRPUBKEY as latest version }; -enum OutputType : int -{ - OUTPUT_TYPE_NONE, - OUTPUT_TYPE_LEGACY, - OUTPUT_TYPE_P2SH_SEGWIT, - OUTPUT_TYPE_BECH32, - - OUTPUT_TYPE_DEFAULT = OUTPUT_TYPE_P2SH_SEGWIT +enum class OutputType { + NONE, + LEGACY, + P2SH_SEGWIT, + BECH32, }; -extern OutputType g_address_type; -extern OutputType g_change_type; +//! Default for -addresstype +constexpr OutputType DEFAULT_ADDRESS_TYPE{OutputType::P2SH_SEGWIT}; /** A key pool entry */ @@ -269,6 +264,9 @@ public: bool IsCoinBase() const { return tx->IsCoinBase(); } }; +//Get the marginal bytes of spending the specified output +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet); + /** * A transaction with a bunch of additional info that only the owner cares about. * It includes any unrecorded transactions needed to link it back to the block chain. @@ -348,11 +346,6 @@ public: mutable CAmount nAvailableWatchCreditCached; mutable CAmount nChangeCached; - CWalletTx() - { - Init(nullptr); - } - CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) : CMerkleTx(std::move(arg)) { Init(pwalletIn); @@ -390,42 +383,36 @@ public: nOrderPos = -1; } - ADD_SERIALIZE_METHODS; - - template <typename Stream, typename Operation> - inline void SerializationOp(Stream& s, Operation ser_action) { - if (ser_action.ForRead()) - Init(nullptr); + template<typename Stream> + void Serialize(Stream& s) const + { char fSpent = false; + mapValue_t mapValueCopy = mapValue; - if (!ser_action.ForRead()) - { - mapValue["fromaccount"] = strFromAccount; - - WriteOrderPos(nOrderPos, mapValue); - - if (nTimeSmart) - mapValue["timesmart"] = strprintf("%u", nTimeSmart); + mapValueCopy["fromaccount"] = strFromAccount; + WriteOrderPos(nOrderPos, mapValueCopy); + if (nTimeSmart) { + mapValueCopy["timesmart"] = strprintf("%u", nTimeSmart); } - READWRITE(*static_cast<CMerkleTx*>(this)); + s << *static_cast<const CMerkleTx*>(this); std::vector<CMerkleTx> vUnused; //!< Used to be vtxPrev - READWRITE(vUnused); - READWRITE(mapValue); - READWRITE(vOrderForm); - READWRITE(fTimeReceivedIsTxTime); - READWRITE(nTimeReceived); - READWRITE(fFromMe); - READWRITE(fSpent); - - if (ser_action.ForRead()) - { - strFromAccount = mapValue["fromaccount"]; + s << vUnused << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << fSpent; + } - ReadOrderPos(nOrderPos, mapValue); + template<typename Stream> + void Unserialize(Stream& s) + { + Init(nullptr); + char fSpent; - nTimeSmart = mapValue.count("timesmart") ? (unsigned int)atoi64(mapValue["timesmart"]) : 0; - } + s >> *static_cast<CMerkleTx*>(this); + std::vector<CMerkleTx> vUnused; //!< Used to be vtxPrev + s >> vUnused >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> fSpent; + + strFromAccount = std::move(mapValue["fromaccount"]); + ReadOrderPos(nOrderPos, mapValue); + nTimeSmart = mapValue.count("timesmart") ? (unsigned int)atoi64(mapValue["timesmart"]) : 0; mapValue.erase("fromaccount"); mapValue.erase("spent"); @@ -462,6 +449,12 @@ public: CAmount GetAvailableWatchOnlyCredit(const bool fUseCache=true) const; CAmount GetChange() const; + // Get the marginal bytes if spending the specified output from this transaction + int GetSpendSize(unsigned int out) const + { + return CalculateMaximumSignedInputSize(tx->vout[out], pwallet); + } + void GetAmounts(std::list<COutputEntry>& listReceived, std::list<COutputEntry>& listSent, CAmount& nFee, std::string& strSentAccount, const isminefilter& filter) const; @@ -488,36 +481,6 @@ public: std::set<uint256> GetConflicts() const; }; - -class CInputCoin { -public: - CInputCoin(const CWalletTx* walletTx, unsigned int i) - { - if (!walletTx) - throw std::invalid_argument("walletTx should not be null"); - if (i >= walletTx->tx->vout.size()) - throw std::out_of_range("The output index is out of range"); - - outpoint = COutPoint(walletTx->GetHash(), i); - txout = walletTx->tx->vout[i]; - } - - COutPoint outpoint; - CTxOut txout; - - bool operator<(const CInputCoin& rhs) const { - return outpoint < rhs.outpoint; - } - - bool operator!=(const CInputCoin& rhs) const { - return outpoint != rhs.outpoint; - } - - bool operator==(const CInputCoin& rhs) const { - return outpoint == rhs.outpoint; - } -}; - class COutput { public: @@ -525,6 +488,9 @@ public: int i; int nDepth; + /** Pre-computed estimated size of this output as a fully-signed input in a transaction. Can be -1 if it could not be calculated */ + int nInputBytes; + /** Whether we have the private keys to spend this output */ bool fSpendable; @@ -540,7 +506,12 @@ public: COutput(const CWalletTx *txIn, int iIn, int nDepthIn, bool fSpendableIn, bool fSolvableIn, bool fSafeIn) { - tx = txIn; i = iIn; nDepth = nDepthIn; fSpendable = fSpendableIn; fSolvable = fSolvableIn; fSafe = fSafeIn; + tx = txIn; i = iIn; nDepth = nDepthIn; fSpendable = fSpendableIn; fSolvable = fSolvableIn; fSafe = fSafeIn; nInputBytes = -1; + // If known and signable by the given wallet, compute nInputBytes + // Failure will keep this value -1 + if (fSpendable && tx) { + nInputBytes = tx->GetSpendSize(i); + } } std::string ToString() const; @@ -608,48 +579,49 @@ public: nEntryNo = 0; } - ADD_SERIALIZE_METHODS; - - template <typename Stream, typename Operation> - inline void SerializationOp(Stream& s, Operation ser_action) { + template <typename Stream> + void Serialize(Stream& s) const { int nVersion = s.GetVersion(); - if (!(s.GetType() & SER_GETHASH)) - READWRITE(nVersion); + if (!(s.GetType() & SER_GETHASH)) { + s << nVersion; + } //! Note: strAccount is serialized as part of the key, not here. - READWRITE(nCreditDebit); - READWRITE(nTime); - READWRITE(LIMITED_STRING(strOtherAccount, 65536)); - - if (!ser_action.ForRead()) - { - WriteOrderPos(nOrderPos, mapValue); - - if (!(mapValue.empty() && _ssExtra.empty())) - { - CDataStream ss(s.GetType(), s.GetVersion()); - ss.insert(ss.begin(), '\0'); - ss << mapValue; - ss.insert(ss.end(), _ssExtra.begin(), _ssExtra.end()); - strComment.append(ss.str()); - } + s << nCreditDebit << nTime << strOtherAccount; + + mapValue_t mapValueCopy = mapValue; + WriteOrderPos(nOrderPos, mapValueCopy); + + std::string strCommentCopy = strComment; + if (!mapValueCopy.empty() || !_ssExtra.empty()) { + CDataStream ss(s.GetType(), s.GetVersion()); + ss.insert(ss.begin(), '\0'); + ss << mapValueCopy; + ss.insert(ss.end(), _ssExtra.begin(), _ssExtra.end()); + strCommentCopy.append(ss.str()); } + s << strCommentCopy; + } - READWRITE(LIMITED_STRING(strComment, 65536)); + template <typename Stream> + void Unserialize(Stream& s) { + int nVersion = s.GetVersion(); + if (!(s.GetType() & SER_GETHASH)) { + s >> nVersion; + } + //! Note: strAccount is serialized as part of the key, not here. + s >> nCreditDebit >> nTime >> LIMITED_STRING(strOtherAccount, 65536) >> LIMITED_STRING(strComment, 65536); size_t nSepPos = strComment.find("\0", 0, 1); - if (ser_action.ForRead()) - { - mapValue.clear(); - if (std::string::npos != nSepPos) - { - CDataStream ss(std::vector<char>(strComment.begin() + nSepPos + 1, strComment.end()), s.GetType(), s.GetVersion()); - ss >> mapValue; - _ssExtra = std::vector<char>(ss.begin(), ss.end()); - } - ReadOrderPos(nOrderPos, mapValue); + mapValue.clear(); + if (std::string::npos != nSepPos) { + CDataStream ss(std::vector<char>(strComment.begin() + nSepPos + 1, strComment.end()), s.GetType(), s.GetVersion()); + ss >> mapValue; + _ssExtra = std::vector<char>(ss.begin(), ss.end()); } - if (std::string::npos != nSepPos) + ReadOrderPos(nOrderPos, mapValue); + if (std::string::npos != nSepPos) { strComment.erase(nSepPos); + } mapValue.erase("n"); } @@ -658,6 +630,26 @@ private: std::vector<char> _ssExtra; }; +struct CoinSelectionParams +{ + bool use_bnb = true; + size_t change_output_size = 0; + size_t change_spend_size = 0; + CFeeRate effective_fee = CFeeRate(0); + size_t tx_noinputs_size = 0; + + CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee, size_t tx_noinputs_size) : use_bnb(use_bnb), change_output_size(change_output_size), change_spend_size(change_spend_size), effective_fee(effective_fee), tx_noinputs_size(tx_noinputs_size) {} + CoinSelectionParams() {} +}; + +struct CoinEligibilityFilter +{ + const int conf_mine; + const int conf_theirs; + const uint64_t max_ancestors; + + CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors) {} +}; class WalletRescanReserver; //forward declarations for ScanForWalletTransactions/RescanFromTime /** @@ -679,7 +671,8 @@ private: * all coins from coinControl are selected; Never select unconfirmed coins * if they are not ours */ - bool SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CCoinControl *coinControl = nullptr) const; + bool SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, + const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const; CWalletDB *pwalletdbEncryption; @@ -860,7 +853,8 @@ public: * completion the coin set and corresponding actual target value is * assembled */ - bool SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int nConfTheirs, uint64_t nMaxAncestors, std::vector<COutput> vCoins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet) const; + bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibilty_filter, std::vector<COutput> vCoins, + std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const; bool IsSpent(const uint256& hash, unsigned int n) const; @@ -974,19 +968,27 @@ public: * selected by SelectCoins(); Also create the change output, when needed * @note passing nChangePosInOut as -1 will result in setting a random position */ - bool CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, + bool CreateTransaction(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign = true); - bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CConnman* connman, CValidationState& state); + bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, std::string fromAccount, CReserveKey& reservekey, CConnman* connman, CValidationState& state); void ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& entries); bool AddAccountingEntry(const CAccountingEntry&); bool AddAccountingEntry(const CAccountingEntry&, CWalletDB *pwalletdb); - template <typename ContainerType> - bool DummySignTx(CMutableTransaction &txNew, const ContainerType &coins) const; + bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts) const + { + std::vector<CTxOut> v_txouts(txouts.size()); + std::copy(txouts.begin(), txouts.end(), v_txouts.begin()); + return DummySignTx(txNew, v_txouts); + } + bool DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts) const; + bool DummySignInput(CTxIn &tx_in, const CTxOut &txout) const; static CFeeRate minTxFee; static CFeeRate fallbackFee; static CFeeRate m_discard_rate; + OutputType m_default_address_type{DEFAULT_ADDRESS_TYPE}; + OutputType m_default_change_type{OutputType::NONE}; // Default to OutputType::NONE if not set by -changetype bool NewKeyPool(); size_t KeypoolCountExternalKeys(); @@ -1163,6 +1165,9 @@ public: * This function will automatically add the necessary scripts to the wallet. */ CTxDestination AddAndGetDestinationForScript(const CScript& script, OutputType); + + /** Whether a given output is spendable by this wallet */ + bool OutputEligibleForSpending(const COutput& output, const CoinEligibilityFilter& eligibilty_filter) const; }; /** A key allocated from the key pool. */ @@ -1227,32 +1232,7 @@ public: } }; -// Helper for producing a bunch of max-sized low-S signatures (eg 72 bytes) -// ContainerType is meant to hold pair<CWalletTx *, int>, and be iterable -// so that each entry corresponds to each vIn, in order. -template <typename ContainerType> -bool CWallet::DummySignTx(CMutableTransaction &txNew, const ContainerType &coins) const -{ - // Fill in dummy signatures for fee calculation. - int nIn = 0; - for (const auto& coin : coins) - { - const CScript& scriptPubKey = coin.txout.scriptPubKey; - SignatureData sigdata; - - if (!ProduceSignature(DummySignatureCreator(this), scriptPubKey, sigdata)) - { - return false; - } else { - UpdateTransaction(txNew, nIn, sigdata); - } - - nIn++; - } - return true; -} - -OutputType ParseOutputType(const std::string& str, OutputType default_type = OUTPUT_TYPE_DEFAULT); +OutputType ParseOutputType(const std::string& str, OutputType default_type); const std::string& FormatOutputType(OutputType type); /** @@ -1299,4 +1279,10 @@ public: } }; +// Calculate the size of the transaction assuming all signatures are max size +// Use DummySignatureCreator, which inserts 72 byte signatures everywhere. +// NOTE: this requires that all inputs must be in mapWallet (eg the tx should +// be IsAllFromMe). +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet); +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts); #endif // BITCOIN_WALLET_WALLET_H diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 0b0880a2ba..7f5f3b84b2 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -265,7 +265,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, { uint256 hash; ssKey >> hash; - CWalletTx wtx; + CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef()); ssValue >> wtx; CValidationState state; if (!(CheckTransaction(*wtx.tx, state) && (wtx.GetHash() == hash) && state.IsValid())) @@ -603,7 +603,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet) pwallet->UpdateTimeFirstKey(1); for (uint256 hash : wss.vWalletUpgrade) - WriteTx(pwallet->mapWallet[hash]); + WriteTx(pwallet->mapWallet.at(hash)); // Rewrite encrypted wallets of versions 0.4.0 and 0.5.0rc: if (wss.fIsEncrypted && (wss.nFileVersion == 40000 || wss.nFileVersion == 50000)) @@ -664,7 +664,7 @@ DBErrors CWalletDB::FindWalletTx(std::vector<uint256>& vTxHash, std::vector<CWal uint256 hash; ssKey >> hash; - CWalletTx wtx; + CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef()); ssValue >> wtx; vTxHash.push_back(hash); |