diff options
Diffstat (limited to 'src')
48 files changed, 212 insertions, 189 deletions
diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 41f1e5786c..8bdc210997 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -313,13 +313,11 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co // Get credentials std::string strRPCUserColonPass; + bool failedToGetAuthCookie = false; if (gArgs.GetArg("-rpcpassword", "") == "") { // Try fall back to cookie-based authentication if no password is provided if (!GetAuthCookie(&strRPCUserColonPass)) { - throw std::runtime_error(strprintf( - _("Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set. See -rpcpassword and -stdinrpcpass. Configuration file: (%s)"), - GetConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME)).string().c_str())); - + failedToGetAuthCookie = true; } } else { strRPCUserColonPass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", ""); @@ -358,11 +356,21 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co event_base_dispatch(base.get()); - if (response.status == 0) - throw CConnectionFailed(strprintf("couldn't connect to server: %s (code %d)\n(make sure server is running and you are connecting to the correct RPC port)", http_errorstring(response.error), response.error)); - else if (response.status == HTTP_UNAUTHORIZED) - throw std::runtime_error("incorrect rpcuser or rpcpassword (authorization failed)"); - else if (response.status >= 400 && response.status != HTTP_BAD_REQUEST && response.status != HTTP_NOT_FOUND && response.status != HTTP_INTERNAL_SERVER_ERROR) + if (response.status == 0) { + std::string responseErrorMessage; + if (response.error != -1) { + responseErrorMessage = strprintf(" (error code %d - \"%s\")", response.error, http_errorstring(response.error)); + } + throw CConnectionFailed(strprintf("Could not connect to the server %s:%d%s\n\nMake sure the bitcoind server is running and that you are connecting to the correct RPC port.", host, port, responseErrorMessage)); + } else if (response.status == HTTP_UNAUTHORIZED) { + if (failedToGetAuthCookie) { + throw std::runtime_error(strprintf( + _("Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set. See -rpcpassword and -stdinrpcpass. Configuration file: (%s)"), + GetConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME)).string().c_str())); + } else { + throw std::runtime_error("Authorization failed: Incorrect rpcuser or rpcpassword"); + } + } else if (response.status >= 400 && response.status != HTTP_BAD_REQUEST && response.status != HTTP_NOT_FOUND && response.status != HTTP_INTERNAL_SERVER_ERROR) throw std::runtime_error(strprintf("server returned HTTP error %d", response.status)); else if (response.body.empty()) throw std::runtime_error("no response from server"); 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/core_write.cpp b/src/core_write.cpp index 91742b7d1b..54b18a4931 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -209,6 +209,6 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, entry.pushKV("blockhash", hashBlock.GetHex()); if (include_hex) { - entry.pushKV("hex", EncodeHexTx(tx, serialize_flags)); // the hex-encoded transaction. used the name "hex" to be consistent with the verbose output of "getrawtransaction". + entry.pushKV("hex", EncodeHexTx(tx, serialize_flags)); // The hex-encoded transaction. Used the name "hex" to be consistent with the verbose output of "getrawtransaction". } } diff --git a/src/cuckoocache.h b/src/cuckoocache.h index 947e1a7185..d1de712024 100644 --- a/src/cuckoocache.h +++ b/src/cuckoocache.h @@ -224,7 +224,7 @@ private: * * Instead we treat the 32-bit random number as a Q32 fixed-point number in the range * [0,1) and simply multiply it by the size. Then we just shift the result down by - * 32-bits to get our bucket number. The results has non-uniformity the same as a + * 32-bits to get our bucket number. The result has non-uniformity the same as a * mod, but it is much faster to compute. More about this technique can be found at * http://lemire.me/blog/2016/06/27/a-fast-alternative-to-the-modulo-reduction/ * diff --git a/src/net.cpp b/src/net.cpp index 53a0a9b180..342dfbaeb9 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 @@ -1954,7 +1955,7 @@ void CConnman::ThreadOpenAddedConnections() for (const AddedNodeInfo& info : vInfo) { if (!info.fConnected) { if (!grant.TryAcquire()) { - // If we've used up our semaphore and need a new one, lets not wait here since while we are waiting + // If we've used up our semaphore and need a new one, let's not wait here since while we are waiting // the addednodeinfo state might change. break; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f5073fe903..61e6ae7448 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -339,7 +339,7 @@ bool MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const CBlockIndex* CNodeState *state = State(nodeid); assert(state != nullptr); - // Short-circuit most stuff in case its from the same node + // Short-circuit most stuff in case it is 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) { if (pit) { @@ -518,7 +518,7 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<con } // Iterate over those blocks in vToFetch (in forward direction), adding the ones that - // are not yet downloaded and not in flight to vBlocks. In the mean time, update + // are not yet downloaded and not in flight to vBlocks. In the meantime, update // pindexLastCommonBlock as long as all ancestors are already downloaded, or if it's // already part of our chain (and therefore don't need it even if pruned). for (const CBlockIndex* pindex : vToFetch) { 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/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp index 78dc9f81dd..f2ddbf259b 100644 --- a/src/qt/addressbookpage.cpp +++ b/src/qt/addressbookpage.cpp @@ -104,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; } @@ -211,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/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/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index b83755ab30..a45e9f85c1 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -206,7 +206,7 @@ void CoinControlDialog::showMenu(const QPoint &point) contextMenuItem = item; // disable some items (like Copy Transaction ID, lock, unlock) for tree roots in context menu - if (item->text(COLUMN_TXHASH).length() == 64) // transaction hash is 64 characters (this means its a child node, so its not a parent node in tree mode) + if (item->text(COLUMN_TXHASH).length() == 64) // transaction hash is 64 characters (this means it is a child node, so it is not a parent node in tree mode) { copyTransactionHashAction->setEnabled(true); if (model->isLockedCoin(uint256S(item->text(COLUMN_TXHASH).toStdString()), item->text(COLUMN_VOUT_INDEX).toUInt())) @@ -374,7 +374,7 @@ void CoinControlDialog::radioListMode(bool checked) // checkbox clicked by user void CoinControlDialog::viewItemChanged(QTreeWidgetItem* item, int column) { - if (column == COLUMN_CHECKBOX && item->text(COLUMN_TXHASH).length() == 64) // transaction hash is 64 characters (this means its a child node, so its not a parent node in tree mode) + if (column == COLUMN_CHECKBOX && item->text(COLUMN_TXHASH).length() == 64) // transaction hash is 64 characters (this means it is a child node, so it is not a parent node in tree mode) { COutPoint outpt(uint256S(item->text(COLUMN_TXHASH).toStdString()), item->text(COLUMN_VOUT_INDEX).toUInt()); 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/guiutil.h b/src/qt/guiutil.h index 71a69483f5..4b856986da 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -141,7 +141,7 @@ namespace GUIUtil * Makes a QTableView last column feel as if it was being resized from its left border. * Also makes sure the column widths are never larger than the table's viewport. * In Qt, all columns are resizable from the right, but it's not intuitive resizing the last column from the right. - * Usually our second to last columns behave as if stretched, and when on strech mode, columns aren't resizable + * Usually our second to last columns behave as if stretched, and when on stretch mode, columns aren't resizable * interactively or programmatically. * * This helper object takes care of this issue. diff --git a/src/qt/modaloverlay.cpp b/src/qt/modaloverlay.cpp index b573dbe226..249418213f 100644 --- a/src/qt/modaloverlay.cpp +++ b/src/qt/modaloverlay.cpp @@ -81,7 +81,7 @@ void ModalOverlay::tipUpdate(int count, const QDateTime& blockDate, double nVeri // keep a vector of samples of verification progress at height blockProcessTime.push_front(qMakePair(currentDate.toMSecsSinceEpoch(), nVerificationProgress)); - // show progress speed if we have more then one sample + // show progress speed if we have more than one sample if (blockProcessTime.size() >= 2) { double progressDelta = 0; double progressPerHour = 0; diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index 9f24fc7cea..05d3d36a74 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -648,7 +648,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"); 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/splashscreen.cpp b/src/qt/splashscreen.cpp index 5df1282f73..66e9dd0465 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -91,7 +91,7 @@ SplashScreen::SplashScreen(Qt::WindowFlags f, const NetworkStyle *networkStyle) pixPaint.setFont(QFont(font, 15*fontFactor)); - // if the version string is to long, reduce size + // if the version string is too long, reduce size fm = pixPaint.fontMetrics(); int versionTextWidth = fm.width(versionText); if(versionTextWidth > titleTextWidth+paddingRight-10) { 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/walletmodel.cpp b/src/qt/walletmodel.cpp index 39ef20c835..d092ebedfb 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -698,7 +698,7 @@ bool WalletModel::bumpFee(uint256 hash) confirmationDialog.exec(); QMessageBox::StandardButton retval = static_cast<QMessageBox::StandardButton>(confirmationDialog.result()); - // cancel sign&broadcast if users doesn't want to bump the fee + // cancel sign&broadcast if user doesn't want to bump the fee if (retval != QMessageBox::Yes) { return false; } @@ -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/random.h b/src/random.h index ea88670a37..5fe8f2fd04 100644 --- a/src/random.h +++ b/src/random.h @@ -32,7 +32,7 @@ void RandAddSeedSleep(); /** * Function to gather random data from multiple sources, failing whenever any - * of those source fail to provide a result. + * of those sources fail to provide a result. */ void GetStrongRandBytes(unsigned char* buf, int num); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 2077723af5..169caddc59 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -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 ? 1u : 0u); 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 diff --git a/src/rpc/server.h b/src/rpc/server.h index 8b32924fbc..d25268a8ab 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -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/script/interpreter.cpp b/src/script/interpreter.cpp index 927b0267ca..ffaba393c0 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -110,7 +110,7 @@ bool static IsValidSignatureEncoding(const std::vector<unsigned char> &sig) { // excluding the sighash byte. // * R-length: 1-byte length descriptor of the R value that follows. // * R: arbitrary-length big-endian encoded R value. It must use the shortest - // possible encoding for a positive integers (which means no null bytes at + // possible encoding for a positive integer (which means no null bytes at // the start, except a single one when the next byte has its highest bit set). // * S-length: 1-byte length descriptor of the S value that follows. // * S: arbitrary-length big-endian encoded S value. The same rules apply. diff --git a/src/serialize.h b/src/serialize.h index c454ba16b7..91da6b0f80 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -296,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++; @@ -312,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) { @@ -329,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,7 +375,7 @@ I ReadVarInt(Stream& is) } #define FLATDATA(obj) CFlatData((char*)&(obj), (char*)&(obj) + sizeof(obj)) -#define VARINT(obj) WrapVarInt(REF(obj)) +#define VARINT(obj, ...) WrapVarInt<__VA_ARGS__>(REF(obj)) #define COMPACTSIZE(obj) CCompactSize(REF(obj)) #define LIMITED_STRING(obj,n) LimitedString< n >(REF(obj)) @@ -395,7 +419,7 @@ public: } }; -template<typename I> +template<VarIntMode Mode, typename I> class CVarInt { protected: @@ -405,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); } }; @@ -461,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 diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index d92ab02d6b..1401188a2b 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -184,7 +184,7 @@ void Win32LockedPageAllocator::FreeLocked(void* addr, size_t len) size_t Win32LockedPageAllocator::GetLimit() { - // TODO is there a limit on windows, how to get it? + // TODO is there a limit on Windows, how to get it? return std::numeric_limits<size_t>::max(); } #endif diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 0d8bd90119..6b188a06b4 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -491,7 +491,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) // this test could be a security issue. BOOST_CHECK(info1.GetNewBucket(nKey1) != info1.GetNewBucket(nKey2)); - // Test: Ports should not effect bucket placement in the addr + // Test: Ports should not affect bucket placement in the addr CAddrInfo info2 = CAddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1), info2.GetNewBucket(nKey1)); diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 36e271295a..de7d8f7b90 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -313,7 +313,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) auto utxod = FindRandomFrom(coinbase_coins); // Reuse the exact same coinbase tx = std::get<0>(utxod->second); - // shouldn't be available for reconnection if its been duplicated + // shouldn't be available for reconnection if it's been duplicated disconnected_coins.erase(utxod->first); duplicate_coins.insert(utxod->first); diff --git a/src/test/cuckoocache_tests.cpp b/src/test/cuckoocache_tests.cpp index 51ebfc3800..ccd5caacd5 100644 --- a/src/test/cuckoocache_tests.cpp +++ b/src/test/cuckoocache_tests.cpp @@ -163,7 +163,7 @@ void test_cache_erase(size_t megabytes) for (uint32_t i = (n_insert / 2); i < n_insert; ++i) set.insert(hashes_insert_copy[i]); - /** elements that we marked erased but that are still there */ + /** elements that we marked as erased but are still there */ size_t count_erased_but_contained = 0; /** elements that we did not erase but are older */ size_t count_stale = 0; @@ -303,7 +303,7 @@ void test_cache_generations() local_rand_ctx = FastRandomContext(true); // block_activity models a chunk of network activity. n_insert elements are - // adde to the cache. The first and last n/4 are stored for removal later + // added to the cache. The first and last n/4 are stored for removal later // and the middle n/2 are not stored. This models a network which uses half // the signatures of recently (since the last block) added transactions // immediately and never uses the other half. diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 754a86344f..6694401a29 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -237,7 +237,7 @@ BOOST_AUTO_TEST_CASE(iterator_ordering) } struct StringContentsSerializer { - // Used to make two serialized objects the same while letting them have a different lengths + // Used to make two serialized objects the same while letting them have different lengths // This is a terrible idea std::string str; StringContentsSerializer() {} diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 1766c6a093..19cd3b0963 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -523,7 +523,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) pool.addUnchecked(tx6.GetHash(), entry.Fee(1100LL).FromTx(tx6)); pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7)); - // we only require this remove, at max, 2 txn, because its not clear what we're really optimizing for aside from that + // we only require this to remove, at max, 2 txn, because it's not clear what we're really optimizing for aside from that pool.TrimToSize(pool.DynamicMemoryUsage() - 1); BOOST_CHECK(pool.exists(tx4.GetHash())); BOOST_CHECK(pool.exists(tx6.GetHash())); diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 42fd59380a..7a79a77e8b 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -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/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index 9ec9d6cba3..1aa54189b6 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -35,7 +35,7 @@ ToMemPool(CMutableTransaction& tx) BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup) { - // Make sure skipping validation of transctions that were + // Make sure skipping validation of transactions that were // validated going into the memory pool does not allow // double-spends in blocks to pass validation when they should not. diff --git a/src/tinyformat.h b/src/tinyformat.h index d34cfaa94f..14b7cd3026 100644 --- a/src/tinyformat.h +++ b/src/tinyformat.h @@ -155,7 +155,7 @@ namespace tfm = tinyformat; #endif #ifdef __APPLE__ -// Workaround OSX linker warning: xcode uses different default symbol +// Workaround OSX linker warning: Xcode uses different default symbol // visibilities for static libs vs executables (see issue #25) # define TINYFORMAT_HIDDEN __attribute__((visibility("hidden"))) #else @@ -592,7 +592,7 @@ inline const char* printFormatStringLiteral(std::ostream& out, const char* fmt) // Formatting options which can't be natively represented using the ostream // state are returned in spacePadPositive (for space padded positive numbers) // and ntrunc (for truncating conversions). argIndex is incremented if -// necessary to pull out variable width and precision . The function returns a +// necessary to pull out variable width and precision. The function returns a // pointer to the character after the end of the current format spec. inline const char* streamStateFromFormat(std::ostream& out, bool& spacePadPositive, int& ntrunc, const char* fmtStart, diff --git a/src/txdb.cpp b/src/txdb.cpp index 7a1d920117..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)); @@ -351,7 +351,7 @@ public: ::Unserialize(s, CTxOutCompressor(vout[i])); } // coinbase height - ::Unserialize(s, VARINT(nHeight)); + ::Unserialize(s, VARINT(nHeight, VarIntMode::NONNEGATIVE_SIGNED)); } }; diff --git a/src/txmempool.h b/src/txmempool.h index c6a1bf08ce..08a3421015 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -689,7 +689,7 @@ private: }; /** - * CCoinsView that brings transactions from a memorypool into view. + * CCoinsView that brings transactions from a mempool into view. * It does not check for spendings by memory pool transactions. * Instead, it provides access to all Coins which are either unspent in the * base CCoinsView, or are outputs from any mempool transaction! diff --git a/src/undo.h b/src/undo.h index 7aae034de2..f292924165 100644 --- a/src/undo.h +++ b/src/undo.h @@ -15,7 +15,7 @@ * * Contains the prevout's CTxOut being spent, and its metadata as well * (coinbase or not, height). The serialization contains a dummy value of - * zero. This is be compatible with older versions which expect to see + * zero. This is compatible with older versions which expect to see * the transaction version there. */ class TxInUndoSerializer @@ -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 ? 1u : 0u))); if (txout->nHeight > 0) { // Required to maintain compatibility with older undo format. ::Serialize(s, (unsigned char)0); @@ -51,7 +51,7 @@ 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, CTxOutCompressor(REF(txout->out))); diff --git a/src/util.h b/src/util.h index e4170d8aa2..aebd2fd590 100644 --- a/src/util.h +++ b/src/util.h @@ -136,7 +136,7 @@ template<typename T, typename... Args> static inline void MarkUsed(const T& t, c // Be conservative when using LogPrintf/error or other things which // unconditionally log to debug.log! It should not be the case that an inbound -// peer can fill up a users disk with debug.log entries. +// peer can fill up a user's disk with debug.log entries. #ifdef USE_COVERAGE #define LogPrintf(...) do { MarkUsed(__VA_ARGS__); } while(0) diff --git a/src/validation.cpp b/src/validation.cpp index bee890437e..614876ea49 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -949,7 +949,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // This transaction should only count for fee estimation if: // - it isn't a BIP 125 replacement transaction (may not be widely supported) - // - it's not being readded during a reorg which bypasses typical mempool fee limits + // - it's not being re-added during a reorg which bypasses typical mempool fee limits // - the node is not behind // - the transaction is not dependent on any other transactions in the mempool bool validForFeeEstimation = !fReplacementTransaction && !bypass_limits && IsCurrentForFeeEstimation() && pool.HasNoInputsOf(tx); @@ -1852,7 +1852,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // Once BIP34 activated it was not possible to create new duplicate coinbases and thus other than starting // with the 2 existing duplicate coinbase pairs, not possible to create overwriting txs. But by the // time BIP34 activated, in each of the existing pairs the duplicate coinbase had overwritten the first - // before the first had been spent. Since those coinbases are sufficiently buried its no longer possible to create further + // before the first had been spent. Since those coinbases are sufficiently buried it's no longer possible to create further // duplicate transactions descending from the known pairs either. // If we're on the known chain at height greater than where BIP34 activated, we can save the db accesses needed for the BIP30 check. 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/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 01125dd618..1721bc6df6 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -83,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; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7ad9efff70..0dc6de9564 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -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())); } } @@ -1221,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())); } } @@ -2015,7 +2015,7 @@ UniValue listsinceblock(const JSONRPCRequest& request) " ],\n" " \"removed\": [\n" " <structure is the same as \"transactions\" above, only present if include_removed=true>\n" - " Note: transactions that were readded in the active chain will appear as-is in this array, and may thus have a positive confirmation count.\n" + " Note: transactions that were re-added in the active chain will appear as-is in this array, and may thus have a positive confirmation count.\n" " ],\n" " \"lastblock\": \"lastblockhash\" (string) The hash of the block (target_confirmations-1) from the best block on the main chain. This is typically used to feed back into listsinceblock the next time you call it. So you would generally use a target_confirmations of say 6, so you will be continually re-notified of transactions until they've reached 6 confirmations plus any new ones\n" "}\n" @@ -2363,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."); } @@ -2429,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."); } @@ -2485,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."); } @@ -2532,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."); } @@ -3183,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())); } } @@ -3578,7 +3570,7 @@ UniValue rescanblockchain(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid stop_height"); } else if (pindexStop->nHeight < pindexStart->nHeight) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "stop_height must be greater then start_height"); + throw JSONRPCError(RPC_INVALID_PARAMETER, "stop_height must be greater than start_height"); } } } 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 d6b0daf4fe..808f8b8838 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -114,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; @@ -272,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); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bd5094085e..611d9bc100 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -42,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; @@ -823,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) @@ -840,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; @@ -1269,7 +1267,7 @@ void CWallet::BlockUntilSyncedToCurrentChain() { // chainActive.Tip()... // We could also take cs_wallet here, and call m_last_block_processed // protected by cs_wallet instead of cs_main, but as long as we need - // cs_main here anyway, its easier to just call it cs_main-protected. + // cs_main here anyway, it's easier to just call it cs_main-protected. LOCK(cs_main); const CBlockIndex* initialChainTip = chainActive.Tip(); @@ -2649,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 @@ -2666,12 +2664,12 @@ 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, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, @@ -2768,7 +2766,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac 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)); @@ -4002,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)); @@ -4015,6 +4012,20 @@ 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. @@ -4186,8 +4197,8 @@ bool CWalletTx::AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& // We must set fInMempool here - while it will be re-set to true by the // entered-mempool callback, if we did not there would be a race where a // user could call sendmoney in a loop and hit spurious out of funds errors - // because we think that the transaction they just generated's change is - // unavailable as we're not yet aware its in mempool. + // because we think that this newly generated transaction's change is + // unavailable as we're not yet aware that it is in the mempool. bool ret = ::AcceptToMemoryPool(mempool, state, tx, nullptr /* pfMissingInputs */, nullptr /* plTxnReplaced */, false /* bypass_limits */, nAbsurdFee); fInMempool |= ret; @@ -4203,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. @@ -4236,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; @@ -4275,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; @@ -4287,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 dc0e8d07d7..5ac8457eb4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -43,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; @@ -96,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 */ @@ -989,6 +987,8 @@ public: 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(); @@ -1232,7 +1232,7 @@ public: } }; -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); /** |