diff --git a/doc/descriptors.md b/doc/descriptors.md index 6a94b867c3f5..fce4f9a323a4 100644 --- a/doc/descriptors.md +++ b/doc/descriptors.md @@ -73,7 +73,8 @@ Descriptors consist of several types of expressions. The top level expression is - Followed by zero or more `/NUM` unhardened and `/NUM'` hardened BIP32 derivation steps. - Optionally followed by a single `/*` or `/*'` final step to denote all (direct) unhardened or hardened children. - The usage of hardened derivation steps requires providing the private key. -- Anywhere a `'` suffix is permitted to denote hardened derivation, the suffix `h` can be used instead. + +(Anywhere a `'` suffix is permitted to denote hardened derivation, the suffix `h` can be used instead.) `ADDR` expressions are any type of supported address: - P2PKH addresses (base58, of the form `X...`). Note that P2PKH addresses in descriptors cannot be used for P2PK outputs (use the `pk` function instead). diff --git a/doc/release-note-26194.md b/doc/release-note-26194.md new file mode 100644 index 000000000000..b72dbf9a238b --- /dev/null +++ b/doc/release-note-26194.md @@ -0,0 +1,4 @@ +Add `next_index` in `listdescriptors` RPC +----------------- + +- Added a new `next_index` field in the response in `listdescriptors` to have the same format as `importdescriptors` diff --git a/doc/release-notes-25158.md b/doc/release-notes-25158.md new file mode 100644 index 000000000000..ce8ab53ddd83 --- /dev/null +++ b/doc/release-notes-25158.md @@ -0,0 +1,6 @@ +RPC Wallet +---------- + +- The `gettransaction`, `listtransactions`, `listsinceblock` RPCs now return + the `abandoned` field for all transactions. Previously, the "abandoned" field + was only returned for sent transactions. (#25158) \ No newline at end of file diff --git a/doc/release-notes-27068.md b/doc/release-notes-27068.md new file mode 100644 index 000000000000..3f5c5dba37bd --- /dev/null +++ b/doc/release-notes-27068.md @@ -0,0 +1,6 @@ +Wallet +------ + +- Wallet passphrases may now contain null characters. + Prior to this change, only characters up to the first + null character were recognized and accepted. (#27068) \ No newline at end of file diff --git a/doc/release-process.md b/doc/release-process.md index 2ebcc1582662..aca1c12e52da 100644 --- a/doc/release-process.md +++ b/doc/release-process.md @@ -71,7 +71,7 @@ Checkout the Dash Core version you'd like to build: pushd ./dash export SIGNER='(your builder key, ie udjinm6, pasta, etc)' export VERSION='(new version, e.g. 20.0.0)' -git fetch "v${VERSION}" +git fetch origin "v${VERSION}" git checkout "v${VERSION}" popd ``` diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index c9f918c76467..ae83c814f895 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -437,11 +437,15 @@ $(srcdir)/qt/dashstrings.cpp: FORCE @test -n $(XGETTEXT) || echo "xgettext is required for updating translations" $(AM_V_GEN) cd $(srcdir); XGETTEXT=$(XGETTEXT) COPYRIGHT_HOLDERS="$(COPYRIGHT_HOLDERS)" $(PYTHON) ../share/qt/extract_strings_qt.py $(libbitcoin_node_a_SOURCES) $(libbitcoin_wallet_a_SOURCES) $(libbitcoin_common_a_SOURCES) $(libbitcoin_zmq_a_SOURCES) $(libbitcoin_consensus_a_SOURCES) $(libbitcoin_util_a_SOURCES) +# The resulted dash_en.xlf source file should follow Transifex requirements. +# See: https://docs.transifex.com/formats/xliff#how-to-distinguish-between-a-source-file-and-a-translation-file translate: $(srcdir)/qt/dashstrings.cpp $(QT_FORMS_UI) $(QT_FORMS_UI) $(BITCOIN_QT_BASE_CPP) qt/bitcoin.cpp $(BITCOIN_QT_WINDOWS_CPP) $(BITCOIN_QT_WALLET_CPP) $(BITCOIN_QT_H) $(BITCOIN_MM) @test -n $(LUPDATE) || echo "lupdate is required for updating translations" $(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(LUPDATE) -no-obsolete -I $(srcdir) -locations relative $^ -ts $(srcdir)/qt/locale/dash_en.ts @test -n $(LCONVERT) || echo "lconvert is required for updating translations" - $(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(LCONVERT) -o $(srcdir)/qt/locale/dash_en.xlf -i $(srcdir)/qt/locale/dash_en.ts + $(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(LCONVERT) -drop-translations -o $(srcdir)/qt/locale/dash_en.xlf -i $(srcdir)/qt/locale/dash_en.ts + @$(SED) -i.old -e 's|source-language="en" target-language="en"|source-language="en"|' -e '/<\/target>/d' $(srcdir)/qt/locale/dash_en.xlf + @rm -f $(srcdir)/qt/locale/dash_en.xlf.old $(QT_QRC_LOCALE_CPP): $(QT_QRC_LOCALE) $(QT_QM) @test -f $(RCC) diff --git a/src/crypto/sha256_arm_shani.cpp b/src/crypto/sha256_arm_shani.cpp index 7ffa56be7060..2ea1d9c2acc4 100644 --- a/src/crypto/sha256_arm_shani.cpp +++ b/src/crypto/sha256_arm_shani.cpp @@ -62,7 +62,7 @@ void Transform(uint32_t* s, const unsigned char* chunk, size_t blocks) MSG3 = vreinterpretq_u32_u8(vrev32q_u8(vld1q_u8(chunk + 48))); chunk += 64; - // Original implemenation preloaded message and constant addition which was 1-3% slower. + // Original implementation preloaded message and constant addition which was 1-3% slower. // Now included as first step in quad round code saving one Q Neon register // "TMP0 = vaddq_u32(MSG0, vld1q_u32(&K[0]));" diff --git a/src/init.cpp b/src/init.cpp index f748a4e93416..d2e165619bf1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -591,7 +591,6 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-peertimeout=", strprintf("Specify a p2p connection timeout delay in seconds. After connecting to a peer, wait this amount of time before considering disconnection based on inactivity (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); // TODO: remove the sentence "Nodes not using ... incoming connections." once the changes from // https://github.com/bitcoin/bitcoin/pull/23542 have become widespread. argsman.AddArg("-port=", strprintf("Listen for connections on . Nodes not using the default ports (default: %u, testnet: %u, regtest: %u) are unlikely to get incoming connections. Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); @@ -746,6 +745,8 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-bytespersigop", strprintf("Equivalent bytes per sigop in transactions for relay and mining (default: %u)", DEFAULT_BYTES_PER_SIGOP), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-datacarriersize", strprintf("Maximum size of data in data carrier transactions we relay and mine (default: %u)", MAX_OP_RETURN_RELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); + argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, + OptionsCategory::NODE_RELAY); argsman.AddArg("-minrelaytxfee=", strprintf("Fees (in %s/kB) smaller than this are considered zero fee for relaying, mining and transaction creation (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_MIN_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-whitelistforcerelay", strprintf("Add 'forcerelay' permission to whitelisted inbound peers with default permissions. This will relay transactions even if the transactions were already in the mempool. (default: %d)", DEFAULT_WHITELISTFORCERELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); diff --git a/src/qt/askpassphrasedialog.cpp b/src/qt/askpassphrasedialog.cpp index 829a7660491b..3b9fd117ffe6 100644 --- a/src/qt/askpassphrasedialog.cpp +++ b/src/qt/askpassphrasedialog.cpp @@ -106,11 +106,10 @@ void AskPassphraseDialog::accept() oldpass.reserve(MAX_PASSPHRASE_SIZE); newpass1.reserve(MAX_PASSPHRASE_SIZE); newpass2.reserve(MAX_PASSPHRASE_SIZE); - // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) - // Alternately, find a way to make this input mlock()'d to begin with. - oldpass.assign(ui->passEdit1->text().toStdString().c_str()); - newpass1.assign(ui->passEdit2->text().toStdString().c_str()); - newpass2.assign(ui->passEdit3->text().toStdString().c_str()); + + oldpass.assign(std::string_view{ui->passEdit1->text().toStdString()}); + newpass1.assign(std::string_view{ui->passEdit2->text().toStdString()}); + newpass2.assign(std::string_view{ui->passEdit3->text().toStdString()}); secureClearPassFields(); @@ -185,8 +184,19 @@ void AskPassphraseDialog::accept() try { if(!model->setWalletLocked(false, oldpass, mode == UnlockMixing)) { - QMessageBox::critical(this, tr("Wallet unlock failed"), - tr("The passphrase entered for the wallet decryption was incorrect.")); + // Check if the passphrase has a null character (see #27067 for details) + if (oldpass.find('\0') == std::string::npos) { + QMessageBox::critical(this, tr("Wallet unlock failed"), + tr("The passphrase entered for the wallet decryption was incorrect.")); + } else { + QMessageBox::critical(this, tr("Wallet unlock failed"), + tr("The passphrase entered for the wallet decryption is incorrect. " + "It contains a null character (ie - a zero byte). " + "If the passphrase was set with a version of this software prior to 23.0, " + "please try again with only the characters up to — but not including — " + "the first null character. If this is successful, please set a new " + "passphrase to avoid this issue in the future.")); + } } else { @@ -207,8 +217,18 @@ void AskPassphraseDialog::accept() } else { - QMessageBox::critical(this, tr("Wallet encryption failed"), - tr("The passphrase entered for the wallet decryption was incorrect.")); + // Check if the old passphrase had a null character (see #27067 for details) + if (oldpass.find('\0') == std::string::npos) { + QMessageBox::critical(this, tr("Passphrase change failed"), + tr("The passphrase entered for the wallet decryption was incorrect.")); + } else { + QMessageBox::critical(this, tr("Passphrase change failed"), + tr("The old passphrase entered for the wallet decryption is incorrect. " + "It contains a null character (ie - a zero byte). " + "If the passphrase was set with a version of this software prior to 23.0, " + "please try again with only the characters up to — but not including — " + "the first null character.")); + } } } else diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 3b99604d8868..e78adbb79884 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -171,6 +171,7 @@ static void initTranslations(QTranslator &qtTranslatorBase, QTranslator &qtTrans static bool InitSettings() { + gArgs.EnsureDataDir(); if (!gArgs.GetSettingsPath()) { return true; // Do nothing if settings file disabled. } diff --git a/src/support/allocators/secure.h b/src/support/allocators/secure.h index 14676dbe4d8e..46be5f4876f1 100644 --- a/src/support/allocators/secure.h +++ b/src/support/allocators/secure.h @@ -55,6 +55,7 @@ struct secure_allocator { }; // This is exactly like std::string, but with a custom allocator. +// TODO: Consider finding a way to make incoming RPC request.params[i] mlock()ed as well typedef std::basic_string, secure_allocator > SecureString; typedef std::vector > SecureVector; diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index 766e817a8ab5..b130b3957229 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -19,6 +19,7 @@ #endif #include +#include #include #ifdef ARENA_DEBUG #include diff --git a/src/support/lockedpool.h b/src/support/lockedpool.h index 1aaa76009d67..794e51cc6baa 100644 --- a/src/support/lockedpool.h +++ b/src/support/lockedpool.h @@ -5,11 +5,11 @@ #ifndef BITCOIN_SUPPORT_LOCKEDPOOL_H #define BITCOIN_SUPPORT_LOCKEDPOOL_H -#include +#include #include #include -#include #include +#include #include /** diff --git a/src/util/system.cpp b/src/util/system.cpp index 424037321024..caf7972e6fe2 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -448,8 +448,7 @@ fs::path ArgsManager::GetDataDir(bool net_specific) const LOCK(cs_args); fs::path& path = net_specific ? m_cached_network_datadir_path : m_cached_datadir_path; - // Cache the path to avoid calling fs::create_directories on every call of - // this function + // Used cached path if available if (!path.empty()) return path; const fs::path datadir{GetPathArg("-datadir")}; @@ -463,15 +462,8 @@ fs::path ArgsManager::GetDataDir(bool net_specific) const path = GetDefaultDataDir(); } - if (!fs::exists(path)) { - fs::create_directories(path / "wallets"); - } - if (net_specific && !BaseParams().DataDir().empty()) { path /= fs::PathFromString(BaseParams().DataDir()); - if (!fs::exists(path)) { - fs::create_directories(path / "wallets"); - } } return path; @@ -485,6 +477,27 @@ fs::path ArgsManager::GetBackupsDirPath() return fs::absolute(GetPathArg("-walletbackupsdir")); } +void ArgsManager::EnsureDataDir() const +{ + /** + * "/wallets" subdirectories are created in all **new** + * datadirs, because wallet code will create new wallets in the "wallets" + * subdirectory only if exists already, otherwise it will create them in + * the top-level datadir where they could interfere with other files. + * Wallet init code currently avoids creating "wallets" directories itself + * for backwards compatibility, but this be changed in the future and + * wallet code here could go away (#16220). + */ + auto path{GetDataDir(false)}; + if (!fs::exists(path)) { + fs::create_directories(path / "wallets"); + } + path = GetDataDir(true); + if (!fs::exists(path)) { + fs::create_directories(path / "wallets"); + } +} + void ArgsManager::ClearPathCache() { LOCK(cs_args); @@ -530,6 +543,7 @@ bool ArgsManager::IsArgSet(const std::string& strArg) const bool ArgsManager::InitSettings(std::string& error) { + EnsureDataDir(); if (!GetSettingsPath()) { return true; // Do nothing if settings file disabled. } @@ -963,6 +977,11 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file return true; } +fs::path ArgsManager::GetConfigFilePath() const +{ + return GetConfigFile(GetPathArg("-conf", BITCOIN_CONF_FILENAME)); +} + bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) { { @@ -971,8 +990,8 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) m_config_sections.clear(); } - const fs::path conf_path = GetPathArg("-conf", BITCOIN_CONF_FILENAME); - std::ifstream stream{GetConfigFile(conf_path)}; + const auto conf_path{GetConfigFilePath()}; + std::ifstream stream{conf_path}; // not ok to have a config file specified that cannot be opened if (IsArgSet("-conf") && !stream.good()) { diff --git a/src/util/system.h b/src/util/system.h index 7bf10098315c..dac0f0d82e8a 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -257,6 +257,11 @@ class ArgsManager void SelectConfigNetwork(const std::string& network); [[nodiscard]] bool ParseParameters(int argc, const char* const argv[], std::string& error); + + /** + * Return config file path (read-only) + */ + fs::path GetConfigFilePath() const; [[nodiscard]] bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false); /** @@ -495,13 +500,18 @@ class ArgsManager */ void LogArgs() const; + /** + * If datadir does not exist, create it along with wallets/ + * subdirectory(s). + */ + void EnsureDataDir() const; + private: /** * Get data directory path * * @param net_specific Append network identifier to the returned path * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned - * @post Returned directory path is created unless it is empty */ fs::path GetDataDir(bool net_specific) const; diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index af293c83b3d7..617d905170df 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1980,7 +1980,8 @@ RPCHelpMan listdescriptors() {RPCResult::Type::NUM, "", "Range start inclusive"}, {RPCResult::Type::NUM, "", "Range end inclusive"}, }}, - {RPCResult::Type::NUM, "next", /*optional=*/true, "The next index to generate addresses from; defined only for ranged descriptors"}, + {RPCResult::Type::NUM, "next", /*optional=*/true, "Same as next_index field. Kept for compatibility reason."}, + {RPCResult::Type::NUM, "next_index", /*optional=*/true, "The next index to generate addresses from; defined only for ranged descriptors"}, }}, }} }}, @@ -2041,6 +2042,7 @@ RPCHelpMan listdescriptors() range.push_back(wallet_descriptor.range_end - 1); spk.pushKV("range", range); spk.pushKV("next", wallet_descriptor.next_index); + spk.pushKV("next_index", wallet_descriptor.next_index); } descriptors.push_back(spk); } diff --git a/src/wallet/rpc/encrypt.cpp b/src/wallet/rpc/encrypt.cpp index 13cdef3d3317..211b255ec1fb 100644 --- a/src/wallet/rpc/encrypt.cpp +++ b/src/wallet/rpc/encrypt.cpp @@ -51,9 +51,7 @@ RPCHelpMan walletpassphrase() // Note that the walletpassphrase is stored in request.params[0] which is not mlock()ed SecureString strWalletPass; strWalletPass.reserve(100); - // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) - // Alternately, find a way to make request.params[0] mlock()'d to begin with. - strWalletPass = request.params[0].get_str().c_str(); + strWalletPass = std::string_view{request.params[0].get_str()}; // Get the timeout nSleepTime = request.params[1].getInt(); @@ -83,7 +81,17 @@ RPCHelpMan walletpassphrase() } if (!pwallet->Unlock(strWalletPass, fForMixingOnly)) { - throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); + // Check if the passphrase has a null character (see #27067 for details) + if (strWalletPass.find('\0') == std::string::npos) { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); + } else { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered is incorrect. " + "It contains a null character (ie - a zero byte). " + "If the passphrase was set with a version of this software prior to 23.0, " + "please try again with only the characters up to — but not including — " + "the first null character. If this is successful, please set a new " + "passphrase to avoid this issue in the future."); + } } pwallet->TopUpKeyPool(); @@ -138,23 +146,29 @@ RPCHelpMan walletpassphrasechange() if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrasechange was called."); } - - // TODO: get rid of these .c_str() calls by implementing SecureString::operator=(std::string) - // Alternately, find a way to make request.params[0] mlock()'d to begin with. SecureString strOldWalletPass; strOldWalletPass.reserve(100); - strOldWalletPass = request.params[0].get_str().c_str(); + strOldWalletPass = std::string_view{request.params[0].get_str()}; SecureString strNewWalletPass; strNewWalletPass.reserve(100); - strNewWalletPass = request.params[1].get_str().c_str(); + strNewWalletPass = std::string_view{request.params[1].get_str()}; if (strOldWalletPass.empty() || strNewWalletPass.empty()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase cannot be empty"); } if (!pwallet->ChangeWalletPassphrase(strOldWalletPass, strNewWalletPass)) { - throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); + // Check if the old passphrase had a null character (see #27067 for details) + if (strOldWalletPass.find('\0') == std::string::npos) { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); + } else { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The old wallet passphrase entered is incorrect. " + "It contains a null character (ie - a zero byte). " + "If the old passphrase was set with a version of this software prior to 23.0, " + "please try again with only the characters up to — but not including — " + "the first null character."); + } } return NullUniValue; @@ -237,12 +251,9 @@ RPCHelpMan encryptwallet() if (pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an encrypted wallet, but encryptwallet was called."); } - - // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) - // Alternately, find a way to make request.params[0] mlock()'d to begin with. SecureString strWalletPass; strWalletPass.reserve(100); - strWalletPass = request.params[0].get_str().c_str(); + strWalletPass = std::string_view{request.params[0].get_str()}; if (strWalletPass.empty()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase cannot be empty"); diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index dbbe2ec394a6..5cfbdc6adbd0 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -409,6 +409,7 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM entry.pushKV("label", label); } entry.pushKV("vout", r.vout); + entry.pushKV("abandoned", wtx.isAbandoned()); if (fLong) WalletTxToJSON(wallet, wtx, entry); ret.push_back(entry); @@ -475,8 +476,7 @@ RPCHelpMan listtransactions() }, TransactionDescriptionString()), { - {RPCResult::Type::BOOL, "abandoned", /* optional */ true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" - "'send' category of transactions."}, + {RPCResult::Type::BOOL, "abandoned", "'true' if the transaction has been abandoned (inputs are respendable)."}, })}, } }, @@ -588,7 +588,7 @@ RPCHelpMan listsinceblock() }, TransactionDescriptionString()), { - {RPCResult::Type::BOOL, "abandoned", /* optional */ true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the 'send' category of transactions."}, + {RPCResult::Type::BOOL, "abandoned", "'true' if the transaction has been abandoned (inputs are respendable)."}, {RPCResult::Type::STR, "label", /* optional */ true, "A comment for the address/transaction, if any."}, {RPCResult::Type::STR, "to", "If a comment to is associated with the transaction."}, })}, @@ -728,8 +728,7 @@ RPCHelpMan gettransaction() {RPCResult::Type::NUM, "vout", "the vout value"}, {RPCResult::Type::STR_AMOUNT, "fee", /* optional */ true, "The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" "'send' category of transactions."}, - {RPCResult::Type::BOOL, "abandoned", /* optional */ true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" - "'send' category of transactions."}, + {RPCResult::Type::BOOL, "abandoned", "'true' if the transaction has been abandoned (inputs are respendable)."}, }}, }}, {RPCResult::Type::STR_HEX, "hex", "Raw data for transaction"}, diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 38597ef270d3..9a3cc0c288e8 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -597,7 +597,7 @@ static RPCHelpMan createwallet() passphrase.reserve(100); std::vector warnings; if (!request.params[3].isNull()) { - passphrase = request.params[3].get_str().c_str(); + passphrase = std::string_view{request.params[3].get_str()}; if (passphrase.empty()) { // Empty string means unencrypted warnings.emplace_back(Untranslated("Empty string given as passphrase, wallet will not be encrypted.")); diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index ae1ded29979d..71ed77e58de4 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -816,6 +816,13 @@ static std::optional CreateTransactionInternal( const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size); CAmount selection_target = recipients_sum + not_input_fees; + // This can only happen if feerate is 0, and requested destinations are value of 0 (e.g. OP_RETURN) + // and no pre-selected inputs. This will result in 0-input transaction, which is consensus-invalid anyways + if (selection_target == 0 && !coin_control.HasSelected()) { + error = _("Transaction requires one destination of non-0 value, a non-0 feerate, or a pre-selected input"); + return std::nullopt; + } + // Get available coins auto res_available_coins = AvailableCoins(wallet, &coin_control, diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 6872fc06b963..6be001466e2c 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -458,7 +458,7 @@ BOOST_AUTO_TEST_CASE(LoadReceiveRequests) // Test some watch-only LegacyScriptPubKeyMan methods by the procedure of loading (LoadWatchOnly), // checking (HaveWatchOnly), getting (GetWatchPubKey) and removing (RemoveWatchOnly) a -// given PubKey, resp. its corresponding P2PK Script. Results of the the impact on +// given PubKey, resp. its corresponding P2PK Script. Results of the impact on // the address -> PubKey map is dependent on whether the PubKey is a point on the curve static void TestWatchOnlyPubKey(LegacyScriptPubKeyMan* spk_man, const CPubKey& add_pubkey) { diff --git a/test/README.md b/test/README.md index fac2d4506d05..6417c4d058e5 100644 --- a/test/README.md +++ b/test/README.md @@ -112,34 +112,57 @@ how many jobs to run, append `--jobs=n` The individual tests and the test_runner harness have many command-line options. Run `test/functional/test_runner.py -h` to see them all. -#### Speed up test runs with a ramdisk +#### Speed up test runs with a RAM disk -If you have available RAM on your system you can create a ramdisk to use as the `cache` and `tmp` directories for the functional tests in order to speed them up. -Speed-up amount varies on each system (and according to your ram speed and other variables), but a 2-3x speed-up is not uncommon. +If you have available RAM on your system you can create a RAM disk to use as the `cache` and `tmp` directories for the functional tests in order to speed them up. +Speed-up amount varies on each system (and according to your RAM speed and other variables), but a 2-3x speed-up is not uncommon. -To create a 4GB ramdisk on Linux at `/mnt/tmp/`: +**Linux** + +To create a 4 GiB RAM disk at `/mnt/tmp/`: ```bash sudo mkdir -p /mnt/tmp sudo mount -t tmpfs -o size=4g tmpfs /mnt/tmp/ ``` -Configure the size of the ramdisk using the `size=` option. -The size of the ramdisk needed is relative to the number of concurrent jobs the test suite runs. -For example running the test suite with `--jobs=100` might need a 16GB ramdisk, but running with `--jobs=4` will only need a 4GB ramdisk. +Configure the size of the RAM disk using the `size=` option. +The size of the RAM disk needed is relative to the number of concurrent jobs the test suite runs. +For example running the test suite with `--jobs=100` might need a 4 GiB RAM disk, but running with `--jobs=32` will only need a 2.5 GiB RAM disk. -To use, run the test suite specifying the ramdisk as the `cachedir` and `tmpdir`: +To use, run the test suite specifying the RAM disk as the `cachedir` and `tmpdir`: ```bash test/functional/test_runner.py --cachedir=/mnt/tmp/cache --tmpdir=/mnt/tmp ``` -Once finished with the tests and the disk, and to free the ram, simply unmount the disk: +Once finished with the tests and the disk, and to free the RAM, simply unmount the disk: ```bash sudo umount /mnt/tmp ``` +**macOS** + +To create a 4 GiB RAM disk named "ramdisk" at `/Volumes/ramdisk/`: + +```bash +diskutil erasevolume HFS+ ramdisk $(hdiutil attach -nomount ram://8388608) +``` + +Configure the RAM disk size, expressed as the number of blocks, at the end of the command +(`4096 MiB * 2048 blocks/MiB = 8388608 blocks` for 4 GiB). To run the tests using the RAM disk: + +```bash +test/functional/test_runner.py --cachedir=/Volumes/ramdisk/cache --tmpdir=/Volumes/ramdisk/tmp +``` + +To unmount: + +```bash +umount /Volumes/ramdisk +``` + #### Troubleshooting and debugging test failures ##### Resource contention diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index bc7a45d7e903..99e70478f71f 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -171,7 +171,7 @@ def run_test(self): self.log.info(f"Reject block with invalid tx: {TxTemplate.__name__}") blockname = f"for_invalid.{TxTemplate.__name__}" - badblock = self.next_block(blockname) + self.next_block(blockname) badtx = template.get_tx() if TxTemplate != invalid_txs.InputMissing: self.sign_tx(badtx, attempt_spend_tx) @@ -483,7 +483,7 @@ def run_test(self): # self.log.info("Check P2SH SIGOPS are correctly counted") self.move_tip(35) - b39 = self.next_block(39) + self.next_block(39) b39_outputs = 0 b39_sigops_per_output = 6 @@ -682,7 +682,7 @@ def run_test(self): self.log.info("Reject a block with two coinbase transactions") self.move_tip(44) - b51 = self.next_block(51) + self.next_block(51) cb2 = create_coinbase(51, self.coinbase_pubkey) b51 = self.update_block(51, [cb2]) self.send_blocks([b51], success=False, reject_reason='bad-cb-multiple', reconnect=True) @@ -762,7 +762,7 @@ def run_test(self): # b57 - a good block with 2 txs, don't submit until end self.move_tip(55) - b57 = self.next_block(57) + self.next_block(57) tx = self.create_and_sign_transaction(out[16], 1) tx1 = self.create_tx(tx, 0, 1) b57 = self.update_block(57, [tx, tx1]) @@ -779,7 +779,7 @@ def run_test(self): # b57p2 - a good block with 6 tx'es, don't submit until end self.move_tip(55) - b57p2 = self.next_block("57p2") + self.next_block("57p2") tx = self.create_and_sign_transaction(out[16], 1) tx1 = self.create_tx(tx, 0, 1) tx2 = self.create_tx(tx1, 0, 1) @@ -813,7 +813,7 @@ def run_test(self): # tx with prevout.n out of range self.log.info("Reject a block with a transaction with prevout.n out of range") self.move_tip(57) - b58 = self.next_block(58, spend=out[17]) + self.next_block(58, spend=out[17]) tx = CTransaction() assert len(out[17].vout) < 42 tx.vin.append(CTxIn(COutPoint(out[17].sha256, 42), CScript([OP_TRUE]), SEQUENCE_FINAL)) @@ -825,7 +825,7 @@ def run_test(self): # tx with output value > input value self.log.info("Reject a block with a transaction with outputs > inputs") self.move_tip(57) - b59 = self.next_block(59) + self.next_block(59) tx = self.create_and_sign_transaction(out[17], 510 * COIN) b59 = self.update_block(59, [tx]) self.send_blocks([b59], success=False, reject_reason='bad-txns-in-belowout', reconnect=True) @@ -861,7 +861,7 @@ def run_test(self): # \-> b_spend_dup_cb (b_dup_cb) -> b_dup_2 () # self.move_tip(57) - b_spend_dup_cb = self.next_block('spend_dup_cb') + self.next_block('spend_dup_cb') tx = CTransaction() tx.vin.append(CTxIn(COutPoint(duplicate_tx.sha256, 0))) tx.vout.append(CTxOut(0, CScript([OP_TRUE]))) @@ -886,7 +886,7 @@ def run_test(self): # self.log.info("Reject a block with a transaction with a nonfinal locktime") self.move_tip('dup_2') - b62 = self.next_block(62) + self.next_block(62) tx = CTransaction() tx.nLockTime = 0xffffffff # this locktime is non-final tx.vin.append(CTxIn(COutPoint(out[18].sha256, 0))) # don't set nSequence @@ -967,7 +967,7 @@ def run_test(self): # self.log.info("Accept a block with a transaction spending an output created in the same block") self.move_tip(64) - b65 = self.next_block(65) + self.next_block(65) tx1 = self.create_and_sign_transaction(out[19], out[19].vout[0].nValue) tx2 = self.create_and_sign_transaction(tx1, 0) b65 = self.update_block(65, [tx1, tx2]) @@ -980,7 +980,7 @@ def run_test(self): # \-> b66 (20) self.log.info("Reject a block with a transaction spending an output created later in the same block") self.move_tip(65) - b66 = self.next_block(66) + self.next_block(66) tx1 = self.create_and_sign_transaction(out[20], out[20].vout[0].nValue) tx2 = self.create_and_sign_transaction(tx1, 1) b66 = self.update_block(66, [tx2, tx1]) @@ -994,7 +994,7 @@ def run_test(self): # self.log.info("Reject a block with a transaction double spending a transaction created in the same block") self.move_tip(65) - b67 = self.next_block(67) + self.next_block(67) tx1 = self.create_and_sign_transaction(out[20], out[20].vout[0].nValue) tx2 = self.create_and_sign_transaction(tx1, 1) tx3 = self.create_and_sign_transaction(tx1, 2) @@ -1015,7 +1015,7 @@ def run_test(self): # self.log.info("Reject a block trying to claim too much subsidy in the coinbase transaction") self.move_tip(65) - b68 = self.next_block(68, additional_coinbase_value=10) + self.next_block(68, additional_coinbase_value=10) tx = self.create_and_sign_transaction(out[20], out[20].vout[0].nValue - 9) b68 = self.update_block(68, [tx]) self.send_blocks([b68], success=False, reject_reason='bad-cb-amount', reconnect=False) @@ -1035,7 +1035,7 @@ def run_test(self): # self.log.info("Reject a block containing a transaction spending from a non-existent input") self.move_tip(69) - b70 = self.next_block(70, spend=out[21]) + self.next_block(70, spend=out[21]) bogus_tx = CTransaction() bogus_tx.sha256 = uint256_from_str(b"23c70ed7c0506e9178fc1a987f40a33946d4ad4c962b5ae3a52546da53af0c5c") tx = CTransaction() @@ -1053,7 +1053,7 @@ def run_test(self): # b71 is a copy of 72, but re-adds one of its transactions. However, it has the same hash as b72. self.log.info("Reject a block containing a duplicate transaction but with the same Merkle root (Merkle tree malleability") self.move_tip(69) - b72 = self.next_block(72) + self.next_block(72) tx1 = self.create_and_sign_transaction(out[21], 2) tx2 = self.create_and_sign_transaction(tx1, 1) b72 = self.update_block(72, [tx1, tx2]) # now tip is 72 @@ -1091,7 +1091,7 @@ def run_test(self): # bytearray[20,526] : OP_CHECKSIG (this puts us over the limit) self.log.info("Reject a block containing too many sigops after a large script element") self.move_tip(72) - b73 = self.next_block(73) + self.next_block(73) size = MAX_BLOCK_SIGOPS - 1 + MAX_SCRIPT_ELEMENT_SIZE + 1 + 5 + 1 a = bytearray([OP_CHECKSIG] * size) a[MAX_BLOCK_SIGOPS - 1] = int("4e", 16) # OP_PUSHDATA4 @@ -1119,7 +1119,7 @@ def run_test(self): # b75 succeeds because we put MAX_BLOCK_SIGOPS before the element self.log.info("Check sigops are counted correctly after an invalid script element") self.move_tip(72) - b74 = self.next_block(74) + self.next_block(74) size = MAX_BLOCK_SIGOPS - 1 + MAX_SCRIPT_ELEMENT_SIZE + 42 # total = 20,561 a = bytearray([OP_CHECKSIG] * size) a[MAX_BLOCK_SIGOPS] = 0x4e @@ -1132,7 +1132,7 @@ def run_test(self): self.send_blocks([b74], success=False, reject_reason='bad-blk-sigops', reconnect=True) self.move_tip(72) - b75 = self.next_block(75) + self.next_block(75) size = MAX_BLOCK_SIGOPS - 1 + MAX_SCRIPT_ELEMENT_SIZE + 42 a = bytearray([OP_CHECKSIG] * size) a[MAX_BLOCK_SIGOPS - 1] = 0x4e @@ -1147,7 +1147,7 @@ def run_test(self): # Check that if we push an element filled with CHECKSIGs, they are not counted self.move_tip(75) - b76 = self.next_block(76) + self.next_block(76) size = MAX_BLOCK_SIGOPS - 1 + MAX_SCRIPT_ELEMENT_SIZE + 1 + 5 a = bytearray([OP_CHECKSIG] * size) a[MAX_BLOCK_SIGOPS - 1] = 0x4e # PUSHDATA4, but leave the following bytes as just checksigs @@ -1175,18 +1175,18 @@ def run_test(self): # updated. (Perhaps to spend to a P2SH OP_TRUE script) self.log.info("Test transaction resurrection during a re-org") self.move_tip(76) - b77 = self.next_block(77) + self.next_block(77) tx77 = self.create_and_sign_transaction(out[24], 10 * COIN) b77 = self.update_block(77, [tx77]) self.send_blocks([b77], True) self.save_spendable_output() - b78 = self.next_block(78) + self.next_block(78) tx78 = self.create_tx(tx77, 0, 9 * COIN) b78 = self.update_block(78, [tx78]) self.send_blocks([b78], True) - b79 = self.next_block(79) + self.next_block(79) tx79 = self.create_tx(tx78, 0, 8 * COIN) b79 = self.update_block(79, [tx79]) self.send_blocks([b79], True) @@ -1218,7 +1218,7 @@ def run_test(self): # -> b81 (26) -> b82 (27) -> b83 (28) # self.log.info("Accept a block with invalid opcodes in dead execution paths") - b83 = self.next_block(83) + self.next_block(83) op_codes = [OP_IF, OP_INVALIDOPCODE, OP_ELSE, OP_TRUE, OP_ENDIF] script = CScript(op_codes) tx1 = self.create_and_sign_transaction(out[28], out[28].vout[0].nValue, script) @@ -1237,7 +1237,7 @@ def run_test(self): # \-> b85 (29) -> b86 (30) \-> b89a (32) # self.log.info("Test re-orging blocks with OP_RETURN in them") - b84 = self.next_block(84) + self.next_block(84) tx1 = self.create_tx(out[29], 0, 0, CScript([OP_RETURN])) tx1.vout.append(CTxOut(0, CScript([OP_TRUE]))) tx1.vout.append(CTxOut(0, CScript([OP_TRUE]))) @@ -1275,7 +1275,7 @@ def run_test(self): self.save_spendable_output() # trying to spend the OP_RETURN output is rejected - b89a = self.next_block("89a", spend=out[32]) + self.next_block("89a", spend=out[32]) tx = self.create_tx(tx1, 0, 0, CScript([OP_TRUE])) b89a = self.update_block("89a", [tx]) self.send_blocks([b89a], success=False, reject_reason='bad-txns-inputs-missingorspent', reconnect=True) diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py index 35c47a12c06a..9ffdcb63bba1 100755 --- a/test/functional/p2p_disconnect_ban.py +++ b/test/functional/p2p_disconnect_ban.py @@ -3,6 +3,7 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test node disconnect and ban behavior""" +from pathlib import Path from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -35,6 +36,17 @@ def run_test(self): self.log.info("clearbanned: successfully clear ban list") self.nodes[1].clearbanned() assert_equal(len(self.nodes[1].listbanned()), 0) + + self.log.info('Test banlist database recreation') + self.stop_node(1) + target_file = self.nodes[1].chain_path / "banlist.json" + Path.unlink(target_file) + with self.nodes[1].assert_debug_log(["Recreating the banlist database"]): + self.start_node(1) + + assert Path.exists(target_file) + assert_equal(self.nodes[1].listbanned(), []) + self.nodes[1].setban("127.0.0.0/24", "add") self.log.info("setban: fail to ban an already banned subnet") diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 747f47c6e2de..7077764a3733 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -501,5 +501,8 @@ def test_psbt_input_keys(psbt_input, keys): assert signed['complete'] self.nodes[0].finalizepsbt(signed['psbt']) + self.log.info("Test we don't crash when making a 0-value funded transaction at 0 fee without forcing an input selection") + assert_raises_rpc_error(-4, "Transaction requires one destination of non-0 value, a non-0 feerate, or a pre-selected input", self.nodes[0].walletcreatefundedpsbt, [], [{"data": "deadbeef"}], 0, {"fee_rate": "0"}) + if __name__ == '__main__': PSBTTest().main() diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index 88880a3d8b5e..c00dde7745c1 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -129,6 +129,13 @@ def run_test(self): assert_equal(tx['confirmations'], 0) assert_equal(tx['trusted'], False) + self.log.info("Check abandoned field is present for all transaction categories in listtransactions") + txAB1_listtransactions = [d for d in self.nodes[0].listtransactions() if d['txid'] == txAB1] + assert len(txAB1_listtransactions) > 0, "Should have transactions for abandoned txid" + for tx in txAB1_listtransactions: + assert 'abandoned' in tx, f"abandoned field should be present for category '{tx['category']}'" + assert_equal(tx['abandoned'], True) + # Verify that even with a low min relay fee, the tx is not reaccepted from wallet on startup once abandoned self.restart_node(0, extra_args=["-minrelaytxfee=0.00001"]) assert self.nodes[0].getmempoolinfo()['loaded'] diff --git a/test/functional/wallet_encryption.py b/test/functional/wallet_encryption.py index 37a79640664d..fb1459b6808f 100755 --- a/test/functional/wallet_encryption.py +++ b/test/functional/wallet_encryption.py @@ -87,6 +87,17 @@ def run_test(self): self.nodes[0].walletpassphrase(passphrase2, MAX_VALUE + 1000) actual_time = self.nodes[0].getwalletinfo()['unlocked_until'] assert_equal(actual_time, expected_time) + self.nodes[0].walletlock() + + # Test passphrase with null characters + passphrase_with_nulls = "Phrase\0With\0Nulls" + self.nodes[0].walletpassphrasechange(passphrase2, passphrase_with_nulls) + # walletpassphrasechange should not stop at null characters + assert_raises_rpc_error(-14, "wallet passphrase entered was incorrect", self.nodes[0].walletpassphrase, passphrase_with_nulls.partition("\0")[0], 10) + self.nodes[0].walletpassphrase(passphrase_with_nulls, 10) + sig = self.nodes[0].signmessage(address, msg) + assert self.nodes[0].verifymessage(address, sig, msg) + self.nodes[0].walletlock() if __name__ == '__main__': diff --git a/test/functional/wallet_listdescriptors.py b/test/functional/wallet_listdescriptors.py index e9295c2bfbd0..28de6b2e313b 100755 --- a/test/functional/wallet_listdescriptors.py +++ b/test/functional/wallet_listdescriptors.py @@ -51,7 +51,7 @@ def run_test(self): assert_equal(1, len([d for d in result['descriptors'] if d['internal']])) for item in result['descriptors']: assert item['desc'] != '' - assert item['next'] == 0 + assert item['next_index'] == 0 assert item['range'] == [0, 0] assert item['timestamp'] is not None @@ -71,7 +71,8 @@ def run_test(self): 'timestamp': TIME_GENESIS_BLOCK, 'active': False, 'range': [0, 0], - 'next': 0}, + 'next': 0, + 'next_index': 0}, ], } assert_equal(expected, wallet.listdescriptors()) @@ -85,7 +86,8 @@ def run_test(self): 'timestamp': TIME_GENESIS_BLOCK, 'active': False, 'range': [0, 0], - 'next': 0}, + 'next': 0, + 'next_index': 0}, ], } assert_equal(expected_private, wallet.listdescriptors(True))