diff options
38 files changed, 516 insertions, 213 deletions
diff --git a/depends/packages/expat.mk b/depends/packages/expat.mk index 81a660e83a..7f484724a4 100644 --- a/depends/packages/expat.mk +++ b/depends/packages/expat.mk @@ -1,8 +1,8 @@ package=expat -$(package)_version=2.2.0 +$(package)_version=2.2.1 $(package)_download_path=https://downloads.sourceforge.net/project/expat/expat/$($(package)_version) $(package)_file_name=$(package)-$($(package)_version).tar.bz2 -$(package)_sha256_hash=d9e50ff2d19b3538bd2127902a89987474e1a4db8e43a66a4d1a712ab9a504ff +$(package)_sha256_hash=1868cadae4c82a018e361e2b2091de103cd820aaacb0d6cfa49bd2cd83978885 define $(package)_set_vars $(package)_config_opts=--disable-static diff --git a/doc/release-notes/release-notes-0.14.2.md b/doc/release-notes/release-notes-0.14.2.md new file mode 100644 index 0000000000..0ad554b773 --- /dev/null +++ b/doc/release-notes/release-notes-0.14.2.md @@ -0,0 +1,102 @@ +Bitcoin Core version 0.14.2 is now available from: + + <https://bitcoin.org/bin/bitcoin-core-0.14.2/> + +This is a new minor version release, including various bugfixes and +performance improvements, as well as updated translations. + +Please report bugs using the issue tracker at github: + + <https://github.com/bitcoin/bitcoin/issues> + +To receive security and update notifications, please subscribe to: + + <https://bitcoincore.org/en/list/announcements/join/> + +Compatibility +============== + +Bitcoin Core is extensively tested on multiple operating systems using +the Linux kernel, macOS 10.8+, and Windows Vista and later. + +Microsoft ended support for Windows XP on [April 8th, 2014](https://www.microsoft.com/en-us/WindowsForBusiness/end-of-xp-support), +No attempt is made to prevent installing or running the software on Windows XP, you +can still do so at your own risk but be aware that there are known instabilities and issues. +Please do not report issues about Windows XP to the issue tracker. + +Bitcoin Core should also work on most other Unix-like systems but is not +frequently tested on them. + +Notable changes +=============== + +miniupnp CVE-2017-8798 +---------------------------- + +Bundled miniupnpc was updated to 2.0.20170509. This fixes an integer signedness error +(present in MiniUPnPc v1.4.20101221 through v2.0) that allows remote attackers +(within the LAN) to cause a denial of service or possibly have unspecified +other impact. + +This only affects users that have explicitly enabled UPnP through the GUI +setting or through the `-upnp` option, as since the last UPnP vulnerability +(in Bitcoin Core 0.10.3) it has been disabled by default. + +If you use this option, it is recommended to upgrade to this version as soon as +possible. + +Known Bugs +========== + +Since 0.14.0 the approximate transaction fee shown in Bitcoin-Qt when using coin +control and smart fee estimation does not reflect any change in target from the +smart fee slider. It will only present an approximate fee calculated using the +default target. The fee calculated using the correct target is still applied to +the transaction and shown in the final send confirmation dialog. + +0.14.2 Change log +================= + +Detailed release notes follow. This overview includes changes that affect +behavior, not code moves, refactors and string updates. For convenience in locating +the code changes and accompanying discussion, both the pull request and +git merge commit are mentioned. + +### RPC and other APIs +- #10410 `321419b` Fix importwallet edge case rescan bug (ryanofsky) + +### P2P protocol and network code +- #10424 `37a8fc5` Populate services in GetLocalAddress (morcos) +- #10441 `9e3ad50` Only enforce expected services for half of outgoing connections (theuni) + +### Build system +- #10414 `ffb0c4b` miniupnpc 2.0.20170509 (fanquake) +- #10228 `ae479bc` Regenerate bitcoin-config.h as necessary (theuni) + +### Miscellaneous +- #10245 `44a17f2` Minor fix in build documentation for FreeBSD 11 (shigeya) +- #10215 `0aee4a1` Check interruptNet during dnsseed lookups (TheBlueMatt) + +### GUI +- #10231 `1e936d7` Reduce a significant cs_main lock freeze (jonasschnelli) + +### Wallet +- #10294 `1847642` Unset change position when there is no change (instagibbs) + +Credits +======= + +Thanks to everyone who directly contributed to this release: + +- Alex Morcos +- Cory Fields +- fanquake +- Gregory Sanders +- Jonas Schnelli +- Matt Corallo +- Russell Yanofsky +- Shigeya Suzuki +- Wladimir J. van der Laan + +As well as everyone that helped translating on [Transifex](https://www.transifex.com/projects/p/bitcoin/). + diff --git a/src/.clang-format b/src/.clang-format index fc53509138..5918819d13 100644 --- a/src/.clang-format +++ b/src/.clang-format @@ -47,6 +47,6 @@ SpacesInAngles: false SpacesInContainerLiterals: true SpacesInCStyleCastParentheses: false SpacesInParentheses: false -Standard: Cpp03 +Standard: Cpp11 TabWidth: 8 UseTab: Never diff --git a/src/arith_uint256.cpp b/src/arith_uint256.cpp index dd34a313b7..b4952af6f4 100644 --- a/src/arith_uint256.cpp +++ b/src/arith_uint256.cpp @@ -15,6 +15,8 @@ template <unsigned int BITS> base_uint<BITS>::base_uint(const std::string& str) { + static_assert(BITS/32 > 0 && BITS%32 == 0, "Template parameter BITS must be a positive multiple of 32."); + SetHex(str); } diff --git a/src/arith_uint256.h b/src/arith_uint256.h index 0f6b3d4fba..c7734035df 100644 --- a/src/arith_uint256.h +++ b/src/arith_uint256.h @@ -31,12 +31,16 @@ public: base_uint() { + static_assert(BITS/32 > 0 && BITS%32 == 0, "Template parameter BITS must be a positive multiple of 32."); + for (int i = 0; i < WIDTH; i++) pn[i] = 0; } base_uint(const base_uint& b) { + static_assert(BITS/32 > 0 && BITS%32 == 0, "Template parameter BITS must be a positive multiple of 32."); + for (int i = 0; i < WIDTH; i++) pn[i] = b.pn[i]; } @@ -50,6 +54,8 @@ public: base_uint(uint64_t b) { + static_assert(BITS/32 > 0 && BITS%32 == 0, "Template parameter BITS must be a positive multiple of 32."); + pn[0] = (unsigned int)b; pn[1] = (unsigned int)(b >> 32); for (int i = 2; i < WIDTH; i++) @@ -174,7 +180,7 @@ public: { // prefix operator int i = 0; - while (++pn[i] == 0 && i < WIDTH-1) + while (i < WIDTH && ++pn[i] == 0) i++; return *this; } @@ -191,7 +197,7 @@ public: { // prefix operator int i = 0; - while (--pn[i] == (uint32_t)-1 && i < WIDTH-1) + while (i < WIDTH && --pn[i] == (uint32_t)-1) i++; return *this; } diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 714ee555ec..6093f78fb1 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -299,7 +299,6 @@ static void MutateTxAddOutPubKey(CMutableTransaction& tx, const std::string& str if (!pubkey.IsFullyValid()) throw std::runtime_error("invalid TX output pubkey"); CScript scriptPubKey = GetScriptForRawPubKey(pubkey); - CBitcoinAddress addr(scriptPubKey); // Extract and validate FLAGS bool bSegWit = false; diff --git a/src/coins.cpp b/src/coins.cpp index b45fc76338..2dceaf09b6 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -91,9 +91,9 @@ void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight) { } } -void CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) { +bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) { CCoinsMap::iterator it = FetchCoin(outpoint); - if (it == cacheCoins.end()) return; + if (it == cacheCoins.end()) return false; cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); if (moveout) { *moveout = std::move(it->second.coin); @@ -104,6 +104,7 @@ void CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) { it->second.flags |= CCoinsCacheEntry::DIRTY; it->second.coin.Clear(); } + return true; } static const Coin coinEmpty; diff --git a/src/coins.h b/src/coins.h index dc3210b8ac..4774c9f6a6 100644 --- a/src/coins.h +++ b/src/coins.h @@ -224,8 +224,13 @@ public: /** * Return a reference to Coin in the cache, or a pruned one if not found. This is - * more efficient than GetCoin. Modifications to other cache entries are - * allowed while accessing the returned pointer. + * more efficient than GetCoin. + * + * Generally, do not hold the reference returned for more than a short scope. + * While the current implementation allows for modifications to the contents + * of the cache while holding the reference, this behavior should not be relied + * on! To be safe, best to not hold the returned reference through any other + * calls to this cache. */ const Coin& AccessCoin(const COutPoint &output) const; @@ -240,7 +245,7 @@ public: * If no unspent output exists for the passed outpoint, this call * has no effect. */ - void SpendCoin(const COutPoint &outpoint, Coin* moveto = nullptr); + bool SpendCoin(const COutPoint &outpoint, Coin* moveto = nullptr); /** * Push the modifications applied to this cache to its base. diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index bf68f8754b..0671cbc132 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -126,7 +126,9 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& in unsigned int nSigOps = 0; for (unsigned int i = 0; i < tx.vin.size(); i++) { - const CTxOut &prevout = inputs.AccessCoin(tx.vin[i].prevout).out; + const Coin& coin = inputs.AccessCoin(tx.vin[i].prevout); + assert(!coin.IsSpent()); + const CTxOut &prevout = coin.out; if (prevout.scriptPubKey.IsPayToScriptHash()) nSigOps += prevout.scriptPubKey.GetSigOpCount(tx.vin[i].scriptSig); } @@ -146,7 +148,9 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i for (unsigned int i = 0; i < tx.vin.size(); i++) { - const CTxOut &prevout = inputs.AccessCoin(tx.vin[i].prevout).out; + const Coin& coin = inputs.AccessCoin(tx.vin[i].prevout); + assert(!coin.IsSpent()); + const CTxOut &prevout = coin.out; nSigOps += CountWitnessSigOps(tx.vin[i].scriptSig, prevout.scriptPubKey, &tx.vin[i].scriptWitness, flags); } return nSigOps; diff --git a/src/init.cpp b/src/init.cpp index ed7695344d..e213958893 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -844,8 +844,6 @@ bool AppInitBasicSetup() // Turn off Microsoft heap dump noise _CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE); _CrtSetReportFile(_CRT_WARN, CreateFileA("NUL", GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, 0)); -#endif -#if _MSC_VER >= 1400 // Disable confusing "helpful" text message on abort, Ctrl-C _set_abort_behavior(0, _WRITE_ABORT_MSG | _CALL_REPORTFAULT); #endif diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b9357440e9..91681265b1 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -342,7 +342,9 @@ bool MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const CBlockIndex* // Short-circuit most stuff in case its from the same node std::map<uint256, std::pair<NodeId, std::list<QueuedBlock>::iterator> >::iterator itInFlight = mapBlocksInFlight.find(hash); if (itInFlight != mapBlocksInFlight.end() && itInFlight->second.first == nodeid) { - *pit = &itInFlight->second.second; + if (pit) { + *pit = &itInFlight->second.second; + } return false; } diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 3c3a2fb651..771491770e 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -16,6 +16,26 @@ static constexpr double INF_FEERATE = 1e99; +std::string StringForFeeReason(FeeReason reason) { + static const std::map<FeeReason, std::string> fee_reason_strings = { + {FeeReason::NONE, "None"}, + {FeeReason::HALF_ESTIMATE, "Half Target 60% Threshold"}, + {FeeReason::FULL_ESTIMATE, "Target 85% Threshold"}, + {FeeReason::DOUBLE_ESTIMATE, "Double Target 95% Threshold"}, + {FeeReason::CONSERVATIVE, "Conservative Double Target longer horizon"}, + {FeeReason::MEMPOOL_MIN, "Mempool Min Fee"}, + {FeeReason::PAYTXFEE, "PayTxFee set"}, + {FeeReason::FALLBACK, "Fallback fee"}, + {FeeReason::REQUIRED, "Minimum Required Fee"}, + {FeeReason::MAXTXFEE, "MaxTxFee limit"} + }; + auto reason_string = fee_reason_strings.find(reason); + + if (reason_string == fee_reason_strings.end()) return "Unknown"; + + return reason_string->second; +} + /** * We will instantiate an instance of this class to track transactions that were * included in a block. We will lump transactions into a bucket according to their @@ -698,31 +718,36 @@ unsigned int CBlockPolicyEstimator::MaxUsableEstimate() const * time horizon which tracks confirmations up to the desired target. If * checkShorterHorizon is requested, also allow short time horizon estimates * for a lower target to reduce the given answer */ -double CBlockPolicyEstimator::estimateCombinedFee(unsigned int confTarget, double successThreshold, bool checkShorterHorizon) const +double CBlockPolicyEstimator::estimateCombinedFee(unsigned int confTarget, double successThreshold, bool checkShorterHorizon, EstimationResult *result) const { double estimate = -1; if (confTarget >= 1 && confTarget <= longStats->GetMaxConfirms()) { // Find estimate from shortest time horizon possible if (confTarget <= shortStats->GetMaxConfirms()) { // short horizon - estimate = shortStats->EstimateMedianVal(confTarget, SUFFICIENT_TXS_SHORT, successThreshold, true, nBestSeenHeight); + estimate = shortStats->EstimateMedianVal(confTarget, SUFFICIENT_TXS_SHORT, successThreshold, true, nBestSeenHeight, result); } else if (confTarget <= feeStats->GetMaxConfirms()) { // medium horizon - estimate = feeStats->EstimateMedianVal(confTarget, SUFFICIENT_FEETXS, successThreshold, true, nBestSeenHeight); + estimate = feeStats->EstimateMedianVal(confTarget, SUFFICIENT_FEETXS, successThreshold, true, nBestSeenHeight, result); } else { // long horizon - estimate = longStats->EstimateMedianVal(confTarget, SUFFICIENT_FEETXS, successThreshold, true, nBestSeenHeight); + estimate = longStats->EstimateMedianVal(confTarget, SUFFICIENT_FEETXS, successThreshold, true, nBestSeenHeight, result); } if (checkShorterHorizon) { + EstimationResult tempResult; // If a lower confTarget from a more recent horizon returns a lower answer use it. if (confTarget > feeStats->GetMaxConfirms()) { - double medMax = feeStats->EstimateMedianVal(feeStats->GetMaxConfirms(), SUFFICIENT_FEETXS, successThreshold, true, nBestSeenHeight); - if (medMax > 0 && (estimate == -1 || medMax < estimate)) + double medMax = feeStats->EstimateMedianVal(feeStats->GetMaxConfirms(), SUFFICIENT_FEETXS, successThreshold, true, nBestSeenHeight, &tempResult); + if (medMax > 0 && (estimate == -1 || medMax < estimate)) { estimate = medMax; + if (result) *result = tempResult; + } } if (confTarget > shortStats->GetMaxConfirms()) { - double shortMax = shortStats->EstimateMedianVal(shortStats->GetMaxConfirms(), SUFFICIENT_TXS_SHORT, successThreshold, true, nBestSeenHeight); - if (shortMax > 0 && (estimate == -1 || shortMax < estimate)) + double shortMax = shortStats->EstimateMedianVal(shortStats->GetMaxConfirms(), SUFFICIENT_TXS_SHORT, successThreshold, true, nBestSeenHeight, &tempResult); + if (shortMax > 0 && (estimate == -1 || shortMax < estimate)) { estimate = shortMax; + if (result) *result = tempResult; + } } } } @@ -732,16 +757,18 @@ double CBlockPolicyEstimator::estimateCombinedFee(unsigned int confTarget, doubl /** Ensure that for a conservative estimate, the DOUBLE_SUCCESS_PCT is also met * at 2 * target for any longer time horizons. */ -double CBlockPolicyEstimator::estimateConservativeFee(unsigned int doubleTarget) const +double CBlockPolicyEstimator::estimateConservativeFee(unsigned int doubleTarget, EstimationResult *result) const { double estimate = -1; + EstimationResult tempResult; if (doubleTarget <= shortStats->GetMaxConfirms()) { - estimate = feeStats->EstimateMedianVal(doubleTarget, SUFFICIENT_FEETXS, DOUBLE_SUCCESS_PCT, true, nBestSeenHeight); + estimate = feeStats->EstimateMedianVal(doubleTarget, SUFFICIENT_FEETXS, DOUBLE_SUCCESS_PCT, true, nBestSeenHeight, result); } if (doubleTarget <= feeStats->GetMaxConfirms()) { - double longEstimate = longStats->EstimateMedianVal(doubleTarget, SUFFICIENT_FEETXS, DOUBLE_SUCCESS_PCT, true, nBestSeenHeight); + double longEstimate = longStats->EstimateMedianVal(doubleTarget, SUFFICIENT_FEETXS, DOUBLE_SUCCESS_PCT, true, nBestSeenHeight, &tempResult); if (longEstimate > estimate) { estimate = longEstimate; + if (result) *result = tempResult; } } return estimate; @@ -754,12 +781,15 @@ double CBlockPolicyEstimator::estimateConservativeFee(unsigned int doubleTarget) * estimates, however, required the 95% threshold at 2 * target be met for any * longer time horizons also. */ -CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool, bool conservative) const +CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation *feeCalc, const CTxMemPool& pool, bool conservative) const { - if (answerFoundAtTarget) - *answerFoundAtTarget = confTarget; + if (feeCalc) { + feeCalc->desiredTarget = confTarget; + feeCalc->returnedTarget = confTarget; + } double median = -1; + EstimationResult tempResult; { LOCK(cs_feeEstimator); @@ -780,7 +810,6 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun } assert(confTarget > 0); //estimateCombinedFee and estimateConservativeFee take unsigned ints - /** true is passed to estimateCombined fee for target/2 and target so * that we check the max confirms for shorter time horizons as well. * This is necessary to preserve monotonically increasing estimates. @@ -791,32 +820,49 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun * the purpose of conservative estimates is not to let short term * fluctuations lower our estimates by too much. */ - double halfEst = estimateCombinedFee(confTarget/2, HALF_SUCCESS_PCT, true); - double actualEst = estimateCombinedFee(confTarget, SUCCESS_PCT, true); - double doubleEst = estimateCombinedFee(2 * confTarget, DOUBLE_SUCCESS_PCT, !conservative); + double halfEst = estimateCombinedFee(confTarget/2, HALF_SUCCESS_PCT, true, &tempResult); + if (feeCalc) { + feeCalc->est = tempResult; + feeCalc->reason = FeeReason::HALF_ESTIMATE; + } median = halfEst; + double actualEst = estimateCombinedFee(confTarget, SUCCESS_PCT, true, &tempResult); if (actualEst > median) { median = actualEst; + if (feeCalc) { + feeCalc->est = tempResult; + feeCalc->reason = FeeReason::FULL_ESTIMATE; + } } + double doubleEst = estimateCombinedFee(2 * confTarget, DOUBLE_SUCCESS_PCT, !conservative, &tempResult); if (doubleEst > median) { median = doubleEst; + if (feeCalc) { + feeCalc->est = tempResult; + feeCalc->reason = FeeReason::DOUBLE_ESTIMATE; + } } if (conservative || median == -1) { - double consEst = estimateConservativeFee(2 * confTarget); + double consEst = estimateConservativeFee(2 * confTarget, &tempResult); if (consEst > median) { median = consEst; + if (feeCalc) { + feeCalc->est = tempResult; + feeCalc->reason = FeeReason::CONSERVATIVE; + } } } } // Must unlock cs_feeEstimator before taking mempool locks - if (answerFoundAtTarget) - *answerFoundAtTarget = confTarget; + if (feeCalc) feeCalc->returnedTarget = confTarget; // If mempool is limiting txs , return at least the min feerate from the mempool CAmount minPoolFee = pool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); - if (minPoolFee > 0 && minPoolFee > median) + if (minPoolFee > 0 && minPoolFee > median) { + if (feeCalc) feeCalc->reason = FeeReason::MEMPOOL_MIN; return CFeeRate(minPoolFee); + } if (median < 0) return CFeeRate(0); diff --git a/src/policy/fees.h b/src/policy/fees.h index e99fec2c39..7125a74f03 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -74,6 +74,22 @@ enum FeeEstimateHorizon { LONG_HALFLIFE = 2 }; +/* Enumeration of reason for returned fee estimate */ +enum class FeeReason { + NONE, + HALF_ESTIMATE, + FULL_ESTIMATE, + DOUBLE_ESTIMATE, + CONSERVATIVE, + MEMPOOL_MIN, + PAYTXFEE, + FALLBACK, + REQUIRED, + MAXTXFEE, +}; + +std::string StringForFeeReason(FeeReason reason); + /* Used to return detailed information about a feerate bucket */ struct EstimatorBucket { @@ -90,8 +106,16 @@ struct EstimationResult { EstimatorBucket pass; EstimatorBucket fail; - double decay; - unsigned int scale; + double decay = 0; + unsigned int scale = 0; +}; + +struct FeeCalculation +{ + EstimationResult est; + FeeReason reason = FeeReason::NONE; + int desiredTarget = 0; + int returnedTarget = 0; }; /** @@ -173,7 +197,7 @@ public: * the closest target where one can be given. 'conservative' estimates are * valid over longer time horizons also. */ - CFeeRate estimateSmartFee(int confTarget, int *answerFoundAtTarget, const CTxMemPool& pool, bool conservative = true) const; + CFeeRate estimateSmartFee(int confTarget, FeeCalculation *feeCalc, const CTxMemPool& pool, bool conservative = true) const; /** Return a specific fee estimate calculation with a given success * threshold and time horizon, and optionally return detailed data about @@ -223,9 +247,9 @@ private: bool processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry); /** Helper for estimateSmartFee */ - double estimateCombinedFee(unsigned int confTarget, double successThreshold, bool checkShorterHorizon) const; + double estimateCombinedFee(unsigned int confTarget, double successThreshold, bool checkShorterHorizon, EstimationResult *result) const; /** Helper for estimateSmartFee */ - double estimateConservativeFee(unsigned int doubleTarget) const; + double estimateConservativeFee(unsigned int doubleTarget, EstimationResult *result) const; /** Number of blocks of data recorded while fee estimates have been running */ unsigned int BlockSpan() const; /** Number of blocks of recorded fee estimate data represented in saved data file */ diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index dfcbaf3d9b..c52cb43f35 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -510,7 +510,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) nBytes -= 34; // Fee - nPayFee = CWallet::GetMinimumFee(nBytes, nTxConfirmTarget, ::mempool, ::feeEstimator); + nPayFee = CWallet::GetMinimumFee(nBytes, coinControl->nConfirmTarget, ::mempool, ::feeEstimator); if (nPayAmount > 0) { @@ -585,7 +585,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) if (payTxFee.GetFeePerK() > 0) dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), payTxFee.GetFeePerK()) / 1000; else { - dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), ::feeEstimator.estimateSmartFee(nTxConfirmTarget, NULL, ::mempool).GetFeePerK()) / 1000; + dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), ::feeEstimator.estimateSmartFee(coinControl->nConfirmTarget, NULL, ::mempool).GetFeePerK()) / 1000; } QString toolTip4 = tr("Can vary +/- %1 satoshi(s) per input.").arg(dFeeVary); diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index cda33076f8..12d2d0f31c 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -651,8 +651,8 @@ void SendCoinsDialog::updateSmartFeeLabel() return; int nBlocksToConfirm = ui->sliderSmartFee->maximum() - ui->sliderSmartFee->value() + 2; - int estimateFoundAtBlocks = nBlocksToConfirm; - CFeeRate feeRate = ::feeEstimator.estimateSmartFee(nBlocksToConfirm, &estimateFoundAtBlocks, ::mempool); + FeeCalculation feeCalc; + CFeeRate feeRate = ::feeEstimator.estimateSmartFee(nBlocksToConfirm, &feeCalc, ::mempool); if (feeRate <= CFeeRate(0)) // not enough data => minfee { ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), @@ -670,7 +670,7 @@ void SendCoinsDialog::updateSmartFeeLabel() ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), std::max(feeRate.GetFeePerK(), CWallet::GetRequiredFee(1000))) + "/kB"); ui->labelSmartFee2->hide(); - ui->labelFeeEstimation->setText(tr("Estimated to begin confirmation within %n block(s).", "", estimateFoundAtBlocks)); + ui->labelFeeEstimation->setText(tr("Estimated to begin confirmation within %n block(s).", "", feeCalc.returnedTarget)); ui->fallbackFeeWarningLabel->setVisible(false); } @@ -822,6 +822,12 @@ void SendCoinsDialog::coinControlUpdateLabels() // set pay amounts CoinControlDialog::payAmounts.clear(); CoinControlDialog::fSubtractFeeFromAmount = false; + if (ui->radioSmartFee->isChecked()) { + CoinControlDialog::coinControl->nConfirmTarget = ui->sliderSmartFee->maximum() - ui->sliderSmartFee->value() + 2; + } else { + CoinControlDialog::coinControl->nConfirmTarget = model->getDefaultConfirmTarget(); + } + for(int i = 0; i < ui->entries->count(); ++i) { SendCoinsEntry *entry = qobject_cast<SendCoinsEntry*>(ui->entries->itemAt(i)->widget()); diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index dada689731..26dec3c610 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -33,8 +33,6 @@ static const CRPCCommand vRPCCommands[] = void RPCNestedTests::rpcNestedTests() { - UniValue jsonRPCError; - // do some test setup // could be moved to a more generic place when we add more tests on QT level const CChainParams& chainparams = Params(); diff --git a/src/random.cpp b/src/random.cpp index 7916643263..67efc7d945 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -72,10 +72,16 @@ static bool rdrand_supported = false; static constexpr uint32_t CPUID_F1_ECX_RDRAND = 0x40000000; static void RDRandInit() { - //! When calling cpuid function #1, ecx register will have this set if RDRAND is available. + uint32_t eax, ecx, edx; +#if defined(__i386__) && ( defined(__PIC__) || defined(__PIE__)) // Avoid clobbering ebx, as that is used for PIC on x86. - uint32_t eax, tmp, ecx, edx; + uint32_t tmp; __asm__ ("mov %%ebx, %1; cpuid; mov %1, %%ebx": "=a"(eax), "=g"(tmp), "=c"(ecx), "=d"(edx) : "a"(1)); +#else + uint32_t ebx; + __asm__ ("cpuid": "=a"(eax), "=b"(ebx), "=c"(ecx), "=d"(edx) : "a"(1)); +#endif + //! When calling cpuid function #1, ecx register will have this set if RDRAND is available. if (ecx & CPUID_F1_ECX_RDRAND) { LogPrintf("Using RdRand as entropy source\n"); rdrand_supported = true; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 9af29652cf..3b212dc0e4 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -867,10 +867,10 @@ UniValue estimatesmartfee(const JSONRPCRequest& request) } UniValue result(UniValue::VOBJ); - int answerFound; - CFeeRate feeRate = ::feeEstimator.estimateSmartFee(nBlocks, &answerFound, ::mempool, conservative); + FeeCalculation feeCalc; + CFeeRate feeRate = ::feeEstimator.estimateSmartFee(nBlocks, &feeCalc, ::mempool, conservative); result.push_back(Pair("feerate", feeRate == CFeeRate(0) ? -1.0 : ValueFromAmount(feeRate.GetFeePerK()))); - result.push_back(Pair("blocks", answerFound)); + result.push_back(Pair("blocks", feeCalc.returnedTarget)); return result; } diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 1883005163..eb0722cd81 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -376,8 +376,10 @@ static std::pair<bool,std::string> ReadBinaryFile(const fs::path &filename, size while ((n=fread(buffer, 1, sizeof(buffer), f)) > 0) { // Check for reading errors so we don't return any data if we couldn't // read the entire file (or up to maxsize) - if (ferror(f)) + if (ferror(f)) { + fclose(f); return std::make_pair(false,""); + } retval.append(buffer, buffer+n); if (retval.size() > maxsize) break; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index afafc695f4..304300239c 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1050,9 +1050,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends for (const CTransaction& tx : txn) { for (const CTxIn& txin : tx.vin) { if (exists(txin.prevout.hash)) continue; - if (!mapNextTx.count(txin.prevout)) { - pvNoSpendsRemaining->push_back(txin.prevout); - } + pvNoSpendsRemaining->push_back(txin.prevout); } } } diff --git a/src/util.h b/src/util.h index 76f8b32a5e..f40db84a10 100644 --- a/src/util.h +++ b/src/util.h @@ -187,62 +187,63 @@ public: void ParseParameters(int argc, const char*const argv[]); void ReadConfigFile(const std::string& confPath); std::vector<std::string> GetArgs(const std::string& strArg); -/** - * Return true if the given argument has been manually set - * - * @param strArg Argument to get (e.g. "-foo") - * @return true if the argument has been set - */ -bool IsArgSet(const std::string& strArg); - -/** - * Return string argument or default value - * - * @param strArg Argument to get (e.g. "-foo") - * @param default (e.g. "1") - * @return command-line argument or default value - */ -std::string GetArg(const std::string& strArg, const std::string& strDefault); - -/** - * Return integer argument or default value - * - * @param strArg Argument to get (e.g. "-foo") - * @param default (e.g. 1) - * @return command-line argument (0 if invalid number) or default value - */ -int64_t GetArg(const std::string& strArg, int64_t nDefault); - -/** - * Return boolean argument or default value - * - * @param strArg Argument to get (e.g. "-foo") - * @param default (true or false) - * @return command-line argument or default value - */ -bool GetBoolArg(const std::string& strArg, bool fDefault); - -/** - * Set an argument if it doesn't already have a value - * - * @param strArg Argument to set (e.g. "-foo") - * @param strValue Value (e.g. "1") - * @return true if argument gets set, false if it already had a value - */ -bool SoftSetArg(const std::string& strArg, const std::string& strValue); - -/** - * Set a boolean argument if it doesn't already have a value - * - * @param strArg Argument to set (e.g. "-foo") - * @param fValue Value (e.g. false) - * @return true if argument gets set, false if it already had a value - */ -bool SoftSetBoolArg(const std::string& strArg, bool fValue); -// Forces an arg setting. Called by SoftSetArg() if the arg hasn't already -// been set. Also called directly in testing. -void ForceSetArg(const std::string& strArg, const std::string& strValue); + /** + * Return true if the given argument has been manually set + * + * @param strArg Argument to get (e.g. "-foo") + * @return true if the argument has been set + */ + bool IsArgSet(const std::string& strArg); + + /** + * Return string argument or default value + * + * @param strArg Argument to get (e.g. "-foo") + * @param default (e.g. "1") + * @return command-line argument or default value + */ + std::string GetArg(const std::string& strArg, const std::string& strDefault); + + /** + * Return integer argument or default value + * + * @param strArg Argument to get (e.g. "-foo") + * @param default (e.g. 1) + * @return command-line argument (0 if invalid number) or default value + */ + int64_t GetArg(const std::string& strArg, int64_t nDefault); + + /** + * Return boolean argument or default value + * + * @param strArg Argument to get (e.g. "-foo") + * @param default (true or false) + * @return command-line argument or default value + */ + bool GetBoolArg(const std::string& strArg, bool fDefault); + + /** + * Set an argument if it doesn't already have a value + * + * @param strArg Argument to set (e.g. "-foo") + * @param strValue Value (e.g. "1") + * @return true if argument gets set, false if it already had a value + */ + bool SoftSetArg(const std::string& strArg, const std::string& strValue); + + /** + * Set a boolean argument if it doesn't already have a value + * + * @param strArg Argument to set (e.g. "-foo") + * @param fValue Value (e.g. false) + * @return true if argument gets set, false if it already had a value + */ + bool SoftSetBoolArg(const std::string& strArg, bool fValue); + + // Forces an arg setting. Called by SoftSetArg() if the arg hasn't already + // been set. Also called directly in testing. + void ForceSetArg(const std::string& strArg, const std::string& strValue); }; extern ArgsManager gArgs; diff --git a/src/validation.cpp b/src/validation.cpp index bef17337b9..cba3b9e4ad 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -430,8 +430,9 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool return state.DoS(0, false, REJECT_NONSTANDARD, "non-final"); // is it already in the memory pool? - if (pool.exists(hash)) - return state.Invalid(false, REJECT_ALREADY_KNOWN, "txn-already-in-mempool"); + if (pool.exists(hash)) { + return state.Invalid(false, REJECT_DUPLICATE, "txn-already-in-mempool"); + } // Check for conflicts with in-memory transactions std::set<uint256> setConflicts; @@ -469,8 +470,9 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool } } } - if (fReplacementOptOut) - return state.Invalid(false, REJECT_CONFLICT, "txn-mempool-conflict"); + if (fReplacementOptOut) { + return state.Invalid(false, REJECT_DUPLICATE, "txn-mempool-conflict"); + } setConflicts.insert(ptxConflicting->GetHash()); } @@ -497,7 +499,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool if (!had_coin_in_cache) { coins_to_uncache.push_back(outpoint); } - return state.Invalid(false, REJECT_ALREADY_KNOWN, "txn-already-known"); + return state.Invalid(false, REJECT_DUPLICATE, "txn-already-known"); } } @@ -1123,7 +1125,8 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund txundo.vprevout.reserve(tx.vin.size()); for (const CTxIn &txin : tx.vin) { txundo.vprevout.emplace_back(); - inputs.SpendCoin(txin.prevout, &txundo.vprevout.back()); + bool is_spent = inputs.SpendCoin(txin.prevout, &txundo.vprevout.back()); + assert(is_spent); } } // add outputs @@ -1368,8 +1371,8 @@ static DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* if (!tx.vout[o].scriptPubKey.IsUnspendable()) { COutPoint out(hash, o); Coin coin; - view.SpendCoin(out, &coin); - if (tx.vout[o] != coin.out) { + bool is_spent = view.SpendCoin(out, &coin); + if (!is_spent || tx.vout[o] != coin.out) { fClean = false; // transaction output mismatch } } diff --git a/src/validation.h b/src/validation.h index 15e19bc511..82df4cb170 100644 --- a/src/validation.h +++ b/src/validation.h @@ -469,10 +469,6 @@ int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Para static const unsigned int REJECT_INTERNAL = 0x100; /** Too high fee. Can not be triggered by P2P transactions */ static const unsigned int REJECT_HIGHFEE = 0x100; -/** Transaction is already known (either in mempool or blockchain) */ -static const unsigned int REJECT_ALREADY_KNOWN = 0x101; -/** Transaction conflicts with a transaction already known */ -static const unsigned int REJECT_CONFLICT = 0x102; /** Get block file info entry for one block file */ CBlockFileInfo* GetBlockFileInfo(size_t n); diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index 836c15b82c..6fa685628f 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -274,7 +274,6 @@ bool CCryptoKeyStore::GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) co // Check for watch-only pubkeys return CBasicKeyStore::GetPubKey(address, vchPubKeyOut); } - return false; } bool CCryptoKeyStore::EncryptKeys(CKeyingMaterial& vMasterKeyIn) diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 46ef87b7b1..6a9e6cf9ff 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -165,7 +165,7 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, int newConf nNewFee = totalFee; nNewFeeRate = CFeeRate(totalFee, maxNewTxSize); } else { - nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool, ::feeEstimator, ignoreGlobalPayTxFee); + nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool, ::feeEstimator, nullptr, ignoreGlobalPayTxFee); nNewFeeRate = CFeeRate(nNewFee, maxNewTxSize); // New fee rate must be at least old rate + minimum incremental relay rate diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 2e4105a569..5bbb5088e2 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1064,7 +1064,6 @@ public: bool operator()(const CNoDestination &dest) const { return false; } bool operator()(const CKeyID &keyID) { - CPubKey pubkey; if (pwallet) { CScript basescript = GetScriptForDestination(keyID); isminetype typ; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index af75906ae7..34178e4625 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -205,7 +205,6 @@ bool CWallet::AddCryptedKey(const CPubKey &vchPubKey, vchCryptedSecret, mapKeyMetadata[vchPubKey.GetID()]); } - return false; } bool CWallet::LoadKeyMetadata(const CTxDestination& keyID, const CKeyMetadata &meta) @@ -2534,7 +2533,8 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT assert(txNew.nLockTime <= (unsigned int)chainActive.Height()); assert(txNew.nLockTime < LOCKTIME_THRESHOLD); - + FeeCalculation feeCalc; + unsigned int nBytes; { std::set<CInputCoin> setCoins; LOCK2(cs_main, cs_wallet); @@ -2684,9 +2684,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT return false; } - unsigned int nBytes = GetVirtualTransactionSize(txNew); - - CTransaction txNewConst(txNew); + nBytes = GetVirtualTransactionSize(txNew); // Remove scriptSigs to eliminate the fee calculation dummy signatures for (auto& vin : txNew.vin) { @@ -2699,7 +2697,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT if (coinControl && coinControl->nConfirmTarget > 0) currentConfirmationTarget = coinControl->nConfirmTarget; - CAmount nFeeNeeded = GetMinimumFee(nBytes, currentConfirmationTarget, ::mempool, ::feeEstimator); + CAmount nFeeNeeded = GetMinimumFee(nBytes, currentConfirmationTarget, ::mempool, ::feeEstimator, &feeCalc); if (coinControl && coinControl->fOverrideFeeRate) nFeeNeeded = coinControl->nFeeRate.GetFee(nBytes); @@ -2796,6 +2794,15 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT return false; } } + + LogPrintf("Fee Calculation: Fee:%d Bytes:%u Tgt:%d (requested %d) Reason:\"%s\" Decay %.5f: Estimation: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n", + nFeeRet, nBytes, feeCalc.returnedTarget, feeCalc.desiredTarget, StringForFeeReason(feeCalc.reason), feeCalc.est.decay, + feeCalc.est.pass.start, feeCalc.est.pass.end, + 100 * feeCalc.est.pass.withinTarget / (feeCalc.est.pass.totalConfirmed + feeCalc.est.pass.inMempool + feeCalc.est.pass.leftMempool), + feeCalc.est.pass.withinTarget, feeCalc.est.pass.totalConfirmed, feeCalc.est.pass.inMempool, feeCalc.est.pass.leftMempool, + feeCalc.est.fail.start, feeCalc.est.fail.end, + 100 * feeCalc.est.fail.withinTarget / (feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool), + feeCalc.est.fail.withinTarget, feeCalc.est.fail.totalConfirmed, feeCalc.est.fail.inMempool, feeCalc.est.fail.leftMempool); return true; } @@ -2871,23 +2878,32 @@ CAmount CWallet::GetRequiredFee(unsigned int nTxBytes) return std::max(minTxFee.GetFee(nTxBytes), ::minRelayTxFee.GetFee(nTxBytes)); } -CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, bool ignoreGlobalPayTxFee) +CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc, bool ignoreGlobalPayTxFee) { // payTxFee is the user-set global for desired feerate CAmount nFeeNeeded = payTxFee.GetFee(nTxBytes); // User didn't set: use -txconfirmtarget to estimate... if (nFeeNeeded == 0 || ignoreGlobalPayTxFee) { - int estimateFoundTarget = nConfirmTarget; - nFeeNeeded = estimator.estimateSmartFee(nConfirmTarget, &estimateFoundTarget, pool).GetFee(nTxBytes); + nFeeNeeded = estimator.estimateSmartFee(nConfirmTarget, feeCalc, pool, true).GetFee(nTxBytes); // ... unless we don't have enough mempool data for estimatefee, then use fallbackFee - if (nFeeNeeded == 0) + if (nFeeNeeded == 0) { nFeeNeeded = fallbackFee.GetFee(nTxBytes); + if (feeCalc) feeCalc->reason = FeeReason::FALLBACK; + } + } else { + if (feeCalc) feeCalc->reason = FeeReason::PAYTXFEE; } // prevent user from paying a fee below minRelayTxFee or minTxFee - nFeeNeeded = std::max(nFeeNeeded, GetRequiredFee(nTxBytes)); + CAmount requiredFee = GetRequiredFee(nTxBytes); + if (requiredFee > nFeeNeeded) { + nFeeNeeded = requiredFee; + if (feeCalc) feeCalc->reason = FeeReason::REQUIRED; + } // But always obey the maximum - if (nFeeNeeded > maxTxFee) + if (nFeeNeeded > maxTxFee) { nFeeNeeded = maxTxFee; + if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE; + } return nFeeNeeded; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 6c6eb69180..ad606b8535 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -77,6 +77,7 @@ class CScheduler; class CTxMemPool; class CBlockPolicyEstimator; class CWalletTx; +struct FeeCalculation; /** (client) version numbers for particular wallet features */ enum WalletFeature @@ -959,7 +960,7 @@ public: * Estimate the minimum fee considering user set parameters * and the required fee */ - static CAmount GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, bool ignoreGlobalPayTxFee = false); + static CAmount GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc = nullptr, bool ignoreGlobalPayTxFee = false); /** * Return the minimum required fee taking into account the * floating relay fee and user set minimum transaction fee diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index eca6706c06..8321719b56 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -76,8 +76,6 @@ bool CWalletDB::WriteCryptedKey(const CPubKey& vchPubKey, const std::vector<unsigned char>& vchCryptedSecret, const CKeyMetadata &keyMeta) { - const bool fEraseUnencryptedKey = true; - if (!WriteIC(std::make_pair(std::string("keymeta"), vchPubKey), keyMeta)) { return false; } @@ -85,12 +83,8 @@ bool CWalletDB::WriteCryptedKey(const CPubKey& vchPubKey, if (!WriteIC(std::make_pair(std::string("ckey"), vchPubKey), vchCryptedSecret, false)) { return false; } - if (fEraseUnencryptedKey) - { - EraseIC(std::make_pair(std::string("key"), vchPubKey)); - EraseIC(std::make_pair(std::string("wkey"), vchPubKey)); - } - + EraseIC(std::make_pair(std::string("key"), vchPubKey)); + EraseIC(std::make_pair(std::string("wkey"), vchPubKey)); return true; } diff --git a/test/functional/blockchain.py b/test/functional/blockchain.py index 6aef6d4489..e205c6400c 100755 --- a/test/functional/blockchain.py +++ b/test/functional/blockchain.py @@ -18,13 +18,16 @@ Tests correspond to code in rpc/blockchain.cpp. """ from decimal import Decimal +import subprocess from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_raises, assert_raises_jsonrpc, assert_is_hex_string, assert_is_hash_string, + bitcoind_processes, ) @@ -34,6 +37,7 @@ class BlockchainTest(BitcoinTestFramework): super().__init__() self.setup_clean_chain = False self.num_nodes = 1 + self.extra_args = [['-stopatheight=207']] def run_test(self): self._test_getchaintxstats() @@ -41,7 +45,8 @@ class BlockchainTest(BitcoinTestFramework): self._test_getblockheader() self._test_getdifficulty() self._test_getnetworkhashps() - self.nodes[0].verifychain(4, 0) + self._test_stopatheight() + assert self.nodes[0].verifychain(4, 0) def _test_getchaintxstats(self): chaintxstats = self.nodes[0].getchaintxstats(1) @@ -129,5 +134,18 @@ class BlockchainTest(BitcoinTestFramework): # This should be 2 hashes every 10 minutes or 1/300 assert abs(hashes_per_second * 300 - 1) < 0.0001 + def _test_stopatheight(self): + assert_equal(self.nodes[0].getblockcount(), 200) + self.nodes[0].generate(6) + assert_equal(self.nodes[0].getblockcount(), 206) + self.log.debug('Node should not stop at this height') + assert_raises(subprocess.TimeoutExpired, lambda: bitcoind_processes[0].wait(timeout=3)) + self.nodes[0].generate(1) + self.log.debug('Node should stop at this height...') + bitcoind_processes[0].wait(timeout=3) + self.nodes[0] = self.start_node(0, self.options.tmpdir) + assert_equal(self.nodes[0].getblockcount(), 207) + + if __name__ == '__main__': BlockchainTest().main() diff --git a/test/functional/multi_rpc.py b/test/functional/multi_rpc.py index 6ff91a960b..a30e15ace9 100755 --- a/test/functional/multi_rpc.py +++ b/test/functional/multi_rpc.py @@ -16,16 +16,21 @@ class HTTPBasicsTest (BitcoinTestFramework): def __init__(self): super().__init__() self.setup_clean_chain = False - self.num_nodes = 1 + self.num_nodes = 2 def setup_chain(self): super().setup_chain() #Append rpcauth to bitcoin.conf before initialization rpcauth = "rpcauth=rt:93648e835a54c573682c2eb19f882535$7681e9c5b74bdd85e78166031d2058e1069b3ed7ed967c93fc63abba06f31144" rpcauth2 = "rpcauth=rt2:f8607b1a88861fac29dfccf9b52ff9f$ff36a0c23c8c62b4846112e50fa888416e94c17bfd4c42f88fd8f55ec6a3137e" + rpcuser = "rpcuser=rpcuser💻" + rpcpassword = "rpcpassword=rpcpassword🔑" with open(os.path.join(self.options.tmpdir+"/node0", "bitcoin.conf"), 'a', encoding='utf8') as f: f.write(rpcauth+"\n") f.write(rpcauth2+"\n") + with open(os.path.join(self.options.tmpdir+"/node1", "bitcoin.conf"), 'a', encoding='utf8') as f: + f.write(rpcuser+"\n") + f.write(rpcpassword+"\n") def run_test(self): @@ -50,7 +55,7 @@ class HTTPBasicsTest (BitcoinTestFramework): conn.connect() conn.request('POST', '/', '{"method": "getbestblockhash"}', headers) resp = conn.getresponse() - assert_equal(resp.status==401, False) + assert_equal(resp.status, 200) conn.close() #Use new authpair to confirm both work @@ -60,7 +65,7 @@ class HTTPBasicsTest (BitcoinTestFramework): conn.connect() conn.request('POST', '/', '{"method": "getbestblockhash"}', headers) resp = conn.getresponse() - assert_equal(resp.status==401, False) + assert_equal(resp.status, 200) conn.close() #Wrong login name with rt's password @@ -71,7 +76,7 @@ class HTTPBasicsTest (BitcoinTestFramework): conn.connect() conn.request('POST', '/', '{"method": "getbestblockhash"}', headers) resp = conn.getresponse() - assert_equal(resp.status==401, True) + assert_equal(resp.status, 401) conn.close() #Wrong password for rt @@ -82,7 +87,7 @@ class HTTPBasicsTest (BitcoinTestFramework): conn.connect() conn.request('POST', '/', '{"method": "getbestblockhash"}', headers) resp = conn.getresponse() - assert_equal(resp.status==401, True) + assert_equal(resp.status, 401) conn.close() #Correct for rt2 @@ -93,7 +98,7 @@ class HTTPBasicsTest (BitcoinTestFramework): conn.connect() conn.request('POST', '/', '{"method": "getbestblockhash"}', headers) resp = conn.getresponse() - assert_equal(resp.status==401, False) + assert_equal(resp.status, 200) conn.close() #Wrong password for rt2 @@ -104,7 +109,46 @@ class HTTPBasicsTest (BitcoinTestFramework): conn.connect() conn.request('POST', '/', '{"method": "getbestblockhash"}', headers) resp = conn.getresponse() - assert_equal(resp.status==401, True) + assert_equal(resp.status, 401) + conn.close() + + ############################################################### + # Check correctness of the rpcuser/rpcpassword config options # + ############################################################### + url = urllib.parse.urlparse(self.nodes[1].url) + + # rpcuser and rpcpassword authpair + rpcuserauthpair = "rpcuser💻:rpcpassword🔑" + + headers = {"Authorization": "Basic " + str_to_b64str(rpcuserauthpair)} + + conn = http.client.HTTPConnection(url.hostname, url.port) + conn.connect() + conn.request('POST', '/', '{"method": "getbestblockhash"}', headers) + resp = conn.getresponse() + assert_equal(resp.status, 200) + conn.close() + + #Wrong login name with rpcuser's password + rpcuserauthpair = "rpcuserwrong:rpcpassword" + headers = {"Authorization": "Basic " + str_to_b64str(rpcuserauthpair)} + + conn = http.client.HTTPConnection(url.hostname, url.port) + conn.connect() + conn.request('POST', '/', '{"method": "getbestblockhash"}', headers) + resp = conn.getresponse() + assert_equal(resp.status, 401) + conn.close() + + #Wrong password for rpcuser + rpcuserauthpair = "rpcuser:rpcpasswordwrong" + headers = {"Authorization": "Basic " + str_to_b64str(rpcuserauthpair)} + + conn = http.client.HTTPConnection(url.hostname, url.port) + conn.connect() + conn.request('POST', '/', '{"method": "getbestblockhash"}', headers) + resp = conn.getresponse() + assert_equal(resp.status, 401) conn.close() diff --git a/test/functional/p2p-segwit.py b/test/functional/p2p-segwit.py index dbc61d21fc..63dfbb8ae6 100755 --- a/test/functional/p2p-segwit.py +++ b/test/functional/p2p-segwit.py @@ -1486,7 +1486,7 @@ class SegWitTest(BitcoinTestFramework): # nodes would have stored, this requires special handling. # To enable this test, pass --oldbinary=<path-to-pre-segwit-bitcoind> to # the test. - def test_upgrade_after_activation(self, node, node_id): + def test_upgrade_after_activation(self, node_id): self.log.info("Testing software upgrade after softfork activation") assert(node_id != 0) # node0 is assumed to be a segwit-active bitcoind @@ -1502,14 +1502,14 @@ class SegWitTest(BitcoinTestFramework): sync_blocks(self.nodes) # Make sure that this peer thinks segwit has activated. - assert(get_bip9_status(node, 'segwit')['status'] == "active") + assert(get_bip9_status(self.nodes[node_id], 'segwit')['status'] == "active") # Make sure this peers blocks match those of node0. - height = node.getblockcount() + height = self.nodes[node_id].getblockcount() while height >= 0: - block_hash = node.getblockhash(height) + block_hash = self.nodes[node_id].getblockhash(height) assert_equal(block_hash, self.nodes[0].getblockhash(height)) - assert_equal(self.nodes[0].getblock(block_hash), node.getblock(block_hash)) + assert_equal(self.nodes[0].getblock(block_hash), self.nodes[node_id].getblock(block_hash)) height -= 1 @@ -1944,7 +1944,7 @@ class SegWitTest(BitcoinTestFramework): self.test_signature_version_1() self.test_non_standard_witness() sync_blocks(self.nodes) - self.test_upgrade_after_activation(self.nodes[2], 2) + self.test_upgrade_after_activation(node_id=2) self.test_witness_sigops() diff --git a/test/functional/pruning.py b/test/functional/pruning.py index 4c3501ad77..37e9d29738 100755 --- a/test/functional/pruning.py +++ b/test/functional/pruning.py @@ -315,7 +315,7 @@ class PruneTest(BitcoinTestFramework): # check that the pruning node's wallet is still in good shape self.log.info("Stop and start pruning node to trigger wallet rescan") self.stop_node(2) - self.start_node(2, self.options.tmpdir, ["-prune=550"]) + self.nodes[2] = self.start_node(2, self.options.tmpdir, ["-prune=550"]) self.log.info("Success") # check that wallet loads loads successfully when restarting a pruned node after IBD. @@ -325,7 +325,7 @@ class PruneTest(BitcoinTestFramework): nds = [self.nodes[0], self.nodes[5]] sync_blocks(nds, wait=5, timeout=300) self.stop_node(5) #stop and start to trigger rescan - self.start_node(5, self.options.tmpdir, ["-prune=550"]) + self.nodes[5] = self.start_node(5, self.options.tmpdir, ["-prune=550"]) self.log.info("Success") def run_test(self): diff --git a/test/functional/rpcbind_test.py b/test/functional/rpcbind_test.py index 5336cf2ec8..198599010e 100755 --- a/test/functional/rpcbind_test.py +++ b/test/functional/rpcbind_test.py @@ -49,7 +49,7 @@ class RPCBindTest(BitcoinTestFramework): base_args = ['-disablewallet', '-nolisten'] + ['-rpcallowip='+x for x in allow_ips] self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, [base_args]) # connect to node through non-loopback interface - node = get_rpc_proxy(rpc_url(0, "%s:%d" % (rpchost, rpcport)), 0) + node = get_rpc_proxy(rpc_url(get_datadir_path(self.options.tmpdir, 0), 0, "%s:%d" % (rpchost, rpcport)), 0) node.getnetworkinfo() self.stop_nodes() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 67abf35687..c7fd44b81c 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -28,6 +28,7 @@ from .util import ( get_mocktime, get_rpc_proxy, initialize_datadir, + get_datadir_path, log_filename, p2p_port, rpc_url, @@ -300,13 +301,13 @@ class BitcoinTestFramework(object): args.append("-connect=127.0.0.1:" + str(p2p_port(0))) bitcoind_processes[i] = subprocess.Popen(args) self.log.debug("initialize_chain: bitcoind started, waiting for RPC to come up") - wait_for_bitcoind_start(bitcoind_processes[i], rpc_url(i), i) + wait_for_bitcoind_start(bitcoind_processes[i], datadir, i) self.log.debug("initialize_chain: RPC successfully started") self.nodes = [] for i in range(MAX_NODES): try: - self.nodes.append(get_rpc_proxy(rpc_url(i), i)) + self.nodes.append(get_rpc_proxy(rpc_url(get_datadir_path(cachedir, i), i), i)) except: self.log.exception("Error connecting to node %d" % i) sys.exit(1) diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 2b0f32c2b6..d2a609451d 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -181,21 +181,40 @@ def initialize_datadir(dirname, n): datadir = os.path.join(dirname, "node"+str(n)) if not os.path.isdir(datadir): os.makedirs(datadir) - rpc_u, rpc_p = rpc_auth_pair(n) with open(os.path.join(datadir, "bitcoin.conf"), 'w', encoding='utf8') as f: f.write("regtest=1\n") - f.write("rpcuser=" + rpc_u + "\n") - f.write("rpcpassword=" + rpc_p + "\n") f.write("port="+str(p2p_port(n))+"\n") f.write("rpcport="+str(rpc_port(n))+"\n") f.write("listenonion=0\n") return datadir -def rpc_auth_pair(n): - return 'rpcuser💻' + str(n), 'rpcpass🔑' + str(n) - -def rpc_url(i, rpchost=None): - rpc_u, rpc_p = rpc_auth_pair(i) +def get_datadir_path(dirname, n): + return os.path.join(dirname, "node"+str(n)) + +def get_auth_cookie(datadir, n): + user = None + password = None + if os.path.isfile(os.path.join(datadir, "bitcoin.conf")): + with open(os.path.join(datadir, "bitcoin.conf"), 'r') as f: + for line in f: + if line.startswith("rpcuser="): + assert user is None # Ensure that there is only one rpcuser line + user = line.split("=")[1].strip("\n") + if line.startswith("rpcpassword="): + assert password is None # Ensure that there is only one rpcpassword line + password = line.split("=")[1].strip("\n") + if os.path.isfile(os.path.join(datadir, "regtest", ".cookie")): + with open(os.path.join(datadir, "regtest", ".cookie"), 'r') as f: + userpass = f.read() + split_userpass = userpass.split(':') + user = split_userpass[0] + password = split_userpass[1] + if user is None or password is None: + raise ValueError("No RPC credentials") + return user, password + +def rpc_url(datadir, i, rpchost=None): + rpc_u, rpc_p = get_auth_cookie(datadir, i) host = '127.0.0.1' port = rpc_port(i) if rpchost: @@ -206,7 +225,7 @@ def rpc_url(i, rpchost=None): host = rpchost return "http://%s:%s@%s:%d" % (rpc_u, rpc_p, host, int(port)) -def wait_for_bitcoind_start(process, url, i): +def wait_for_bitcoind_start(process, datadir, i, rpchost=None): ''' Wait for bitcoind to start. This means that RPC is accessible and fully initialized. Raise an exception if bitcoind exits during initialization. @@ -215,7 +234,8 @@ def wait_for_bitcoind_start(process, url, i): if process.poll() is not None: raise Exception('bitcoind exited with status %i during initialization' % process.returncode) try: - rpc = get_rpc_proxy(url, i) + # Check if .cookie file to be created + rpc = get_rpc_proxy(rpc_url(datadir, i, rpchost), i) blocks = rpc.getblockcount() break # break out of loop on success except IOError as e: @@ -224,6 +244,9 @@ def wait_for_bitcoind_start(process, url, i): except JSONRPCException as e: # Initialization phase if e.error['code'] != -28: # RPC in warmup? raise # unknown JSON RPC exception + except ValueError as e: # cookie file not found and no rpcuser or rpcassword. bitcoind still starting + if "No RPC credentials" not in str(e): + raise time.sleep(0.25) @@ -239,10 +262,9 @@ def _start_node(i, dirname, extra_args=None, rpchost=None, timewait=None, binary if extra_args is not None: args.extend(extra_args) bitcoind_processes[i] = subprocess.Popen(args, stderr=stderr) logger.debug("initialize_chain: bitcoind started, waiting for RPC to come up") - url = rpc_url(i, rpchost) - wait_for_bitcoind_start(bitcoind_processes[i], url, i) + wait_for_bitcoind_start(bitcoind_processes[i], datadir, i, rpchost) logger.debug("initialize_chain: RPC successfully started") - proxy = get_rpc_proxy(url, i, timeout=timewait) + proxy = get_rpc_proxy(rpc_url(datadir, i, rpchost), i, timeout=timewait) if COVERAGE_DIR: coverage.write_all_rpc_commands(COVERAGE_DIR, proxy) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index c7de31510a..0dca318af8 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -20,6 +20,7 @@ import datetime import os import time import shutil +import signal import sys import subprocess import tempfile @@ -78,7 +79,7 @@ BASE_SCRIPTS= [ 'rawtransactions.py', 'reindex.py', # vv Tests less than 30s vv - "zmq_test.py", + 'zmq_test.py', 'mempool_resurrect_test.py', 'txn_doublespend.py --mineblock', 'txn_clone.py', @@ -390,6 +391,10 @@ class TestHandler: time.sleep(.5) for j in self.jobs: (name, time0, proc, log_out, log_err) = j + if os.getenv('TRAVIS') == 'true' and int(time.time() - time0) > 20 * 60: + # In travis, timeout individual tests after 20 minutes (to stop tests hanging and not + # providing useful output. + proc.send_signal(signal.SIGINT) if proc.poll() is not None: log_out.seek(0), log_err.seek(0) [stdout, stderr] = [l.read().decode('utf-8') for l in (log_out, log_err)] diff --git a/test/functional/zmq_test.py b/test/functional/zmq_test.py index ce39cfefdc..26c946d215 100755 --- a/test/functional/zmq_test.py +++ b/test/functional/zmq_test.py @@ -8,15 +8,15 @@ import os import struct from test_framework.test_framework import BitcoinTestFramework, SkipTest -from test_framework.util import * +from test_framework.util import (assert_equal, + bytes_to_hex_str, + ) class ZMQTest (BitcoinTestFramework): def __init__(self): super().__init__() - self.num_nodes = 4 - - port = 28332 + self.num_nodes = 2 def setup_nodes(self): # Try to import python3-zmq. Skip this test if the import fails. @@ -28,7 +28,7 @@ class ZMQTest (BitcoinTestFramework): # Check that bitcoin has been built with ZMQ enabled config = configparser.ConfigParser() if not self.options.configfile: - self.options.configfile = os.path.dirname(__file__) + "/config.ini" + self.options.configfile = os.path.dirname(__file__) + "/../config.ini" config.read_file(open(self.options.configfile)) if not config["components"].getboolean("ENABLE_ZMQ"): @@ -36,59 +36,66 @@ class ZMQTest (BitcoinTestFramework): self.zmqContext = zmq.Context() self.zmqSubSocket = self.zmqContext.socket(zmq.SUB) + self.zmqSubSocket.set(zmq.RCVTIMEO, 60000) self.zmqSubSocket.setsockopt(zmq.SUBSCRIBE, b"hashblock") self.zmqSubSocket.setsockopt(zmq.SUBSCRIBE, b"hashtx") - self.zmqSubSocket.connect("tcp://127.0.0.1:%i" % self.port) - self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, extra_args=[ - ['-zmqpubhashtx=tcp://127.0.0.1:'+str(self.port), '-zmqpubhashblock=tcp://127.0.0.1:'+str(self.port)], - [], - [], - [] - ]) + ip_address = "tcp://127.0.0.1:28332" + self.zmqSubSocket.connect(ip_address) + extra_args = [['-zmqpubhashtx=%s' % ip_address, '-zmqpubhashblock=%s' % ip_address], []] + self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir, extra_args) def run_test(self): - self.sync_all() + try: + self._zmq_test() + finally: + # Destroy the zmq context + self.log.debug("Destroying zmq context") + self.zmqContext.destroy(linger=None) + def _zmq_test(self): genhashes = self.nodes[0].generate(1) self.sync_all() - self.log.info("listen...") + self.log.info("Wait for tx") msg = self.zmqSubSocket.recv_multipart() topic = msg[0] assert_equal(topic, b"hashtx") body = msg[1] msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, 0) #must be sequence 0 on hashtx + assert_equal(msgSequence, 0) # must be sequence 0 on hashtx + self.log.info("Wait for block") msg = self.zmqSubSocket.recv_multipart() topic = msg[0] body = msg[1] msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, 0) #must be sequence 0 on hashblock + assert_equal(msgSequence, 0) # must be sequence 0 on hashblock blkhash = bytes_to_hex_str(body) - assert_equal(genhashes[0], blkhash) #blockhash from generate must be equal to the hash received over zmq + assert_equal(genhashes[0], blkhash) # blockhash from generate must be equal to the hash received over zmq + self.log.info("Generate 10 blocks (and 10 coinbase txes)") n = 10 genhashes = self.nodes[1].generate(n) self.sync_all() zmqHashes = [] blockcount = 0 - for x in range(0,n*2): + for x in range(n * 2): msg = self.zmqSubSocket.recv_multipart() topic = msg[0] body = msg[1] if topic == b"hashblock": zmqHashes.append(bytes_to_hex_str(body)) msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, blockcount+1) + assert_equal(msgSequence, blockcount + 1) blockcount += 1 - for x in range(0,n): - assert_equal(genhashes[x], zmqHashes[x]) #blockhash from generate must be equal to the hash received over zmq + for x in range(n): + assert_equal(genhashes[x], zmqHashes[x]) # blockhash from generate must be equal to the hash received over zmq - #test tx from a second node + self.log.info("Wait for tx from second node") + # test tx from a second node hashRPC = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1.0) self.sync_all() @@ -96,14 +103,12 @@ class ZMQTest (BitcoinTestFramework): msg = self.zmqSubSocket.recv_multipart() topic = msg[0] body = msg[1] - hashZMQ = "" - if topic == b"hashtx": - hashZMQ = bytes_to_hex_str(body) - msgSequence = struct.unpack('<I', msg[-1])[-1] - assert_equal(msgSequence, blockcount+1) - - assert_equal(hashRPC, hashZMQ) #blockhash from generate must be equal to the hash received over zmq + assert_equal(topic, b"hashtx") + hashZMQ = bytes_to_hex_str(body) + msgSequence = struct.unpack('<I', msg[-1])[-1] + assert_equal(msgSequence, blockcount + 1) + assert_equal(hashRPC, hashZMQ) # txid from sendtoaddress must be equal to the hash received over zmq if __name__ == '__main__': - ZMQTest ().main () + ZMQTest().main() |