diff options
Diffstat (limited to 'src')
61 files changed, 915 insertions, 378 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index 0b25ef9c6b..b4ff556eb6 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -143,6 +143,7 @@ BITCOIN_CORE_H = \ compat/compat.h \ compat/cpuid.h \ compat/endian.h \ + common/settings.h \ common/system.h \ compressor.h \ consensus/consensus.h \ @@ -309,7 +310,6 @@ BITCOIN_CORE_H = \ util/readwritefile.h \ util/result.h \ util/serfloat.h \ - util/settings.h \ util/sock.h \ util/spanparsing.h \ util/string.h \ @@ -663,6 +663,7 @@ libbitcoin_common_a_SOURCES = \ common/init.cpp \ common/interfaces.cpp \ common/run_command.cpp \ + common/settings.cpp \ common/system.cpp \ compressor.cpp \ core_read.cpp \ @@ -733,7 +734,6 @@ libbitcoin_util_a_SOURCES = \ util/moneystr.cpp \ util/rbf.cpp \ util/readwritefile.cpp \ - util/settings.cpp \ util/thread.cpp \ util/threadinterrupt.cpp \ util/threadnames.cpp \ @@ -912,12 +912,8 @@ libbitcoinkernel_la_SOURCES = \ kernel/bitcoinkernel.cpp \ arith_uint256.cpp \ chain.cpp \ - chainparamsbase.cpp \ - chainparams.cpp \ clientversion.cpp \ coins.cpp \ - common/args.cpp \ - common/config.cpp \ compressor.cpp \ consensus/merkle.cpp \ consensus/tx_check.cpp \ @@ -978,7 +974,6 @@ libbitcoinkernel_la_SOURCES = \ util/moneystr.cpp \ util/rbf.cpp \ util/serfloat.cpp \ - util/settings.cpp \ util/strencodings.cpp \ util/string.cpp \ util/syscall_sandbox.cpp \ diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 23f9600ea5..cb1c49050e 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -9,6 +9,7 @@ #include <chainparams.h> #include <clientversion.h> #include <common/args.h> +#include <common/settings.h> #include <cstdint> #include <hash.h> #include <logging.h> @@ -21,7 +22,6 @@ #include <univalue.h> #include <util/fs.h> #include <util/fs_helpers.h> -#include <util/settings.h> #include <util/translation.h> namespace { @@ -132,7 +132,7 @@ CBanDB::CBanDB(fs::path ban_list_path) bool CBanDB::Write(const banmap_t& banSet) { std::vector<std::string> errors; - if (util::WriteSettings(m_banlist_json, {{JSON_KEY, BanMapToJson(banSet)}}, errors)) { + if (common::WriteSettings(m_banlist_json, {{JSON_KEY, BanMapToJson(banSet)}}, errors)) { return true; } @@ -152,10 +152,10 @@ bool CBanDB::Read(banmap_t& banSet) return false; } - std::map<std::string, util::SettingsValue> settings; + std::map<std::string, common::SettingsValue> settings; std::vector<std::string> errors; - if (!util::ReadSettings(m_banlist_json, settings, errors)) { + if (!common::ReadSettings(m_banlist_json, settings, errors)) { for (const auto& err : errors) { LogPrintf("Cannot load banlist %s: %s\n", fs::PathToString(m_banlist_json), err); } diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 5678d4a526..432bdc8e33 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -17,8 +17,6 @@ #include <kernel/context.h> #include <kernel/validation_cache_sizes.h> -#include <chainparams.h> -#include <common/args.h> #include <consensus/validation.h> #include <core_io.h> #include <node/blockstorage.h> @@ -53,13 +51,9 @@ int main(int argc, char* argv[]) } std::filesystem::path abs_datadir = std::filesystem::absolute(argv[1]); std::filesystem::create_directories(abs_datadir); - gArgs.ForceSetArg("-datadir", abs_datadir.string()); - // SETUP: Misc Globals - SelectParams(ChainType::MAIN); - auto chainparams = CChainParams::Main(); - + // SETUP: Context kernel::Context kernel_context{}; // We can't use a goto here, but we can use an assert since none of the // things instantiated so far requires running the epilogue to be torn down @@ -106,16 +100,18 @@ int main(int argc, char* argv[]) }; auto notifications = std::make_unique<KernelNotifications>(); + // SETUP: Chainstate + auto chainparams = CChainParams::Main(); const ChainstateManager::Options chainman_opts{ .chainparams = *chainparams, - .datadir = gArgs.GetDataDirNet(), + .datadir = abs_datadir, .adjusted_time_callback = NodeClock::now, .notifications = *notifications, }; const node::BlockManager::Options blockman_opts{ .chainparams = chainman_opts.chainparams, - .blocks_dir = gArgs.GetBlocksDirPath(), + .blocks_dir = abs_datadir / "blocks", }; ChainstateManager chainman{chainman_opts, blockman_opts}; @@ -148,7 +144,8 @@ int main(int argc, char* argv[]) // Main program logic starts here std::cout << "Hello! I'm going to print out some information about your datadir." << std::endl - << "\t" << "Path: " << gArgs.GetDataDirNet() << std::endl; + << "\t" + << "Path: " << abs_datadir << std::endl; { LOCK(chainman.GetMutex()); std::cout diff --git a/src/common/args.cpp b/src/common/args.cpp index c9af2d7f5e..643838399f 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -6,6 +6,7 @@ #include <common/args.h> #include <chainparamsbase.h> +#include <common/settings.h> #include <logging.h> #include <sync.h> #include <tinyformat.h> @@ -14,7 +15,6 @@ #include <util/check.h> #include <util/fs.h> #include <util/fs_helpers.h> -#include <util/settings.h> #include <util/strencodings.h> #ifdef WIN32 @@ -104,7 +104,7 @@ KeyInfo InterpretKey(std::string key) * @return parsed settings value if it is valid, otherwise nullopt accompanied * by a descriptive error string */ -std::optional<util::SettingsValue> InterpretValue(const KeyInfo& key, const std::string* value, +std::optional<common::SettingsValue> InterpretValue(const KeyInfo& key, const std::string* value, unsigned int flags, std::string& error) { // Return negated settings as false values. @@ -238,15 +238,15 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin return false; } - std::optional<util::SettingsValue> value = InterpretValue(keyinfo, val ? &*val : nullptr, *flags, error); + std::optional<common::SettingsValue> value = InterpretValue(keyinfo, val ? &*val : nullptr, *flags, error); if (!value) return false; m_settings.command_line_options[keyinfo.name].push_back(*value); } // we do not allow -includeconf from command line, only -noincludeconf - if (auto* includes = util::FindKey(m_settings.command_line_options, "includeconf")) { - const util::SettingsSpan values{*includes}; + if (auto* includes = common::FindKey(m_settings.command_line_options, "includeconf")) { + const common::SettingsSpan values{*includes}; // Range may be empty if -noincludeconf was passed if (!values.empty()) { error = "-includeconf cannot be used from commandline; -includeconf=" + values.begin()->write(); @@ -361,7 +361,7 @@ std::optional<const ArgsManager::Command> ArgsManager::GetCommand() const std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const { std::vector<std::string> result; - for (const util::SettingsValue& value : GetSettingsList(strArg)) { + for (const common::SettingsValue& value : GetSettingsList(strArg)) { result.push_back(value.isFalse() ? "0" : value.isTrue() ? "1" : value.get_str()); } return result; @@ -408,7 +408,7 @@ bool ArgsManager::ReadSettingsFile(std::vector<std::string>* errors) LOCK(cs_args); m_settings.rw_settings.clear(); std::vector<std::string> read_errors; - if (!util::ReadSettings(path, m_settings.rw_settings, read_errors)) { + if (!common::ReadSettings(path, m_settings.rw_settings, read_errors)) { SaveErrors(read_errors, errors); return false; } @@ -430,7 +430,7 @@ bool ArgsManager::WriteSettingsFile(std::vector<std::string>* errors, bool backu LOCK(cs_args); std::vector<std::string> write_errors; - if (!util::WriteSettings(path_tmp, m_settings.rw_settings, write_errors)) { + if (!common::WriteSettings(path_tmp, m_settings.rw_settings, write_errors)) { SaveErrors(write_errors, errors); return false; } @@ -441,10 +441,10 @@ bool ArgsManager::WriteSettingsFile(std::vector<std::string>* errors, bool backu return true; } -util::SettingsValue ArgsManager::GetPersistentSetting(const std::string& name) const +common::SettingsValue ArgsManager::GetPersistentSetting(const std::string& name) const { LOCK(cs_args); - return util::GetSetting(m_settings, m_network, name, !UseDefaultSection("-" + name), + return common::GetSetting(m_settings, m_network, name, !UseDefaultSection("-" + name), /*ignore_nonpersistent=*/true, /*get_chain_type=*/false); } @@ -460,11 +460,11 @@ std::string ArgsManager::GetArg(const std::string& strArg, const std::string& st std::optional<std::string> ArgsManager::GetArg(const std::string& strArg) const { - const util::SettingsValue value = GetSetting(strArg); + const common::SettingsValue value = GetSetting(strArg); return SettingToString(value); } -std::optional<std::string> SettingToString(const util::SettingsValue& value) +std::optional<std::string> SettingToString(const common::SettingsValue& value) { if (value.isNull()) return std::nullopt; if (value.isFalse()) return "0"; @@ -473,7 +473,7 @@ std::optional<std::string> SettingToString(const util::SettingsValue& value) return value.get_str(); } -std::string SettingToString(const util::SettingsValue& value, const std::string& strDefault) +std::string SettingToString(const common::SettingsValue& value, const std::string& strDefault) { return SettingToString(value).value_or(strDefault); } @@ -485,11 +485,11 @@ int64_t ArgsManager::GetIntArg(const std::string& strArg, int64_t nDefault) cons std::optional<int64_t> ArgsManager::GetIntArg(const std::string& strArg) const { - const util::SettingsValue value = GetSetting(strArg); + const common::SettingsValue value = GetSetting(strArg); return SettingToInt(value); } -std::optional<int64_t> SettingToInt(const util::SettingsValue& value) +std::optional<int64_t> SettingToInt(const common::SettingsValue& value) { if (value.isNull()) return std::nullopt; if (value.isFalse()) return 0; @@ -498,7 +498,7 @@ std::optional<int64_t> SettingToInt(const util::SettingsValue& value) return LocaleIndependentAtoi<int64_t>(value.get_str()); } -int64_t SettingToInt(const util::SettingsValue& value, int64_t nDefault) +int64_t SettingToInt(const common::SettingsValue& value, int64_t nDefault) { return SettingToInt(value).value_or(nDefault); } @@ -510,18 +510,18 @@ bool ArgsManager::GetBoolArg(const std::string& strArg, bool fDefault) const std::optional<bool> ArgsManager::GetBoolArg(const std::string& strArg) const { - const util::SettingsValue value = GetSetting(strArg); + const common::SettingsValue value = GetSetting(strArg); return SettingToBool(value); } -std::optional<bool> SettingToBool(const util::SettingsValue& value) +std::optional<bool> SettingToBool(const common::SettingsValue& value) { if (value.isNull()) return std::nullopt; if (value.isBool()) return value.get_bool(); return InterpretBool(value.get_str()); } -bool SettingToBool(const util::SettingsValue& value, bool fDefault) +bool SettingToBool(const common::SettingsValue& value, bool fDefault) { return SettingToBool(value).value_or(fDefault); } @@ -738,7 +738,7 @@ std::variant<ChainType, std::string> ArgsManager::GetChainArg() const { auto get_net = [&](const std::string& arg) { LOCK(cs_args); - util::SettingsValue value = util::GetSetting(m_settings, /* section= */ "", SettingName(arg), + common::SettingsValue value = common::GetSetting(m_settings, /* section= */ "", SettingName(arg), /* ignore_default_section_config= */ false, /*ignore_nonpersistent=*/false, /* get_chain_type= */ true); @@ -769,24 +769,24 @@ bool ArgsManager::UseDefaultSection(const std::string& arg) const return m_network == ChainTypeToString(ChainType::MAIN) || m_network_only_args.count(arg) == 0; } -util::SettingsValue ArgsManager::GetSetting(const std::string& arg) const +common::SettingsValue ArgsManager::GetSetting(const std::string& arg) const { LOCK(cs_args); - return util::GetSetting( + return common::GetSetting( m_settings, m_network, SettingName(arg), !UseDefaultSection(arg), /*ignore_nonpersistent=*/false, /*get_chain_type=*/false); } -std::vector<util::SettingsValue> ArgsManager::GetSettingsList(const std::string& arg) const +std::vector<common::SettingsValue> ArgsManager::GetSettingsList(const std::string& arg) const { LOCK(cs_args); - return util::GetSettingsList(m_settings, m_network, SettingName(arg), !UseDefaultSection(arg)); + return common::GetSettingsList(m_settings, m_network, SettingName(arg), !UseDefaultSection(arg)); } void ArgsManager::logArgsPrefix( const std::string& prefix, const std::string& section, - const std::map<std::string, std::vector<util::SettingsValue>>& args) const + const std::map<std::string, std::vector<common::SettingsValue>>& args) const { std::string section_str = section.empty() ? "" : "[" + section + "] "; for (const auto& arg : args) { diff --git a/src/common/args.h b/src/common/args.h index 7569297a74..ae3ed02bc7 100644 --- a/src/common/args.h +++ b/src/common/args.h @@ -5,11 +5,11 @@ #ifndef BITCOIN_COMMON_ARGS_H #define BITCOIN_COMMON_ARGS_H +#include <common/settings.h> #include <compat/compat.h> #include <sync.h> #include <util/chaintype.h> #include <util/fs.h> -#include <util/settings.h> #include <iosfwd> #include <list> @@ -75,7 +75,7 @@ struct KeyInfo { KeyInfo InterpretKey(std::string key); -std::optional<util::SettingsValue> InterpretValue(const KeyInfo& key, const std::string* value, +std::optional<common::SettingsValue> InterpretValue(const KeyInfo& key, const std::string* value, unsigned int flags, std::string& error); struct SectionInfo { @@ -84,14 +84,14 @@ struct SectionInfo { int m_line; }; -std::string SettingToString(const util::SettingsValue&, const std::string&); -std::optional<std::string> SettingToString(const util::SettingsValue&); +std::string SettingToString(const common::SettingsValue&, const std::string&); +std::optional<std::string> SettingToString(const common::SettingsValue&); -int64_t SettingToInt(const util::SettingsValue&, int64_t); -std::optional<int64_t> SettingToInt(const util::SettingsValue&); +int64_t SettingToInt(const common::SettingsValue&, int64_t); +std::optional<int64_t> SettingToInt(const common::SettingsValue&); -bool SettingToBool(const util::SettingsValue&, bool); -std::optional<bool> SettingToBool(const util::SettingsValue&); +bool SettingToBool(const common::SettingsValue&, bool); +std::optional<bool> SettingToBool(const common::SettingsValue&); class ArgsManager { @@ -130,7 +130,7 @@ protected: }; mutable RecursiveMutex cs_args; - util::Settings m_settings GUARDED_BY(cs_args); + common::Settings m_settings GUARDED_BY(cs_args); std::vector<std::string> m_command GUARDED_BY(cs_args); std::string m_network GUARDED_BY(cs_args); std::set<std::string> m_network_only_args GUARDED_BY(cs_args); @@ -159,12 +159,12 @@ protected: * false if "-nosetting" argument was passed, and a string if a "-setting=value" * argument was passed. */ - util::SettingsValue GetSetting(const std::string& arg) const; + common::SettingsValue GetSetting(const std::string& arg) const; /** * Get list of setting values. */ - std::vector<util::SettingsValue> GetSettingsList(const std::string& arg) const; + std::vector<common::SettingsValue> GetSettingsList(const std::string& arg) const; ArgsManager(); ~ArgsManager(); @@ -394,7 +394,7 @@ protected: * Get current setting from config file or read/write settings file, * ignoring nonpersistent command line or forced settings values. */ - util::SettingsValue GetPersistentSetting(const std::string& name) const; + common::SettingsValue GetPersistentSetting(const std::string& name) const; /** * Access settings with lock held. @@ -433,7 +433,7 @@ private: void logArgsPrefix( const std::string& prefix, const std::string& section, - const std::map<std::string, std::vector<util::SettingsValue>>& args) const; + const std::map<std::string, std::vector<common::SettingsValue>>& args) const; }; extern ArgsManager gArgs; diff --git a/src/common/config.cpp b/src/common/config.cpp index e25b4fe2df..1c85273f69 100644 --- a/src/common/config.cpp +++ b/src/common/config.cpp @@ -4,13 +4,13 @@ #include <common/args.h> +#include <common/settings.h> #include <logging.h> #include <sync.h> #include <tinyformat.h> #include <univalue.h> #include <util/chaintype.h> #include <util/fs.h> -#include <util/settings.h> #include <util/string.h> #include <algorithm> @@ -98,7 +98,7 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file std::optional<unsigned int> flags = GetArgFlags('-' + key.name); if (!IsConfSupported(key, error)) return false; if (flags) { - std::optional<util::SettingsValue> value = InterpretValue(key, &option.second, *flags, error); + std::optional<common::SettingsValue> value = InterpretValue(key, &option.second, *flags, error); if (!value) { return false; } @@ -142,9 +142,9 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) bool use_conf_file{true}; { LOCK(cs_args); - if (auto* includes = util::FindKey(m_settings.command_line_options, "includeconf")) { + if (auto* includes = common::FindKey(m_settings.command_line_options, "includeconf")) { // ParseParameters() fails if a non-negated -includeconf is passed on the command-line - assert(util::SettingsSpan(*includes).last_negated()); + assert(common::SettingsSpan(*includes).last_negated()); use_conf_file = false; } } @@ -155,9 +155,9 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) auto add_includes = [&](const std::string& network, size_t skip = 0) { size_t num_values = 0; LOCK(cs_args); - if (auto* section = util::FindKey(m_settings.ro_config, network)) { - if (auto* values = util::FindKey(*section, "includeconf")) { - for (size_t i = std::max(skip, util::SettingsSpan(*values).negated()); i < values->size(); ++i) { + if (auto* section = common::FindKey(m_settings.ro_config, network)) { + if (auto* values = common::FindKey(*section, "includeconf")) { + for (size_t i = std::max(skip, common::SettingsSpan(*values).negated()); i < values->size(); ++i) { conf_file_names.push_back((*values)[i].get_str()); } num_values = values->size(); diff --git a/src/util/settings.cpp b/src/common/settings.cpp index db3d60046e..9187f242eb 100644 --- a/src/util/settings.cpp +++ b/src/common/settings.cpp @@ -2,18 +2,21 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <util/fs.h> -#include <util/settings.h> +#include <common/settings.h> #include <tinyformat.h> #include <univalue.h> +#include <util/fs.h> +#include <algorithm> #include <fstream> +#include <iterator> #include <map> #include <string> +#include <utility> #include <vector> -namespace util { +namespace common { namespace { enum class Source { @@ -255,4 +258,4 @@ size_t SettingsSpan::negated() const return 0; } -} // namespace util +} // namespace common diff --git a/src/util/settings.h b/src/common/settings.h index bb1fe585e1..0e9d376e23 100644 --- a/src/util/settings.h +++ b/src/common/settings.h @@ -2,18 +2,19 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#ifndef BITCOIN_UTIL_SETTINGS_H -#define BITCOIN_UTIL_SETTINGS_H +#ifndef BITCOIN_COMMON_SETTINGS_H +#define BITCOIN_COMMON_SETTINGS_H #include <util/fs.h> +#include <cstddef> #include <map> #include <string> #include <vector> class UniValue; -namespace util { +namespace common { //! Settings value type (string/integer/boolean/null variant). //! @@ -109,6 +110,6 @@ auto FindKey(Map&& map, Key&& key) -> decltype(&map.at(key)) return it == map.end() ? nullptr : &it->second; } -} // namespace util +} // namespace common -#endif // BITCOIN_UTIL_SETTINGS_H +#endif // BITCOIN_COMMON_SETTINGS_H diff --git a/src/index/base.cpp b/src/index/base.cpp index 7444579395..3f91910db2 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -103,15 +103,12 @@ bool BaseIndex::Init() SetBestBlockIndex(locator_index); } - // Note: this will latch to true immediately if the user starts up with an empty - // datadir and an index enabled. If this is the case, indexation will happen solely - // via `BlockConnected` signals until, possibly, the next restart. - m_synced = m_best_block_index.load() == active_chain.Tip(); - // Skip pruning check if indexes are not ready to sync (because reindex-chainstate has wiped the chain). - if (!m_synced && g_indexes_ready_to_sync) { + const CBlockIndex* start_block = m_best_block_index.load(); + bool synced = start_block == active_chain.Tip(); + if (!synced && g_indexes_ready_to_sync) { bool prune_violation = false; - if (!m_best_block_index) { + if (!start_block) { // index is not built yet // make sure we have all block data back to the genesis prune_violation = m_chainstate->m_blockman.GetFirstStoredBlock(*active_chain.Tip()) != active_chain.Genesis(); @@ -119,7 +116,7 @@ bool BaseIndex::Init() // in case the index has a best block set and is not fully synced // check if we have the required blocks to continue building the index else { - const CBlockIndex* block_to_test = m_best_block_index.load(); + const CBlockIndex* block_to_test = start_block; if (!active_chain.Contains(block_to_test)) { // if the bestblock is not part of the mainchain, find the fork // and make sure we have all data down to the fork @@ -143,6 +140,16 @@ bool BaseIndex::Init() return InitError(strprintf(Untranslated("%s best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)"), GetName())); } } + + // Child init + if (!CustomInit(start_block ? std::make_optional(interfaces::BlockKey{start_block->GetBlockHash(), start_block->nHeight}) : std::nullopt)) { + return false; + } + + // Note: this will latch to true immediately if the user starts up with an empty + // datadir and an index enabled. If this is the case, indexation will happen solely + // via `BlockConnected` signals until, possibly, the next restart. + m_synced = synced; return true; } @@ -408,11 +415,6 @@ bool BaseIndex::Start() RegisterValidationInterface(this); if (!Init()) return false; - const CBlockIndex* index = m_best_block_index.load(); - if (!CustomInit(index ? std::make_optional(interfaces::BlockKey{index->GetBlockHash(), index->nHeight}) : std::nullopt)) { - return false; - } - m_thread_sync = std::thread(&util::TraceThread, GetName(), [this] { ThreadSync(); }); return true; } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 40bf0b680c..dd664165d3 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -6,8 +6,8 @@ #define BITCOIN_INTERFACES_CHAIN_H #include <blockfilter.h> +#include <common/settings.h> #include <primitives/transaction.h> // For CTransactionRef -#include <util/settings.h> // For util::SettingsValue #include <functional> #include <memory> @@ -300,17 +300,17 @@ public: virtual int rpcSerializationFlags() = 0; //! Get settings value. - virtual util::SettingsValue getSetting(const std::string& arg) = 0; + virtual common::SettingsValue getSetting(const std::string& arg) = 0; //! Get list of settings values. - virtual std::vector<util::SettingsValue> getSettingsList(const std::string& arg) = 0; + virtual std::vector<common::SettingsValue> getSettingsList(const std::string& arg) = 0; //! Return <datadir>/settings.json setting value. - virtual util::SettingsValue getRwSetting(const std::string& name) = 0; + virtual common::SettingsValue getRwSetting(const std::string& name) = 0; //! Write a setting to <datadir>/settings.json. Optionally just update the //! setting in memory and do not write the file. - virtual bool updateRwSetting(const std::string& name, const util::SettingsValue& value, bool write=true) = 0; + virtual bool updateRwSetting(const std::string& name, const common::SettingsValue& value, bool write=true) = 0; //! Synchronously send transactionAddedToMempool notifications about all //! current mempool transactions to the specified handler and return after diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 7e87d5a523..479c585b88 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -5,13 +5,13 @@ #ifndef BITCOIN_INTERFACES_NODE_H #define BITCOIN_INTERFACES_NODE_H +#include <common/settings.h> #include <consensus/amount.h> // For CAmount #include <net.h> // For NodeId #include <net_types.h> // For banmap_t #include <netaddress.h> // For Network #include <netbase.h> // For ConnectionDirection #include <support/allocators/secure.h> // For SecureString -#include <util/settings.h> // For util::SettingsValue #include <util/translation.h> #include <functional> @@ -103,14 +103,14 @@ public: virtual bool isSettingIgnored(const std::string& name) = 0; //! Return setting value from <datadir>/settings.json or bitcoin.conf. - virtual util::SettingsValue getPersistentSetting(const std::string& name) = 0; + virtual common::SettingsValue getPersistentSetting(const std::string& name) = 0; //! Update a setting in <datadir>/settings.json. - virtual void updateRwSetting(const std::string& name, const util::SettingsValue& value) = 0; + virtual void updateRwSetting(const std::string& name, const common::SettingsValue& value) = 0; //! Force a setting value to be applied, overriding any other configuration //! source, but not being persisted. - virtual void forceSetting(const std::string& name, const util::SettingsValue& value) = 0; + virtual void forceSetting(const std::string& name, const common::SettingsValue& value) = 0; //! Clear all settings in <datadir>/settings.json and store a backup of //! previous settings in <datadir>/settings.json.bak. diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index 917f7d226c..035a913d10 100644 --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -21,6 +21,7 @@ class CChainParams; static constexpr bool DEFAULT_CHECKPOINTS_ENABLED{true}; static constexpr auto DEFAULT_MAX_TIP_AGE{24h}; +static constexpr int DEFAULT_STOPATHEIGHT{0}; namespace kernel { @@ -45,6 +46,7 @@ struct ChainstateManagerOpts { DBOptions coins_db{}; CoinsViewOptions coins_view{}; Notifications& notifications; + int stop_at_height{DEFAULT_STOPATHEIGHT}; }; } // namespace kernel diff --git a/src/net.cpp b/src/net.cpp index a53835f999..282c8fb741 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1703,7 +1703,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) int nOutboundFullRelay = 0; int nOutboundBlockRelay = 0; int outbound_privacy_network_peers = 0; - std::set<std::vector<unsigned char>> setConnected; // netgroups of our ipv4/ipv6 outbound peers + std::set<std::vector<unsigned char>> outbound_ipv46_peer_netgroups; { LOCK(m_nodes_mutex); @@ -1725,7 +1725,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) case ConnectionType::MANUAL: case ConnectionType::OUTBOUND_FULL_RELAY: case ConnectionType::BLOCK_RELAY: - CAddress address{pnode->addr}; + const CAddress address{pnode->addr}; if (address.IsTor() || address.IsI2P() || address.IsCJDNS()) { // Since our addrman-groups for these networks are // random, without relation to the route we @@ -1736,7 +1736,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) // these networks. ++outbound_privacy_network_peers; } else { - setConnected.insert(m_netgroupman.GetGroup(address)); + outbound_ipv46_peer_netgroups.insert(m_netgroupman.GetGroup(address)); } } // no default case, so the compiler can warn about missing cases } @@ -1811,7 +1811,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) m_anchors.pop_back(); if (!addr.IsValid() || IsLocal(addr) || !IsReachable(addr) || !HasAllDesirableServiceFlags(addr.nServices) || - setConnected.count(m_netgroupman.GetGroup(addr))) continue; + outbound_ipv46_peer_netgroups.count(m_netgroupman.GetGroup(addr))) continue; addrConnect = addr; LogPrint(BCLog::NET, "Trying to make an anchor connection to %s\n", addrConnect.ToStringAddrPort()); break; @@ -1851,8 +1851,8 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) std::tie(addr, addr_last_try) = addrman.Select(); } - // Require outbound connections, other than feelers, to be to distinct network groups - if (!fFeeler && setConnected.count(m_netgroupman.GetGroup(addr))) { + // Require outbound IPv4/IPv6 connections, other than feelers, to be to distinct network groups + if (!fFeeler && outbound_ipv46_peer_netgroups.count(m_netgroupman.GetGroup(addr))) { break; } @@ -1898,8 +1898,9 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) // Record addrman failure attempts when node has at least 2 persistent outbound connections to peers with // different netgroups in ipv4/ipv6 networks + all peers in Tor/I2P/CJDNS networks. // Don't record addrman failure attempts when node is offline. This can be identified since all local - // network connections(if any) belong in the same netgroup and size of setConnected would only be 1. - OpenNetworkConnection(addrConnect, (int)setConnected.size() + outbound_privacy_network_peers >= std::min(nMaxConnections - 1, 2), &grant, nullptr, conn_type); + // network connections (if any) belong in the same netgroup, and the size of `outbound_ipv46_peer_netgroups` would only be 1. + const bool count_failures{((int)outbound_ipv46_peer_netgroups.size() + outbound_privacy_network_peers) >= std::min(nMaxConnections - 1, 2)}; + OpenNetworkConnection(addrConnect, count_failures, &grant, /*strDest=*/nullptr, conn_type); } } } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e723d4657d..6597019797 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -51,9 +51,7 @@ #include <optional> #include <typeinfo> -/** How long to cache transactions in mapRelay for normal relay */ -static constexpr auto RELAY_TX_CACHE_TIME = 15min; -/** How long a transaction has to be in the mempool before it can unconditionally be relayed (even when not in mapRelay). */ +/** How long a transaction has to be in the mempool before it can unconditionally be relayed. */ static constexpr auto UNCONDITIONAL_RELAY_DELAY = 2min; /** Headers download timeout. * Timeout = base + per_header * (expected number of headers) */ @@ -851,6 +849,7 @@ private: std::shared_ptr<const CBlock> m_most_recent_block GUARDED_BY(m_most_recent_block_mutex); std::shared_ptr<const CBlockHeaderAndShortTxIDs> m_most_recent_compact_block GUARDED_BY(m_most_recent_block_mutex); uint256 m_most_recent_block_hash GUARDED_BY(m_most_recent_block_mutex); + std::unique_ptr<const std::map<uint256, CTransactionRef>> m_most_recent_block_txs GUARDED_BY(m_most_recent_block_mutex); // Data about the low-work headers synchronization, aggregated from all peers' HeadersSyncStates. /** Mutex guarding the other m_headers_presync_* variables. */ @@ -910,7 +909,7 @@ private: /** Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). */ CTransactionRef FindTxForGetData(const Peer::TxRelay& tx_relay, const GenTxid& gtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) - EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex, NetEventsInterface::g_msgproc_mutex); void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic<bool>& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex, peer.m_getdata_requests_mutex, NetEventsInterface::g_msgproc_mutex) @@ -919,12 +918,6 @@ private: /** Process a new block. Perform any post-processing housekeeping */ void ProcessBlock(CNode& node, const std::shared_ptr<const CBlock>& block, bool force_processing, bool min_pow_checked); - /** Relay map (txid or wtxid -> CTransactionRef) */ - typedef std::map<uint256, CTransactionRef> MapRelay; - MapRelay mapRelay GUARDED_BY(NetEventsInterface::g_msgproc_mutex); - /** Expiration-time ordered list of (expire time, relay map entry) pairs. */ - std::deque<std::pair<std::chrono::microseconds, MapRelay::iterator>> g_relay_expiration GUARDED_BY(NetEventsInterface::g_msgproc_mutex); - /** * When a peer sends us a valid block, instruct it to announce blocks to us * using CMPCTBLOCK if possible by adding its nodeid to the end of @@ -1927,10 +1920,17 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha std::async(std::launch::deferred, [&] { return msgMaker.Make(NetMsgType::CMPCTBLOCK, *pcmpctblock); })}; { + auto most_recent_block_txs = std::make_unique<std::map<uint256, CTransactionRef>>(); + for (const auto& tx : pblock->vtx) { + most_recent_block_txs->emplace(tx->GetHash(), tx); + most_recent_block_txs->emplace(tx->GetWitnessHash(), tx); + } + LOCK(m_most_recent_block_mutex); m_most_recent_block_hash = hashBlock; m_most_recent_block = pblock; m_most_recent_compact_block = pcmpctblock; + m_most_recent_block_txs = std::move(most_recent_block_txs); } m_connman.ForEachNode([this, pindex, &lazy_ser, &hashBlock](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { @@ -2301,13 +2301,17 @@ CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer::TxRelay& tx_relay, } } - // Otherwise, the transaction must have been announced recently. - if (tx_relay.m_recently_announced_invs.contains(gtxid.GetHash())) { - // If it was, it can be relayed from either the mempool... - if (txinfo.tx) return std::move(txinfo.tx); - // ... or the relay pool. - auto mi = mapRelay.find(gtxid.GetHash()); - if (mi != mapRelay.end()) return mi->second; + // Otherwise, the transaction might have been announced recently. + bool recent = tx_relay.m_recently_announced_invs.contains(gtxid.GetHash()); + if (recent && txinfo.tx) return std::move(txinfo.tx); + + // Or it might be from the most recent block + { + LOCK(m_most_recent_block_mutex); + if (m_most_recent_block_txs != nullptr) { + auto it = m_most_recent_block_txs->find(gtxid.GetHash()); + if (it != m_most_recent_block_txs->end()) return it->second; + } } return {}; @@ -3418,8 +3422,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // If the peer is old enough to have the old alert system, send it the final alert. if (greatest_common_version <= 70012) { - DataStream finalAlert{ParseHex("60010000000000000000000000ffffff7f00000000ffffff7ffeffff7f01ffffff7f00000000ffffff7f00ffffff7f002f555247454e543a20416c657274206b657920636f6d70726f6d697365642c2075706772616465207265717569726564004630440220653febd6410f470f6bae11cad19c48413becb1ac2c17f908fd0fd53bdc3abd5202206d0e9c96fe88d4a0f01ed9dedae2b6f9e00da94cad0fecaae66ecf689bf71b50")}; - m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version).Make("alert", finalAlert)); + const auto finalAlert{ParseHex("60010000000000000000000000ffffff7f00000000ffffff7ffeffff7f01ffffff7f00000000ffffff7f00ffffff7f002f555247454e543a20416c657274206b657920636f6d70726f6d697365642c2075706772616465207265717569726564004630440220653febd6410f470f6bae11cad19c48413becb1ac2c17f908fd0fd53bdc3abd5202206d0e9c96fe88d4a0f01ed9dedae2b6f9e00da94cad0fecaae66ecf689bf71b50")}; + m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version).Make("alert", Span{finalAlert})); } // Feeler connections exist only to verify if address is online. @@ -5778,7 +5782,6 @@ bool PeerManagerImpl::SendMessages(CNode* pto) continue; } auto txid = txinfo.tx->GetHash(); - auto wtxid = txinfo.tx->GetWitnessHash(); // Peer told you to not send transactions at that feerate? Don't bother sending it. if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) { continue; @@ -5788,24 +5791,6 @@ bool PeerManagerImpl::SendMessages(CNode* pto) tx_relay->m_recently_announced_invs.insert(hash); vInv.push_back(inv); nRelayedTransactions++; - { - // Expire old relay messages - while (!g_relay_expiration.empty() && g_relay_expiration.front().first < current_time) - { - mapRelay.erase(g_relay_expiration.front().second); - g_relay_expiration.pop_front(); - } - - auto ret = mapRelay.emplace(txid, std::move(txinfo.tx)); - if (ret.second) { - g_relay_expiration.emplace_back(current_time + RELAY_TX_CACHE_TIME, ret.first); - } - // Add wtxid-based lookup into mapRelay as well, so that peers can request by wtxid - auto ret2 = mapRelay.emplace(wtxid, ret.first->second); - if (ret2.second) { - g_relay_expiration.emplace_back(current_time + RELAY_TX_CACHE_TIME, ret2.first); - } - } if (vInv.size() == MAX_INV_SZ) { m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); vInv.clear(); diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp index 87d9238c18..a7f7303348 100644 --- a/src/node/chainstatemanager_args.cpp +++ b/src/node/chainstatemanager_args.cpp @@ -37,6 +37,8 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManage if (auto value{args.GetIntArg("-maxtipage")}) opts.max_tip_age = std::chrono::seconds{*value}; + if (auto value{args.GetIntArg("-stopatheight")}) opts.stop_at_height = *value; + ReadDatabaseArgs(args, opts.block_tree_db); ReadDatabaseArgs(args, opts.coins_db); ReadCoinsViewArgs(args, opts.coins_view); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 6e39ccf34e..57c0d302fd 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -125,17 +125,17 @@ public: bool isSettingIgnored(const std::string& name) override { bool ignored = false; - args().LockSettings([&](util::Settings& settings) { - if (auto* options = util::FindKey(settings.command_line_options, name)) { + args().LockSettings([&](common::Settings& settings) { + if (auto* options = common::FindKey(settings.command_line_options, name)) { ignored = !options->empty(); } }); return ignored; } - util::SettingsValue getPersistentSetting(const std::string& name) override { return args().GetPersistentSetting(name); } - void updateRwSetting(const std::string& name, const util::SettingsValue& value) override + common::SettingsValue getPersistentSetting(const std::string& name) override { return args().GetPersistentSetting(name); } + void updateRwSetting(const std::string& name, const common::SettingsValue& value) override { - args().LockSettings([&](util::Settings& settings) { + args().LockSettings([&](common::Settings& settings) { if (value.isNull()) { settings.rw_settings.erase(name); } else { @@ -144,9 +144,9 @@ public: }); args().WriteSettingsFile(); } - void forceSetting(const std::string& name, const util::SettingsValue& value) override + void forceSetting(const std::string& name, const common::SettingsValue& value) override { - args().LockSettings([&](util::Settings& settings) { + args().LockSettings([&](common::Settings& settings) { if (value.isNull()) { settings.forced_settings.erase(name); } else { @@ -157,7 +157,7 @@ public: void resetSettings() override { args().WriteSettingsFile(/*errors=*/nullptr, /*backup=*/true); - args().LockSettings([&](util::Settings& settings) { + args().LockSettings([&](common::Settings& settings) { settings.rw_settings.clear(); }); args().WriteSettingsFile(); @@ -744,27 +744,27 @@ public: RPCRunLater(name, std::move(fn), seconds); } int rpcSerializationFlags() override { return RPCSerializationFlags(); } - util::SettingsValue getSetting(const std::string& name) override + common::SettingsValue getSetting(const std::string& name) override { return args().GetSetting(name); } - std::vector<util::SettingsValue> getSettingsList(const std::string& name) override + std::vector<common::SettingsValue> getSettingsList(const std::string& name) override { return args().GetSettingsList(name); } - util::SettingsValue getRwSetting(const std::string& name) override + common::SettingsValue getRwSetting(const std::string& name) override { - util::SettingsValue result; - args().LockSettings([&](const util::Settings& settings) { - if (const util::SettingsValue* value = util::FindKey(settings.rw_settings, name)) { + common::SettingsValue result; + args().LockSettings([&](const common::Settings& settings) { + if (const common::SettingsValue* value = common::FindKey(settings.rw_settings, name)) { result = *value; } }); return result; } - bool updateRwSetting(const std::string& name, const util::SettingsValue& value, bool write) override + bool updateRwSetting(const std::string& name, const common::SettingsValue& value, bool write) override { - args().LockSettings([&](util::Settings& settings) { + args().LockSettings([&](common::Settings& settings) { if (value.isNull()) { settings.rw_settings.erase(name); } else { diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp index 71ae9d23c7..6f253eddfa 100644 --- a/src/node/mini_miner.cpp +++ b/src/node/mini_miner.cpp @@ -346,15 +346,20 @@ std::optional<CAmount> MiniMiner::CalculateTotalBumpFees(const CFeeRate& target_ to_process.insert(iter); ancestors.insert(iter); } + + std::set<uint256> has_been_processed; while (!to_process.empty()) { auto iter = to_process.begin(); const CTransaction& tx = (*iter)->second.GetTx(); for (const auto& input : tx.vin) { if (auto parent_it{m_entries_by_txid.find(input.prevout.hash)}; parent_it != m_entries_by_txid.end()) { - to_process.insert(parent_it); + if (!has_been_processed.count(input.prevout.hash)) { + to_process.insert(parent_it); + } ancestors.insert(parent_it); } } + has_been_processed.insert(tx.GetHash()); to_process.erase(iter); } const auto ancestor_package_size = std::accumulate(ancestors.cbegin(), ancestors.cend(), int64_t{0}, diff --git a/src/node/utxo_snapshot.cpp b/src/node/utxo_snapshot.cpp index 3dae46fb84..036a25d0a5 100644 --- a/src/node/utxo_snapshot.cpp +++ b/src/node/utxo_snapshot.cpp @@ -4,7 +4,6 @@ #include <node/utxo_snapshot.h> -#include <common/args.h> #include <logging.h> #include <streams.h> #include <sync.h> @@ -82,10 +81,10 @@ std::optional<uint256> ReadSnapshotBaseBlockhash(fs::path chaindir) return base_blockhash; } -std::optional<fs::path> FindSnapshotChainstateDir() +std::optional<fs::path> FindSnapshotChainstateDir(const fs::path& data_dir) { fs::path possible_dir = - gArgs.GetDataDirNet() / fs::u8path(strprintf("chainstate%s", SNAPSHOT_CHAINSTATE_SUFFIX)); + data_dir / fs::u8path(strprintf("chainstate%s", SNAPSHOT_CHAINSTATE_SUFFIX)); if (fs::exists(possible_dir)) { return possible_dir; diff --git a/src/node/utxo_snapshot.h b/src/node/utxo_snapshot.h index 44ddd77dc3..a6dd3f3f13 100644 --- a/src/node/utxo_snapshot.h +++ b/src/node/utxo_snapshot.h @@ -67,7 +67,7 @@ constexpr std::string_view SNAPSHOT_CHAINSTATE_SUFFIX = "_snapshot"; //! Return a path to the snapshot-based chainstate dir, if one exists. -std::optional<fs::path> FindSnapshotChainstateDir(); +std::optional<fs::path> FindSnapshotChainstateDir(const fs::path& data_dir); } // namespace node diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 87a7e703b8..c1563fe1e2 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -60,7 +60,7 @@ static const char* SettingName(OptionsModel::OptionID option) } /** Call node.updateRwSetting() with Bitcoin 22.x workaround. */ -static void UpdateRwSetting(interfaces::Node& node, OptionsModel::OptionID option, const std::string& suffix, const util::SettingsValue& value) +static void UpdateRwSetting(interfaces::Node& node, OptionsModel::OptionID option, const std::string& suffix, const common::SettingsValue& value) { if (value.isNum() && (option == OptionsModel::DatabaseCache || @@ -81,14 +81,14 @@ static void UpdateRwSetting(interfaces::Node& node, OptionsModel::OptionID optio } //! Convert enabled/size values to bitcoin -prune setting. -static util::SettingsValue PruneSetting(bool prune_enabled, int prune_size_gb) +static common::SettingsValue PruneSetting(bool prune_enabled, int prune_size_gb) { assert(!prune_enabled || prune_size_gb >= 1); // PruneSizeGB and ParsePruneSizeGB never return less return prune_enabled ? PruneGBtoMiB(prune_size_gb) : 0; } //! Get pruning enabled value to show in GUI from bitcoin -prune setting. -static bool PruneEnabled(const util::SettingsValue& prune_setting) +static bool PruneEnabled(const common::SettingsValue& prune_setting) { // -prune=1 setting is manual pruning mode, so disabled for purposes of the gui return SettingToInt(prune_setting, 0) > 1; @@ -96,7 +96,7 @@ static bool PruneEnabled(const util::SettingsValue& prune_setting) //! Get pruning size value to show in GUI from bitcoin -prune setting. If //! pruning is not enabled, just show default recommended pruning size (2GB). -static int PruneSizeGB(const util::SettingsValue& prune_setting) +static int PruneSizeGB(const common::SettingsValue& prune_setting) { int value = SettingToInt(prune_setting, 0); return value > 1 ? PruneMiBtoGB(value) : DEFAULT_PRUNE_TARGET_GB; @@ -311,8 +311,8 @@ static QString GetDefaultProxyAddress() void OptionsModel::SetPruneTargetGB(int prune_target_gb) { - const util::SettingsValue cur_value = node().getPersistentSetting("prune"); - const util::SettingsValue new_value = PruneSetting(prune_target_gb > 0, prune_target_gb); + const common::SettingsValue cur_value = node().getPersistentSetting("prune"); + const common::SettingsValue new_value = PruneSetting(prune_target_gb > 0, prune_target_gb); // Force setting to take effect. It is still safe to change the value at // this point because this function is only called after the intro screen is @@ -331,7 +331,7 @@ void OptionsModel::SetPruneTargetGB(int prune_target_gb) // Keep previous pruning size, if pruning was disabled. if (PruneEnabled(cur_value)) { - UpdateRwSetting(node(), Prune, "-prev", PruneEnabled(new_value) ? util::SettingsValue{} : cur_value); + UpdateRwSetting(node(), Prune, "-prev", PruneEnabled(new_value) ? common::SettingsValue{} : cur_value); } } @@ -457,7 +457,7 @@ QVariant OptionsModel::getOption(OptionID option, const std::string& suffix) con bool OptionsModel::setOption(OptionID option, const QVariant& value, const std::string& suffix) { auto changed = [&] { return value.isValid() && value != getOption(option, suffix); }; - auto update = [&](const util::SettingsValue& value) { return UpdateRwSetting(node(), option, suffix, value); }; + auto update = [&](const common::SettingsValue& value) { return UpdateRwSetting(node(), option, suffix, value); }; bool successful = true; /* set to false on parse error */ QSettings settings; diff --git a/src/qt/test/optiontests.cpp b/src/qt/test/optiontests.cpp index 0838e21678..e5a5179d87 100644 --- a/src/qt/test/optiontests.cpp +++ b/src/qt/test/optiontests.cpp @@ -18,13 +18,13 @@ OptionTests::OptionTests(interfaces::Node& node) : m_node(node) { - gArgs.LockSettings([&](util::Settings& s) { m_previous_settings = s; }); + gArgs.LockSettings([&](common::Settings& s) { m_previous_settings = s; }); } void OptionTests::init() { // reset args - gArgs.LockSettings([&](util::Settings& s) { s = m_previous_settings; }); + gArgs.LockSettings([&](common::Settings& s) { s = m_previous_settings; }); gArgs.ClearPathCache(); } @@ -76,14 +76,14 @@ void OptionTests::integerGetArgBug() // Test regression https://github.com/bitcoin/bitcoin/issues/24457. Ensure // that setting integer prune value doesn't cause an exception to be thrown // in the OptionsModel constructor - gArgs.LockSettings([&](util::Settings& settings) { + gArgs.LockSettings([&](common::Settings& settings) { settings.forced_settings.erase("prune"); settings.rw_settings["prune"] = 3814; }); gArgs.WriteSettingsFile(); bilingual_str error; QVERIFY(OptionsModel{m_node}.Init(error)); - gArgs.LockSettings([&](util::Settings& settings) { + gArgs.LockSettings([&](common::Settings& settings) { settings.rw_settings.erase("prune"); }); gArgs.WriteSettingsFile(); @@ -95,7 +95,7 @@ void OptionTests::parametersInteraction() // It was fixed via https://github.com/bitcoin-core/gui/pull/568. // With fListen=false in ~/.config/Bitcoin/Bitcoin-Qt.conf and all else left as default, // bitcoin-qt should set both -listen and -listenonion to false and start successfully. - gArgs.LockSettings([&](util::Settings& s) { + gArgs.LockSettings([&](common::Settings& s) { s.forced_settings.erase("listen"); s.forced_settings.erase("listenonion"); }); diff --git a/src/qt/test/optiontests.h b/src/qt/test/optiontests.h index 0c458c97a6..2d7b94933f 100644 --- a/src/qt/test/optiontests.h +++ b/src/qt/test/optiontests.h @@ -5,9 +5,9 @@ #ifndef BITCOIN_QT_TEST_OPTIONTESTS_H #define BITCOIN_QT_TEST_OPTIONTESTS_H +#include <common/settings.h> #include <qt/optionsmodel.h> #include <univalue.h> -#include <util/settings.h> #include <QObject> @@ -26,7 +26,7 @@ private Q_SLOTS: private: interfaces::Node& m_node; - util::Settings m_previous_settings; + common::Settings m_previous_settings; }; #endif // BITCOIN_QT_TEST_OPTIONTESTS_H diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index d08e2d55d1..edc0fb05d7 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -101,6 +101,11 @@ static const CRPCConvertParam vRPCConvertParams[] = { "listunspent", 2, "addresses" }, { "listunspent", 3, "include_unsafe" }, { "listunspent", 4, "query_options" }, + { "listunspent", 4, "minimumAmount" }, + { "listunspent", 4, "maximumAmount" }, + { "listunspent", 4, "maximumCount" }, + { "listunspent", 4, "minimumSumAmount" }, + { "listunspent", 4, "include_immature_coinbase" }, { "getblock", 1, "verbosity" }, { "getblock", 1, "verbose" }, { "getblockheader", 1, "verbose" }, @@ -124,11 +129,38 @@ static const CRPCConvertParam vRPCConvertParams[] = { "submitpackage", 0, "package" }, { "combinerawtransaction", 0, "txs" }, { "fundrawtransaction", 1, "options" }, + { "fundrawtransaction", 1, "add_inputs"}, + { "fundrawtransaction", 1, "include_unsafe"}, + { "fundrawtransaction", 1, "minconf"}, + { "fundrawtransaction", 1, "maxconf"}, + { "fundrawtransaction", 1, "changePosition"}, + { "fundrawtransaction", 1, "includeWatching"}, + { "fundrawtransaction", 1, "lockUnspents"}, + { "fundrawtransaction", 1, "fee_rate"}, + { "fundrawtransaction", 1, "feeRate"}, + { "fundrawtransaction", 1, "subtractFeeFromOutputs"}, + { "fundrawtransaction", 1, "input_weights"}, + { "fundrawtransaction", 1, "conf_target"}, + { "fundrawtransaction", 1, "replaceable"}, + { "fundrawtransaction", 1, "solving_data"}, { "fundrawtransaction", 2, "iswitness" }, { "walletcreatefundedpsbt", 0, "inputs" }, { "walletcreatefundedpsbt", 1, "outputs" }, { "walletcreatefundedpsbt", 2, "locktime" }, { "walletcreatefundedpsbt", 3, "options" }, + { "walletcreatefundedpsbt", 3, "add_inputs"}, + { "walletcreatefundedpsbt", 3, "include_unsafe"}, + { "walletcreatefundedpsbt", 3, "minconf"}, + { "walletcreatefundedpsbt", 3, "maxconf"}, + { "walletcreatefundedpsbt", 3, "changePosition"}, + { "walletcreatefundedpsbt", 3, "includeWatching"}, + { "walletcreatefundedpsbt", 3, "lockUnspents"}, + { "walletcreatefundedpsbt", 3, "fee_rate"}, + { "walletcreatefundedpsbt", 3, "feeRate"}, + { "walletcreatefundedpsbt", 3, "subtractFeeFromOutputs"}, + { "walletcreatefundedpsbt", 3, "conf_target"}, + { "walletcreatefundedpsbt", 3, "replaceable"}, + { "walletcreatefundedpsbt", 3, "solving_data"}, { "walletcreatefundedpsbt", 4, "bip32derivs" }, { "walletprocesspsbt", 1, "sign" }, { "walletprocesspsbt", 3, "bip32derivs" }, @@ -157,18 +189,49 @@ static const CRPCConvertParam vRPCConvertParams[] = { "send", 1, "conf_target" }, { "send", 3, "fee_rate"}, { "send", 4, "options" }, + { "send", 4, "add_inputs"}, + { "send", 4, "include_unsafe"}, + { "send", 4, "minconf"}, + { "send", 4, "maxconf"}, + { "send", 4, "add_to_wallet"}, + { "send", 4, "change_position"}, + { "send", 4, "fee_rate"}, + { "send", 4, "include_watching"}, + { "send", 4, "inputs"}, + { "send", 4, "locktime"}, + { "send", 4, "lock_unspents"}, + { "send", 4, "psbt"}, + { "send", 4, "subtract_fee_from_outputs"}, + { "send", 4, "conf_target"}, + { "send", 4, "replaceable"}, + { "send", 4, "solving_data"}, { "sendall", 0, "recipients" }, { "sendall", 1, "conf_target" }, { "sendall", 3, "fee_rate"}, { "sendall", 4, "options" }, + { "sendall", 4, "add_to_wallet"}, + { "sendall", 4, "fee_rate"}, + { "sendall", 4, "include_watching"}, + { "sendall", 4, "inputs"}, + { "sendall", 4, "locktime"}, + { "sendall", 4, "lock_unspents"}, + { "sendall", 4, "psbt"}, + { "sendall", 4, "send_max"}, + { "sendall", 4, "minconf"}, + { "sendall", 4, "maxconf"}, + { "sendall", 4, "conf_target"}, + { "sendall", 4, "replaceable"}, + { "sendall", 4, "solving_data"}, { "simulaterawtransaction", 0, "rawtxs" }, { "simulaterawtransaction", 1, "options" }, + { "simulaterawtransaction", 1, "include_watchonly"}, { "importprivkey", 2, "rescan" }, { "importaddress", 2, "rescan" }, { "importaddress", 3, "p2sh" }, { "importpubkey", 2, "rescan" }, { "importmulti", 0, "requests" }, { "importmulti", 1, "options" }, + { "importmulti", 1, "rescan" }, { "importdescriptors", 0, "requests" }, { "listdescriptors", 0, "private" }, { "verifychain", 0, "checklevel" }, @@ -192,7 +255,15 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getmempooldescendants", 1, "verbose" }, { "gettxspendingprevout", 0, "outputs" }, { "bumpfee", 1, "options" }, + { "bumpfee", 1, "conf_target"}, + { "bumpfee", 1, "fee_rate"}, + { "bumpfee", 1, "replaceable"}, + { "bumpfee", 1, "outputs"}, { "psbtbumpfee", 1, "options" }, + { "psbtbumpfee", 1, "conf_target"}, + { "psbtbumpfee", 1, "fee_rate"}, + { "psbtbumpfee", 1, "replaceable"}, + { "psbtbumpfee", 1, "outputs"}, { "logging", 0, "include" }, { "logging", 1, "exclude" }, { "disconnectnode", 1, "nodeid" }, @@ -226,6 +297,14 @@ static const CRPCConvertParam vRPCConvertParams[] = }; // clang-format on +/** Parse string to UniValue or throw runtime_error if string contains invalid JSON */ +static UniValue Parse(std::string_view raw) +{ + UniValue parsed; + if (!parsed.read(raw)) throw std::runtime_error(tfm::format("Error parsing JSON: %s", raw)); + return parsed; +} + class CRPCConvertTable { private: @@ -238,13 +317,13 @@ public: /** Return arg_value as UniValue, and first parse it if it is a non-string parameter */ UniValue ArgToUniValue(std::string_view arg_value, const std::string& method, int param_idx) { - return members.count({method, param_idx}) > 0 ? ParseNonRFCJSONValue(arg_value) : arg_value; + return members.count({method, param_idx}) > 0 ? Parse(arg_value) : arg_value; } /** Return arg_value as UniValue, and first parse it if it is a non-string parameter */ UniValue ArgToUniValue(std::string_view arg_value, const std::string& method, const std::string& param_name) { - return membersByName.count({method, param_name}) > 0 ? ParseNonRFCJSONValue(arg_value) : arg_value; + return membersByName.count({method, param_name}) > 0 ? Parse(arg_value) : arg_value; } }; @@ -258,16 +337,6 @@ CRPCConvertTable::CRPCConvertTable() static CRPCConvertTable rpcCvtTable; -/** Non-RFC4627 JSON parser, accepts internal values (such as numbers, true, false, null) - * as well as objects and arrays. - */ -UniValue ParseNonRFCJSONValue(std::string_view raw) -{ - UniValue parsed; - if (!parsed.read(raw)) throw std::runtime_error(tfm::format("Error parsing JSON: %s", raw)); - return parsed; -} - UniValue RPCConvertValues(const std::string &strMethod, const std::vector<std::string> &strParams) { UniValue params(UniValue::VARR); diff --git a/src/rpc/client.h b/src/rpc/client.h index 3c5c4fc4d6..b67cd27fdf 100644 --- a/src/rpc/client.h +++ b/src/rpc/client.h @@ -17,9 +17,4 @@ UniValue RPCConvertValues(const std::string& strMethod, const std::vector<std::s /** Convert named arguments to command-specific RPC representation */ UniValue RPCConvertNamedValues(const std::string& strMethod, const std::vector<std::string>& strParams); -/** Non-RFC4627 JSON parser, accepts internal values (such as numbers, true, false, null) - * as well as objects and arrays. - */ -UniValue ParseNonRFCJSONValue(std::string_view raw); - #endif // BITCOIN_RPC_CLIENT_H diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index eb61d58a33..074cecadd2 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -480,6 +480,40 @@ static RPCHelpMan prioritisetransaction() }; } +static RPCHelpMan getprioritisedtransactions() +{ + return RPCHelpMan{"getprioritisedtransactions", + "Returns a map of all user-created (see prioritisetransaction) fee deltas by txid, and whether the tx is present in mempool.", + {}, + RPCResult{ + RPCResult::Type::OBJ_DYN, "prioritisation-map", "prioritisation keyed by txid", + { + {RPCResult::Type::OBJ, "txid", "", { + {RPCResult::Type::NUM, "fee_delta", "transaction fee delta in satoshis"}, + {RPCResult::Type::BOOL, "in_mempool", "whether this transaction is currently in mempool"}, + }} + }, + }, + RPCExamples{ + HelpExampleCli("getprioritisedtransactions", "") + + HelpExampleRpc("getprioritisedtransactions", "") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + { + NodeContext& node = EnsureAnyNodeContext(request.context); + CTxMemPool& mempool = EnsureMemPool(node); + UniValue rpc_result{UniValue::VOBJ}; + for (const auto& delta_info : mempool.GetPrioritisedTransactions()) { + UniValue result_inner{UniValue::VOBJ}; + result_inner.pushKV("fee_delta", delta_info.delta); + result_inner.pushKV("in_mempool", delta_info.in_mempool); + rpc_result.pushKV(delta_info.txid.GetHex(), result_inner); + } + return rpc_result; + }, + }; +} + // NOTE: Assumes a conclusive result; if result is inconclusive, it must be handled by caller static UniValue BIP22ValidationResult(const BlockValidationState& state) @@ -1048,6 +1082,7 @@ void RegisterMiningRPCCommands(CRPCTable& t) {"mining", &getnetworkhashps}, {"mining", &getmininginfo}, {"mining", &prioritisetransaction}, + {"mining", &getprioritisedtransactions}, {"mining", &getblocktemplate}, {"mining", &submitblock}, {"mining", &submitheader}, diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 474b318c66..d39efcb245 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -392,7 +392,7 @@ std::string JSONRPCExecBatch(const JSONRPCRequest& jreq, const UniValue& vReq) * Process named arguments into a vector of positional arguments, based on the * passed-in specification for the RPC call's arguments. */ -static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, const std::vector<std::string>& argNames) +static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, const std::vector<std::pair<std::string, bool>>& argNames) { JSONRPCRequest out = in; out.params = UniValue(UniValue::VARR); @@ -417,7 +417,9 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c // "args" parameter, if present. int hole = 0; int initial_hole_size = 0; - for (const std::string &argNamePattern: argNames) { + const std::string* initial_param = nullptr; + UniValue options{UniValue::VOBJ}; + for (const auto& [argNamePattern, named_only]: argNames) { std::vector<std::string> vargNames = SplitString(argNamePattern, '|'); auto fr = argsIn.end(); for (const std::string & argName : vargNames) { @@ -426,7 +428,22 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c break; } } - if (fr != argsIn.end()) { + + // Handle named-only parameters by pushing them into a temporary options + // object, and then pushing the accumulated options as the next + // positional argument. + if (named_only) { + if (fr != argsIn.end()) { + if (options.exists(fr->first)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + fr->first + " specified multiple times"); + } + options.__pushKV(fr->first, *fr->second); + argsIn.erase(fr); + } + continue; + } + + if (!options.empty() || fr != argsIn.end()) { for (int i = 0; i < hole; ++i) { // Fill hole between specified parameters with JSON nulls, // but not at the end (for backwards compatibility with calls @@ -434,12 +451,26 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c out.params.push_back(UniValue()); } hole = 0; - out.params.push_back(*fr->second); - argsIn.erase(fr); + if (!initial_param) initial_param = &argNamePattern; } else { hole += 1; if (out.params.empty()) initial_hole_size = hole; } + + // If named input parameter "fr" is present, push it onto out.params. If + // options are present, push them onto out.params. If both are present, + // throw an error. + if (fr != argsIn.end()) { + if (!options.empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + fr->first + " conflicts with parameter " + options.getKeys().front()); + } + out.params.push_back(*fr->second); + argsIn.erase(fr); + } + if (!options.empty()) { + out.params.push_back(std::move(options)); + options = UniValue{UniValue::VOBJ}; + } } // If leftover "args" param was found, use it as a source of positional // arguments and add named arguments after. This is a convenience for @@ -447,9 +478,8 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c // arguments as described in doc/JSON-RPC-interface.md#parameter-passing auto positional_args{argsIn.extract("args")}; if (positional_args && positional_args.mapped()->isArray()) { - const bool has_named_arguments{initial_hole_size < (int)argNames.size()}; - if (initial_hole_size < (int)positional_args.mapped()->size() && has_named_arguments) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + argNames[initial_hole_size] + " specified twice both as positional and named argument"); + if (initial_hole_size < (int)positional_args.mapped()->size() && initial_param) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + *initial_param + " specified twice both as positional and named argument"); } // Assign positional_args to out.params and append named_args after. UniValue named_args{std::move(out.params)}; diff --git a/src/rpc/server.h b/src/rpc/server.h index 01e8556050..24658ddb8b 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -95,7 +95,7 @@ public: using Actor = std::function<bool(const JSONRPCRequest& request, UniValue& result, bool last_handler)>; //! Constructor taking Actor callback supporting multiple handlers. - CRPCCommand(std::string category, std::string name, Actor actor, std::vector<std::string> args, intptr_t unique_id) + CRPCCommand(std::string category, std::string name, Actor actor, std::vector<std::pair<std::string, bool>> args, intptr_t unique_id) : category(std::move(category)), name(std::move(name)), actor(std::move(actor)), argNames(std::move(args)), unique_id(unique_id) { @@ -115,7 +115,16 @@ public: std::string category; std::string name; Actor actor; - std::vector<std::string> argNames; + //! List of method arguments and whether they are named-only. Incoming RPC + //! requests contain a "params" field that can either be an array containing + //! unnamed arguments or an object containing named arguments. The + //! "argNames" vector is used in the latter case to transform the params + //! object into an array. Each argument in "argNames" gets mapped to a + //! unique position in the array, based on the order it is listed, unless + //! the argument is a named-only argument with argNames[x].second set to + //! true. Named-only arguments are combined into a JSON object that is + //! appended after other arguments, see transformNamedArguments for details. + std::vector<std::pair<std::string, bool>> argNames; intptr_t unique_id; }; diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index d1ff3f3871..19e14f88df 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -389,7 +389,8 @@ struct Sections { case RPCArg::Type::NUM: case RPCArg::Type::AMOUNT: case RPCArg::Type::RANGE: - case RPCArg::Type::BOOL: { + case RPCArg::Type::BOOL: + case RPCArg::Type::OBJ_NAMED_PARAMS: { if (is_top_level_arg) return; // Nothing more to do for non-recursive types on first recursion auto left = indent; if (arg.m_opts.type_str.size() != 0 && push_name) { @@ -485,12 +486,32 @@ RPCHelpMan::RPCHelpMan(std::string name, std::string description, std::vector<RP m_results{std::move(results)}, m_examples{std::move(examples)} { - std::set<std::string> named_args; + // Map of parameter names and types just used to check whether the names are + // unique. Parameter names always need to be unique, with the exception that + // there can be pairs of POSITIONAL and NAMED parameters with the same name. + enum ParamType { POSITIONAL = 1, NAMED = 2, NAMED_ONLY = 4 }; + std::map<std::string, int> param_names; + for (const auto& arg : m_args) { std::vector<std::string> names = SplitString(arg.m_names, '|'); // Should have unique named arguments for (const std::string& name : names) { - CHECK_NONFATAL(named_args.insert(name).second); + auto& param_type = param_names[name]; + CHECK_NONFATAL(!(param_type & POSITIONAL)); + CHECK_NONFATAL(!(param_type & NAMED_ONLY)); + param_type |= POSITIONAL; + } + if (arg.m_type == RPCArg::Type::OBJ_NAMED_PARAMS) { + for (const auto& inner : arg.m_inner) { + std::vector<std::string> inner_names = SplitString(inner.m_names, '|'); + for (const std::string& inner_name : inner_names) { + auto& param_type = param_names[inner_name]; + CHECK_NONFATAL(!(param_type & POSITIONAL) || inner.m_opts.also_positional); + CHECK_NONFATAL(!(param_type & NAMED)); + CHECK_NONFATAL(!(param_type & NAMED_ONLY)); + param_type |= inner.m_opts.also_positional ? NAMED : NAMED_ONLY; + } + } } // Default value type should match argument type only when defined if (arg.m_fallback.index() == 2) { @@ -605,12 +626,17 @@ bool RPCHelpMan::IsValidNumArgs(size_t num_args) const return num_required_args <= num_args && num_args <= m_args.size(); } -std::vector<std::string> RPCHelpMan::GetArgNames() const +std::vector<std::pair<std::string, bool>> RPCHelpMan::GetArgNames() const { - std::vector<std::string> ret; + std::vector<std::pair<std::string, bool>> ret; ret.reserve(m_args.size()); for (const auto& arg : m_args) { - ret.emplace_back(arg.m_names); + if (arg.m_type == RPCArg::Type::OBJ_NAMED_PARAMS) { + for (const auto& inner : arg.m_inner) { + ret.emplace_back(inner.m_names, /*named_only=*/true); + } + } + ret.emplace_back(arg.m_names, /*named_only=*/false); } return ret; } @@ -642,20 +668,31 @@ std::string RPCHelpMan::ToString() const // Arguments Sections sections; + Sections named_only_sections; for (size_t i{0}; i < m_args.size(); ++i) { const auto& arg = m_args.at(i); if (arg.m_opts.hidden) break; // Any arg that follows is also hidden - if (i == 0) ret += "\nArguments:\n"; - // Push named argument name and description sections.m_sections.emplace_back(::ToString(i + 1) + ". " + arg.GetFirstName(), arg.ToDescriptionString(/*is_named_arg=*/true)); sections.m_max_pad = std::max(sections.m_max_pad, sections.m_sections.back().m_left.size()); // Recursively push nested args sections.Push(arg); + + // Push named-only argument sections + if (arg.m_type == RPCArg::Type::OBJ_NAMED_PARAMS) { + for (const auto& arg_inner : arg.m_inner) { + named_only_sections.PushSection({arg_inner.GetFirstName(), arg_inner.ToDescriptionString(/*is_named_arg=*/true)}); + named_only_sections.Push(arg_inner); + } + } } + + if (!sections.m_sections.empty()) ret += "\nArguments:\n"; ret += sections.ToString(); + if (!named_only_sections.m_sections.empty()) ret += "\nNamed Arguments:\n"; + ret += named_only_sections.ToString(); // Result ret += m_results.ToDescriptionString(); @@ -669,17 +706,30 @@ std::string RPCHelpMan::ToString() const UniValue RPCHelpMan::GetArgMap() const { UniValue arr{UniValue::VARR}; + + auto push_back_arg_info = [&arr](const std::string& rpc_name, int pos, const std::string& arg_name, const RPCArg::Type& type) { + UniValue map{UniValue::VARR}; + map.push_back(rpc_name); + map.push_back(pos); + map.push_back(arg_name); + map.push_back(type == RPCArg::Type::STR || + type == RPCArg::Type::STR_HEX); + arr.push_back(map); + }; + for (int i{0}; i < int(m_args.size()); ++i) { const auto& arg = m_args.at(i); std::vector<std::string> arg_names = SplitString(arg.m_names, '|'); for (const auto& arg_name : arg_names) { - UniValue map{UniValue::VARR}; - map.push_back(m_name); - map.push_back(i); - map.push_back(arg_name); - map.push_back(arg.m_type == RPCArg::Type::STR || - arg.m_type == RPCArg::Type::STR_HEX); - arr.push_back(map); + push_back_arg_info(m_name, i, arg_name, arg.m_type); + if (arg.m_type == RPCArg::Type::OBJ_NAMED_PARAMS) { + for (const auto& inner : arg.m_inner) { + std::vector<std::string> inner_names = SplitString(inner.m_names, '|'); + for (const std::string& inner_name : inner_names) { + push_back_arg_info(m_name, i, inner_name, inner.m_type); + } + } + } } } return arr; @@ -708,6 +758,7 @@ static std::optional<UniValue::VType> ExpectedType(RPCArg::Type type) return UniValue::VBOOL; } case Type::OBJ: + case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: { return UniValue::VOBJ; } @@ -781,6 +832,7 @@ std::string RPCArg::ToDescriptionString(bool is_named_arg) const break; } case Type::OBJ: + case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: { ret += "json object"; break; @@ -809,6 +861,7 @@ std::string RPCArg::ToDescriptionString(bool is_named_arg) const } // no default case, so the compiler can warn about missing cases } ret += ")"; + if (m_type == Type::OBJ_NAMED_PARAMS) ret += " Options object that can be used to pass named arguments, listed below."; ret += m_description.empty() ? "" : " " + m_description; return ret; } @@ -1054,6 +1107,7 @@ std::string RPCArg::ToStringObj(const bool oneline) const } return res + "...]"; case Type::OBJ: + case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: // Currently unused, so avoid writing dead code NONFATAL_UNREACHABLE(); @@ -1077,6 +1131,7 @@ std::string RPCArg::ToString(const bool oneline) const return GetFirstName(); } case Type::OBJ: + case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: { const std::string res = Join(m_inner, ",", [&](const RPCArg& i) { return i.ToStringObj(oneline); }); if (m_type == Type::OBJ) { diff --git a/src/rpc/util.h b/src/rpc/util.h index 3ff02582a6..4cba5a9818 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -130,6 +130,15 @@ struct RPCArgOptions { std::string oneline_description{}; //!< Should be empty unless it is supposed to override the auto-generated summary line std::vector<std::string> type_str{}; //!< Should be empty unless it is supposed to override the auto-generated type strings. Vector length is either 0 or 2, m_opts.type_str.at(0) will override the type of the value in a key-value pair, m_opts.type_str.at(1) will override the type in the argument description. bool hidden{false}; //!< For testing only + bool also_positional{false}; //!< If set allows a named-parameter field in an OBJ_NAMED_PARAM options object + //!< to have the same name as a top-level parameter. By default the RPC + //!< framework disallows this, because if an RPC request passes the value by + //!< name, it is assigned to top-level parameter position, not to the options + //!< position, defeating the purpose of using OBJ_NAMED_PARAMS instead OBJ for + //!< that option. But sometimes it makes sense to allow less-commonly used + //!< options to be passed by name only, and more commonly used options to be + //!< passed by name or position, so the RPC framework allows this as long as + //!< methods set the also_positional flag and read values from both positions. }; struct RPCArg { @@ -139,6 +148,13 @@ struct RPCArg { STR, NUM, BOOL, + OBJ_NAMED_PARAMS, //!< Special type that behaves almost exactly like + //!< OBJ, defining an options object with a list of + //!< pre-defined keys. The only difference between OBJ + //!< and OBJ_NAMED_PARAMS is that OBJ_NAMED_PARMS + //!< also allows the keys to be passed as top-level + //!< named parameters, as a more convenient way to pass + //!< options to the RPC method without nesting them. OBJ_USER_KEYS, //!< Special type where the user must set the keys e.g. to define multiple addresses; as opposed to e.g. an options object where the keys are predefined AMOUNT, //!< Special type representing a floating point amount (can be either NUM or STR) STR_HEX, //!< Special type that is a STR with only hex chars @@ -183,7 +199,7 @@ struct RPCArg { m_description{std::move(description)}, m_opts{std::move(opts)} { - CHECK_NONFATAL(type != Type::ARR && type != Type::OBJ && type != Type::OBJ_USER_KEYS); + CHECK_NONFATAL(type != Type::ARR && type != Type::OBJ && type != Type::OBJ_NAMED_PARAMS && type != Type::OBJ_USER_KEYS); } RPCArg( @@ -200,7 +216,7 @@ struct RPCArg { m_description{std::move(description)}, m_opts{std::move(opts)} { - CHECK_NONFATAL(type == Type::ARR || type == Type::OBJ || type == Type::OBJ_USER_KEYS); + CHECK_NONFATAL(type == Type::ARR || type == Type::OBJ || type == Type::OBJ_NAMED_PARAMS || type == Type::OBJ_USER_KEYS); } bool IsOptional() const; @@ -369,7 +385,8 @@ public: UniValue GetArgMap() const; /** If the supplied number of args is neither too small nor too high */ bool IsValidNumArgs(size_t num_args) const; - std::vector<std::string> GetArgNames() const; + //! Return list of arguments and whether they are named-only. + std::vector<std::pair<std::string, bool>> GetArgNames() const; const std::string m_name; diff --git a/src/streams.h b/src/streams.h index e346aa0a3f..03df20b5db 100644 --- a/src/streams.h +++ b/src/streams.h @@ -293,14 +293,6 @@ public: vch.insert(vch.end(), src.begin(), src.end()); } - template<typename Stream> - void Serialize(Stream& s) const - { - // Special case: stream << stream concatenates like stream += stream - if (!vch.empty()) - s.write(MakeByteSpan(vch)); - } - template<typename T> DataStream& operator<<(const T& obj) { diff --git a/src/test/argsman_tests.cpp b/src/test/argsman_tests.cpp index 48bffc4ac9..0b789e7f5c 100644 --- a/src/test/argsman_tests.cpp +++ b/src/test/argsman_tests.cpp @@ -85,7 +85,7 @@ class CheckValueTest : public TestChain100Setup { public: struct Expect { - util::SettingsValue setting; + common::SettingsValue setting; bool default_string = false; bool default_int = false; bool default_bool = false; @@ -95,7 +95,7 @@ public: std::optional<std::vector<std::string>> list_value; const char* error = nullptr; - explicit Expect(util::SettingsValue s) : setting(std::move(s)) {} + explicit Expect(common::SettingsValue s) : setting(std::move(s)) {} Expect& DefaultString() { default_string = true; return *this; } Expect& DefaultInt() { default_int = true; return *this; } Expect& DefaultBool() { default_bool = true; return *this; } @@ -1022,14 +1022,14 @@ BOOST_AUTO_TEST_CASE(util_ReadWriteSettings) // Test writing setting. TestArgsManager args1; args1.ForceSetArg("-datadir", fs::PathToString(m_path_root)); - args1.LockSettings([&](util::Settings& settings) { settings.rw_settings["name"] = "value"; }); + args1.LockSettings([&](common::Settings& settings) { settings.rw_settings["name"] = "value"; }); args1.WriteSettingsFile(); // Test reading setting. TestArgsManager args2; args2.ForceSetArg("-datadir", fs::PathToString(m_path_root)); args2.ReadSettingsFile(); - args2.LockSettings([&](util::Settings& settings) { BOOST_CHECK_EQUAL(settings.rw_settings["name"].get_str(), "value"); }); + args2.LockSettings([&](common::Settings& settings) { BOOST_CHECK_EQUAL(settings.rw_settings["name"].get_str(), "value"); }); // Test error logging, and remove previously written setting. { diff --git a/src/test/fuzz/parse_univalue.cpp b/src/test/fuzz/parse_univalue.cpp index be15a38e92..6d33c1a8cc 100644 --- a/src/test/fuzz/parse_univalue.cpp +++ b/src/test/fuzz/parse_univalue.cpp @@ -22,12 +22,9 @@ FUZZ_TARGET_INIT(parse_univalue, initialize_parse_univalue) const std::string random_string(buffer.begin(), buffer.end()); bool valid = true; const UniValue univalue = [&] { - try { - return ParseNonRFCJSONValue(random_string); - } catch (const std::runtime_error&) { - valid = false; - return UniValue{}; - } + UniValue uv; + if (!uv.read(random_string)) valid = false; + return valid ? uv : UniValue{}; }(); if (!valid) { return; diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 6424f756a0..b1858a1800 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -136,6 +136,7 @@ const std::vector<std::string> RPC_COMMANDS_SAFE_FOR_FUZZING{ "getnetworkinfo", "getnodeaddresses", "getpeerinfo", + "getprioritisedtransactions", "getrawmempool", "getrawtransaction", "getrpcinfo", diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp index 75c78ce1bd..e81efac6e0 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -5,6 +5,7 @@ #include <blockfilter.h> #include <clientversion.h> #include <common/args.h> +#include <common/settings.h> #include <common/system.h> #include <common/url.h> #include <netbase.h> @@ -22,7 +23,6 @@ #include <test/fuzz/util.h> #include <util/error.h> #include <util/fees.h> -#include <util/settings.h> #include <util/strencodings.h> #include <util/string.h> #include <util/translation.h> @@ -63,13 +63,9 @@ FUZZ_TARGET(string) (void)IsDeprecatedRPCEnabled(random_string_1); (void)Join(random_string_vector, random_string_1); (void)JSONRPCError(fuzzed_data_provider.ConsumeIntegral<int>(), random_string_1); - const util::Settings settings; + const common::Settings settings; (void)OnlyHasDefaultSectionSetting(settings, random_string_1, random_string_2); (void)ParseNetwork(random_string_1); - try { - (void)ParseNonRFCJSONValue(random_string_1); - } catch (const std::runtime_error&) { - } (void)ParseOutputType(random_string_1); (void)RemovePrefix(random_string_1, random_string_2); (void)ResolveErrMsg(random_string_1, random_string_2); diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index ea78edd05f..318797faf2 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/src/test/fuzz/utxo_total_supply.cpp @@ -144,13 +144,13 @@ FUZZ_TARGET(utxo_total_supply) node::RegenerateCommitments(*current_block, chainman); const bool was_valid = !MineBlock(node, current_block).IsNull(); - if (duplicate_coinbase_height == ActiveHeight()) { - // we mined the duplicate coinbase - assert(current_block->vtx.at(0)->vin.at(0).scriptSig == duplicate_coinbase_script); - } - const auto prev_utxo_stats = utxo_stats; if (was_valid) { + if (duplicate_coinbase_height == ActiveHeight()) { + // we mined the duplicate coinbase + assert(current_block->vtx.at(0)->vin.at(0).scriptSig == duplicate_coinbase_script); + } + circulation += GetBlockSubsidy(ActiveHeight(), Params().GetConsensus()); } diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp index 715b6885f5..c73b675388 100644 --- a/src/test/getarg_tests.cpp +++ b/src/test/getarg_tests.cpp @@ -3,10 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <common/args.h> +#include <common/settings.h> #include <logging.h> #include <test/util/setup_common.h> #include <univalue.h> -#include <util/settings.h> #include <util/strencodings.h> #include <limits> @@ -57,8 +57,8 @@ BOOST_AUTO_TEST_CASE(setting_args) ArgsManager args; SetupArgs(args, {{"-foo", ArgsManager::ALLOW_ANY}}); - auto set_foo = [&](const util::SettingsValue& value) { - args.LockSettings([&](util::Settings& settings) { + auto set_foo = [&](const common::SettingsValue& value) { + args.LockSettings([&](common::Settings& settings) { settings.rw_settings["foo"] = value; }); }; diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index da0029c737..2f783a4b95 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -42,11 +42,11 @@ private: class RPCTestingSetup : public TestingSetup { public: - UniValue TransformParams(const UniValue& params, std::vector<std::string> arg_names) const; + UniValue TransformParams(const UniValue& params, std::vector<std::pair<std::string, bool>> arg_names) const; UniValue CallRPC(std::string args); }; -UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector<std::string> arg_names) const +UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector<std::pair<std::string, bool>> arg_names) const { UniValue transformed_params; CRPCTable table; @@ -84,7 +84,7 @@ BOOST_FIXTURE_TEST_SUITE(rpc_tests, RPCTestingSetup) BOOST_AUTO_TEST_CASE(rpc_namedparams) { - const std::vector<std::string> arg_names{"arg1", "arg2", "arg3", "arg4", "arg5"}; + const std::vector<std::pair<std::string, bool>> arg_names{{"arg1", false}, {"arg2", false}, {"arg3", false}, {"arg4", false}, {"arg5", false}}; // Make sure named arguments are transformed into positional arguments in correct places separated by nulls BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg2": 2, "arg4": 4})"), arg_names).write(), "[null,2,null,4]"); @@ -109,6 +109,28 @@ BOOST_AUTO_TEST_CASE(rpc_namedparams) BOOST_CHECK_EQUAL(TransformParams(JSON(R"([1,2,3,4,5,6,7,8,9,10])"), arg_names).write(), "[1,2,3,4,5,6,7,8,9,10]"); } +BOOST_AUTO_TEST_CASE(rpc_namedonlyparams) +{ + const std::vector<std::pair<std::string, bool>> arg_names{{"arg1", false}, {"arg2", false}, {"opt1", true}, {"opt2", true}, {"options", false}}; + + // Make sure optional parameters are really optional. + BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg1": 1, "arg2": 2})"), arg_names).write(), "[1,2]"); + + // Make sure named-only parameters are passed as options. + BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg1": 1, "arg2": 2, "opt1": 10, "opt2": 20})"), arg_names).write(), R"([1,2,{"opt1":10,"opt2":20}])"); + + // Make sure options can be passed directly. + BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg1": 1, "arg2": 2, "options":{"opt1": 10, "opt2": 20}})"), arg_names).write(), R"([1,2,{"opt1":10,"opt2":20}])"); + + // Make sure options and named parameters conflict. + BOOST_CHECK_EXCEPTION(TransformParams(JSON(R"({"arg1": 1, "arg2": 2, "opt1": 10, "options":{"opt1": 10}})"), arg_names), UniValue, + HasJSON(R"({"code":-8,"message":"Parameter options conflicts with parameter opt1"})")); + + // Make sure options object specified through args array conflicts. + BOOST_CHECK_EXCEPTION(TransformParams(JSON(R"({"args": [1, 2, {"opt1": 10}], "opt2": 20})"), arg_names), UniValue, + HasJSON(R"({"code":-8,"message":"Parameter options specified twice both as positional and named argument"})")); +} + BOOST_AUTO_TEST_CASE(rpc_rawparams) { // Test raw transaction API argument handling @@ -278,6 +300,7 @@ BOOST_AUTO_TEST_CASE(rpc_parse_monetary_values) BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("0.00000001000000")), 1LL); //should pass, cut trailing 0 BOOST_CHECK_THROW(AmountFromValue(ValueFromString("19e-9")), UniValue); //should fail BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("0.19e-6")), 19); //should pass, leading 0 is present + BOOST_CHECK_EXCEPTION(AmountFromValue(".19e-6"), UniValue, HasJSON(R"({"code":-3,"message":"Invalid amount"})")); //should fail, no leading 0 BOOST_CHECK_THROW(AmountFromValue(ValueFromString("92233720368.54775808")), UniValue); //overflow error BOOST_CHECK_THROW(AmountFromValue(ValueFromString("1e+11")), UniValue); //overflow error @@ -285,36 +308,6 @@ BOOST_AUTO_TEST_CASE(rpc_parse_monetary_values) BOOST_CHECK_THROW(AmountFromValue(ValueFromString("93e+9")), UniValue); //overflow error } -BOOST_AUTO_TEST_CASE(json_parse_errors) -{ - // Valid - BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("1.0").get_real(), 1.0); - BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("true").get_bool(), true); - BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("[false]")[0].get_bool(), false); - BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("{\"a\": true}")["a"].get_bool(), true); - BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("{\"1\": \"true\"}")["1"].get_str(), "true"); - // Valid, with leading or trailing whitespace - BOOST_CHECK_EQUAL(ParseNonRFCJSONValue(" 1.0").get_real(), 1.0); - BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("1.0 ").get_real(), 1.0); - - BOOST_CHECK_THROW(AmountFromValue(ParseNonRFCJSONValue(".19e-6")), std::runtime_error); //should fail, missing leading 0, therefore invalid JSON - BOOST_CHECK_EQUAL(AmountFromValue(ParseNonRFCJSONValue("0.00000000000000000000000000000000000001e+30 ")), 1); - // Invalid, initial garbage - BOOST_CHECK_THROW(ParseNonRFCJSONValue("[1.0"), std::runtime_error); - BOOST_CHECK_THROW(ParseNonRFCJSONValue("a1.0"), std::runtime_error); - // Invalid, trailing garbage - BOOST_CHECK_THROW(ParseNonRFCJSONValue("1.0sds"), std::runtime_error); - BOOST_CHECK_THROW(ParseNonRFCJSONValue("1.0]"), std::runtime_error); - // Invalid, keys have to be names - BOOST_CHECK_THROW(ParseNonRFCJSONValue("{1: \"true\"}"), std::runtime_error); - BOOST_CHECK_THROW(ParseNonRFCJSONValue("{true: 1}"), std::runtime_error); - BOOST_CHECK_THROW(ParseNonRFCJSONValue("{[1]: 1}"), std::runtime_error); - BOOST_CHECK_THROW(ParseNonRFCJSONValue("{{\"a\": \"a\"}: 1}"), std::runtime_error); - // BTC addresses should fail parsing - BOOST_CHECK_THROW(ParseNonRFCJSONValue("175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W"), std::runtime_error); - BOOST_CHECK_THROW(ParseNonRFCJSONValue("3J98t1WpEZ73CNmQviecrnyiWrnqRhWNL"), std::runtime_error); -} - BOOST_AUTO_TEST_CASE(rpc_ban) { BOOST_CHECK_NO_THROW(CallRPC(std::string("clearbanned"))); @@ -506,6 +499,53 @@ BOOST_AUTO_TEST_CASE(rpc_getblockstats_calculate_percentiles_by_weight) } } +// Make sure errors are triggered appropriately if parameters have the same names. +BOOST_AUTO_TEST_CASE(check_dup_param_names) +{ + enum ParamType { POSITIONAL, NAMED, NAMED_ONLY }; + auto make_rpc = [](std::vector<std::tuple<std::string, ParamType>> param_names) { + std::vector<RPCArg> params; + std::vector<RPCArg> options; + auto push_options = [&] { if (!options.empty()) params.emplace_back(RPCArg{strprintf("options%i", params.size()), RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", std::move(options)}); }; + for (auto& [param_name, param_type] : param_names) { + if (param_type == POSITIONAL) { + push_options(); + params.emplace_back(std::move(param_name), RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "description"); + } else { + options.emplace_back(std::move(param_name), RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "description", RPCArgOptions{.also_positional = param_type == NAMED}); + } + } + push_options(); + return RPCHelpMan{"method_name", "description", params, RPCResults{}, RPCExamples{""}}; + }; + + // No errors if parameter names are unique. + make_rpc({{"p1", POSITIONAL}, {"p2", POSITIONAL}}); + make_rpc({{"p1", POSITIONAL}, {"p2", NAMED}}); + make_rpc({{"p1", POSITIONAL}, {"p2", NAMED_ONLY}}); + make_rpc({{"p1", NAMED}, {"p2", POSITIONAL}}); + make_rpc({{"p1", NAMED}, {"p2", NAMED}}); + make_rpc({{"p1", NAMED}, {"p2", NAMED_ONLY}}); + make_rpc({{"p1", NAMED_ONLY}, {"p2", POSITIONAL}}); + make_rpc({{"p1", NAMED_ONLY}, {"p2", NAMED}}); + make_rpc({{"p1", NAMED_ONLY}, {"p2", NAMED_ONLY}}); + + // Error if parameters names are duplicates, unless one parameter is + // positional and the other is named and .also_positional is true. + BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p1", POSITIONAL}}), NonFatalCheckError); + make_rpc({{"p1", POSITIONAL}, {"p1", NAMED}}); + BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p1", NAMED_ONLY}}), NonFatalCheckError); + make_rpc({{"p1", NAMED}, {"p1", POSITIONAL}}); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED}, {"p1", NAMED}}), NonFatalCheckError); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED}, {"p1", NAMED_ONLY}}), NonFatalCheckError); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", POSITIONAL}}), NonFatalCheckError); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", NAMED}}), NonFatalCheckError); + BOOST_CHECK_THROW(make_rpc({{"p1", NAMED_ONLY}, {"p1", NAMED_ONLY}}), NonFatalCheckError); + + // Make sure duplicate aliases are detected too. + BOOST_CHECK_THROW(make_rpc({{"p1", POSITIONAL}, {"p2|p1", NAMED_ONLY}}), NonFatalCheckError); +} + BOOST_AUTO_TEST_CASE(help_example) { // test different argument types diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp index fff84d24f0..c24921bf9b 100644 --- a/src/test/settings_tests.cpp +++ b/src/test/settings_tests.cpp @@ -2,7 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <util/settings.h> +#include <common/settings.h> #include <test/util/setup_common.h> #include <test/util/str.h> @@ -21,20 +21,20 @@ #include <system_error> #include <vector> -inline bool operator==(const util::SettingsValue& a, const util::SettingsValue& b) +inline bool operator==(const common::SettingsValue& a, const common::SettingsValue& b) { return a.write() == b.write(); } -inline std::ostream& operator<<(std::ostream& os, const util::SettingsValue& value) +inline std::ostream& operator<<(std::ostream& os, const common::SettingsValue& value) { os << value.write(); return os; } -inline std::ostream& operator<<(std::ostream& os, const std::pair<std::string, util::SettingsValue>& kv) +inline std::ostream& operator<<(std::ostream& os, const std::pair<std::string, common::SettingsValue>& kv) { - util::SettingsValue out(util::SettingsValue::VOBJ); + common::SettingsValue out(common::SettingsValue::VOBJ); out.__pushKV(kv.first, kv.second); os << out.write(); return os; @@ -60,7 +60,7 @@ BOOST_AUTO_TEST_CASE(ReadWrite) "null": null })"); - std::map<std::string, util::SettingsValue> expected{ + std::map<std::string, common::SettingsValue> expected{ {"string", "string"}, {"num", 5}, {"bool", true}, @@ -68,15 +68,15 @@ BOOST_AUTO_TEST_CASE(ReadWrite) }; // Check file read. - std::map<std::string, util::SettingsValue> values; + std::map<std::string, common::SettingsValue> values; std::vector<std::string> errors; - BOOST_CHECK(util::ReadSettings(path, values, errors)); + BOOST_CHECK(common::ReadSettings(path, values, errors)); BOOST_CHECK_EQUAL_COLLECTIONS(values.begin(), values.end(), expected.begin(), expected.end()); BOOST_CHECK(errors.empty()); // Check no errors if file doesn't exist. fs::remove(path); - BOOST_CHECK(util::ReadSettings(path, values, errors)); + BOOST_CHECK(common::ReadSettings(path, values, errors)); BOOST_CHECK(values.empty()); BOOST_CHECK(errors.empty()); @@ -85,29 +85,29 @@ BOOST_AUTO_TEST_CASE(ReadWrite) "dupe": "string", "dupe": "dupe" })"); - BOOST_CHECK(!util::ReadSettings(path, values, errors)); + BOOST_CHECK(!common::ReadSettings(path, values, errors)); std::vector<std::string> dup_keys = {strprintf("Found duplicate key dupe in settings file %s", fs::PathToString(path))}; BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), dup_keys.begin(), dup_keys.end()); BOOST_CHECK(values.empty()); // Check non-kv json files not allowed WriteText(path, R"("non-kv")"); - BOOST_CHECK(!util::ReadSettings(path, values, errors)); + BOOST_CHECK(!common::ReadSettings(path, values, errors)); std::vector<std::string> non_kv = {strprintf("Found non-object value \"non-kv\" in settings file %s", fs::PathToString(path))}; BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), non_kv.begin(), non_kv.end()); // Check invalid json not allowed WriteText(path, R"(invalid json)"); - BOOST_CHECK(!util::ReadSettings(path, values, errors)); + BOOST_CHECK(!common::ReadSettings(path, values, errors)); std::vector<std::string> fail_parse = {strprintf("Unable to parse settings file %s", fs::PathToString(path))}; BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), fail_parse.begin(), fail_parse.end()); } //! Check settings struct contents against expected json strings. -static void CheckValues(const util::Settings& settings, const std::string& single_val, const std::string& list_val) +static void CheckValues(const common::Settings& settings, const std::string& single_val, const std::string& list_val) { - util::SettingsValue single_value = GetSetting(settings, "section", "name", false, false, false); - util::SettingsValue list_value(util::SettingsValue::VARR); + common::SettingsValue single_value = GetSetting(settings, "section", "name", false, false, false); + common::SettingsValue list_value(common::SettingsValue::VARR); for (const auto& item : GetSettingsList(settings, "section", "name", false)) { list_value.push_back(item); } @@ -118,7 +118,7 @@ static void CheckValues(const util::Settings& settings, const std::string& singl // Simple settings merge test case. BOOST_AUTO_TEST_CASE(Simple) { - util::Settings settings; + common::Settings settings; settings.command_line_options["name"].push_back("val1"); settings.command_line_options["name"].push_back("val2"); settings.ro_config["section"]["name"].push_back(2); @@ -126,7 +126,7 @@ BOOST_AUTO_TEST_CASE(Simple) // The last given arg takes precedence when specified via commandline. CheckValues(settings, R"("val2")", R"(["val1","val2",2])"); - util::Settings settings2; + common::Settings settings2; settings2.ro_config["section"]["name"].push_back("val2"); settings2.ro_config["section"]["name"].push_back("val3"); @@ -140,7 +140,7 @@ BOOST_AUTO_TEST_CASE(Simple) // its default value. BOOST_AUTO_TEST_CASE(NullOverride) { - util::Settings settings; + common::Settings settings; settings.command_line_options["name"].push_back("value"); BOOST_CHECK_EQUAL(R"("value")", GetSetting(settings, "section", "name", false, false, false).write().c_str()); settings.forced_settings["name"] = {}; @@ -195,11 +195,11 @@ BOOST_FIXTURE_TEST_CASE(Merge, MergeTestingSetup) bool ignore_default_section_config) { std::string desc; int value_suffix = 0; - util::Settings settings; + common::Settings settings; const std::string& name = ignore_default_section_config ? "wallet" : "server"; auto push_values = [&](Action action, const char* value_prefix, const std::string& name_prefix, - std::vector<util::SettingsValue>& dest) { + std::vector<common::SettingsValue>& dest) { if (action == SET || action == SECTION_SET) { for (int i = 0; i < 2; ++i) { dest.push_back(value_prefix + ToString(++value_suffix)); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 8ca4e62e27..a09a598332 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -184,7 +184,7 @@ struct SnapshotTestSetup : TestChain100Setup { { LOCK(::cs_main); BOOST_CHECK(!chainman.IsSnapshotValidated()); - BOOST_CHECK(!node::FindSnapshotChainstateDir()); + BOOST_CHECK(!node::FindSnapshotChainstateDir(m_args.GetDataDirNet())); } size_t initial_size; @@ -234,7 +234,7 @@ struct SnapshotTestSetup : TestChain100Setup { auto_infile >> coin; })); - BOOST_CHECK(!node::FindSnapshotChainstateDir()); + BOOST_CHECK(!node::FindSnapshotChainstateDir(m_args.GetDataDirNet())); BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot( this, [](AutoFile& auto_infile, SnapshotMetadata& metadata) { @@ -258,7 +258,7 @@ struct SnapshotTestSetup : TestChain100Setup { })); BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(this)); - BOOST_CHECK(fs::exists(*node::FindSnapshotChainstateDir())); + BOOST_CHECK(fs::exists(*node::FindSnapshotChainstateDir(m_args.GetDataDirNet()))); // Ensure our active chain is the snapshot chainstate. BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash->IsNull()); @@ -271,7 +271,7 @@ struct SnapshotTestSetup : TestChain100Setup { { LOCK(::cs_main); - fs::path found = *node::FindSnapshotChainstateDir(); + fs::path found = *node::FindSnapshotChainstateDir(m_args.GetDataDirNet()); // Note: WriteSnapshotBaseBlockhash() is implicitly tested above. BOOST_CHECK_EQUAL( @@ -491,7 +491,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup) this->SetupSnapshot(); - fs::path snapshot_chainstate_dir = *node::FindSnapshotChainstateDir(); + fs::path snapshot_chainstate_dir = *node::FindSnapshotChainstateDir(m_args.GetDataDirNet()); BOOST_CHECK(fs::exists(snapshot_chainstate_dir)); BOOST_CHECK_EQUAL(snapshot_chainstate_dir, gArgs.GetDataDirNet() / "chainstate_snapshot"); @@ -565,7 +565,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup SnapshotCompletionResult res; auto mock_shutdown = [](bilingual_str msg) {}; - fs::path snapshot_chainstate_dir = *node::FindSnapshotChainstateDir(); + fs::path snapshot_chainstate_dir = *node::FindSnapshotChainstateDir(m_args.GetDataDirNet()); BOOST_CHECK(fs::exists(snapshot_chainstate_dir)); BOOST_CHECK_EQUAL(snapshot_chainstate_dir, gArgs.GetDataDirNet() / "chainstate_snapshot"); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 1ba110d9cb..9ce4a17c5e 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -876,8 +876,17 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD } ++nTransactionsUpdated; } + if (delta == 0) { + mapDeltas.erase(hash); + LogPrintf("PrioritiseTransaction: %s (%sin mempool) delta cleared\n", hash.ToString(), it == mapTx.end() ? "not " : ""); + } else { + LogPrintf("PrioritiseTransaction: %s (%sin mempool) fee += %s, new delta=%s\n", + hash.ToString(), + it == mapTx.end() ? "not " : "", + FormatMoney(nFeeDelta), + FormatMoney(delta)); + } } - LogPrintf("PrioritiseTransaction: %s fee += %s\n", hash.ToString(), FormatMoney(nFeeDelta)); } void CTxMemPool::ApplyDelta(const uint256& hash, CAmount &nFeeDelta) const @@ -896,6 +905,22 @@ void CTxMemPool::ClearPrioritisation(const uint256& hash) mapDeltas.erase(hash); } +std::vector<CTxMemPool::delta_info> CTxMemPool::GetPrioritisedTransactions() const +{ + AssertLockNotHeld(cs); + LOCK(cs); + std::vector<delta_info> result; + result.reserve(mapDeltas.size()); + for (const auto& [txid, delta] : mapDeltas) { + const auto iter{mapTx.find(txid)}; + const bool in_mempool{iter != mapTx.end()}; + std::optional<CAmount> modified_fee; + if (in_mempool) modified_fee = iter->GetModifiedFee(); + result.emplace_back(delta_info{in_mempool, delta, modified_fee, txid}); + } + return result; +} + const CTransaction* CTxMemPool::GetConflictTx(const COutPoint& prevout) const { const auto it = mapNextTx.find(prevout); diff --git a/src/txmempool.h b/src/txmempool.h index e7a42c50e3..1ea029e949 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -519,6 +519,19 @@ public: void ApplyDelta(const uint256& hash, CAmount &nFeeDelta) const EXCLUSIVE_LOCKS_REQUIRED(cs); void ClearPrioritisation(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs); + struct delta_info { + /** Whether this transaction is in the mempool. */ + const bool in_mempool; + /** The fee delta added using PrioritiseTransaction(). */ + const CAmount delta; + /** The modified fee (base fee + delta) of this entry. Only present if in_mempool=true. */ + std::optional<CAmount> modified_fee; + /** The prioritised transaction's txid. */ + const uint256 txid; + }; + /** Return a vector of all entries in mapDeltas with their corresponding delta_info. */ + std::vector<delta_info> GetPrioritisedTransactions() const EXCLUSIVE_LOCKS_REQUIRED(!cs); + /** Get the transaction in the pool that spends the same prevout */ const CTransaction* GetConflictTx(const COutPoint& prevout) const EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/univalue/test/object.cpp b/src/univalue/test/object.cpp index 5ddf300393..5fb973c67b 100644 --- a/src/univalue/test/object.cpp +++ b/src/univalue/test/object.cpp @@ -412,6 +412,33 @@ void univalue_readwrite() BOOST_CHECK_EQUAL(strJson1, v.write()); + // Valid + BOOST_CHECK(v.read("1.0") && (v.get_real() == 1.0)); + BOOST_CHECK(v.read("true") && v.get_bool()); + BOOST_CHECK(v.read("[false]") && !v[0].get_bool()); + BOOST_CHECK(v.read("{\"a\": true}") && v["a"].get_bool()); + BOOST_CHECK(v.read("{\"1\": \"true\"}") && (v["1"].get_str() == "true")); + // Valid, with leading or trailing whitespace + BOOST_CHECK(v.read(" 1.0") && (v.get_real() == 1.0)); + BOOST_CHECK(v.read("1.0 ") && (v.get_real() == 1.0)); + BOOST_CHECK(v.read("0.00000000000000000000000000000000000001e+30 ") && v.get_real() == 1e-8); + + BOOST_CHECK(!v.read(".19e-6")); //should fail, missing leading 0, therefore invalid JSON + // Invalid, initial garbage + BOOST_CHECK(!v.read("[1.0")); + BOOST_CHECK(!v.read("a1.0")); + // Invalid, trailing garbage + BOOST_CHECK(!v.read("1.0sds")); + BOOST_CHECK(!v.read("1.0]")); + // Invalid, keys have to be names + BOOST_CHECK(!v.read("{1: \"true\"}")); + BOOST_CHECK(!v.read("{true: 1}")); + BOOST_CHECK(!v.read("{[1]: 1}")); + BOOST_CHECK(!v.read("{{\"a\": \"a\"}: 1}")); + // BTC addresses should fail parsing + BOOST_CHECK(!v.read("175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W")); + BOOST_CHECK(!v.read("3J98t1WpEZ73CNmQviecrnyiWrnqRhWNL")); + /* Check for (correctly reporting) a parsing error if the initial JSON construct is followed by more stuff. Note that whitespace is, of course, exempt. */ diff --git a/src/validation.cpp b/src/validation.cpp index 5fd2d05447..d9a0fce34f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -11,7 +11,6 @@ #include <arith_uint256.h> #include <chain.h> #include <checkqueue.h> -#include <common/args.h> #include <consensus/amount.h> #include <consensus/consensus.h> #include <consensus/merkle.h> @@ -1994,8 +1993,6 @@ public: } }; -static std::array<ThresholdConditionCache, VERSIONBITS_NUM_BITS> warningcache GUARDED_BY(cs_main); - static unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const ChainstateManager& chainman) { const Consensus::Params& consensusparams = chainman.GetConsensus(); @@ -2500,7 +2497,7 @@ bool Chainstate::FlushStateToDisk( // Write blocks and block index to disk. if (fDoFullFlush || fPeriodicWrite) { // Ensure we can write block index - if (!CheckDiskSpace(gArgs.GetBlocksDirPath())) { + if (!CheckDiskSpace(m_blockman.m_opts.blocks_dir)) { return AbortNode(state, "Disk space is too low!", _("Disk space is too low!")); } { @@ -2536,7 +2533,7 @@ bool Chainstate::FlushStateToDisk( // twice (once in the log, and once in the tables). This is already // an overestimation, as most will delete an existing entry or // overwrite one. Still, use a conservative safety factor of 2. - if (!CheckDiskSpace(gArgs.GetDataDirNet(), 48 * 2 * 2 * CoinsTip().GetCacheSize())) { + if (!CheckDiskSpace(m_chainman.m_options.datadir, 48 * 2 * 2 * CoinsTip().GetCacheSize())) { return AbortNode(state, "Disk space is too low!", _("Disk space is too low!")); } // Flush the chainstate (which may refer to block index entries). @@ -2641,7 +2638,7 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew) const CBlockIndex* pindex = pindexNew; for (int bit = 0; bit < VERSIONBITS_NUM_BITS; bit++) { WarningBitsConditionChecker checker(m_chainman, bit); - ThresholdState state = checker.GetStateFor(pindex, params.GetConsensus(), warningcache.at(bit)); + ThresholdState state = checker.GetStateFor(pindex, params.GetConsensus(), m_chainman.m_warningcache.at(bit)); if (state == ThresholdState::ACTIVE || state == ThresholdState::LOCKED_IN) { const bilingual_str warning = strprintf(_("Unknown new rules activated (versionbit %i)"), bit); if (state == ThresholdState::ACTIVE) { @@ -3104,7 +3101,6 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< CBlockIndex *pindexMostWork = nullptr; CBlockIndex *pindexNewTip = nullptr; - int nStopAtHeight = gArgs.GetIntArg("-stopatheight", DEFAULT_STOPATHEIGHT); do { // Block until the validation queue drains. This should largely // never happen in normal operation, however may happen during @@ -3179,7 +3175,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< } // When we reach this point, we switched to a new tip (stored in pindexNewTip). - if (nStopAtHeight && pindexNewTip && pindexNewTip->nHeight >= nStopAtHeight) StartShutdown(); + if (m_chainman.StopAtHeight() && pindexNewTip && pindexNewTip->nHeight >= m_chainman.StopAtHeight()) StartShutdown(); if (WITH_LOCK(::cs_main, return m_disabled)) { // Background chainstate has reached the snapshot base block, so exit. @@ -5108,7 +5104,7 @@ bool ChainstateManager::ActivateSnapshot( // PopulateAndValidateSnapshot can return (in error) before the leveldb datadir // has been created, so only attempt removal if we got that far. - if (auto snapshot_datadir = node::FindSnapshotChainstateDir()) { + if (auto snapshot_datadir = node::FindSnapshotChainstateDir(m_options.datadir)) { // We have to destruct leveldb::DB in order to release the db lock, otherwise // DestroyDB() (in DeleteCoinsDBFromDisk()) will fail. See `leveldb::~DBImpl()`. // Destructing the chainstate (and so resetting the coinsviews object) does this. @@ -5588,17 +5584,12 @@ ChainstateManager::~ChainstateManager() LOCK(::cs_main); m_versionbitscache.Clear(); - - // TODO: The warning cache should probably become non-global - for (auto& i : warningcache) { - i.clear(); - } } bool ChainstateManager::DetectSnapshotChainstate(CTxMemPool* mempool) { assert(!m_snapshot_chainstate); - std::optional<fs::path> path = node::FindSnapshotChainstateDir(); + std::optional<fs::path> path = node::FindSnapshotChainstateDir(m_options.datadir); if (!path) { return false; } diff --git a/src/validation.h b/src/validation.h index 444fe72db4..cb5b291dab 100644 --- a/src/validation.h +++ b/src/validation.h @@ -65,8 +65,6 @@ struct Params; static const int MAX_SCRIPTCHECK_THREADS = 15; /** -par default (number of script-checking threads, 0 = auto) */ static const int DEFAULT_SCRIPTCHECK_THREADS = 0; -/** Default for -stopatheight */ -static const int DEFAULT_STOPATHEIGHT = 0; /** Block files containing a block-height within MIN_BLOCKS_TO_KEEP of ActiveChain().Tip() will not be pruned. */ static const unsigned int MIN_BLOCKS_TO_KEEP = 288; static const signed int DEFAULT_CHECKBLOCKS = 6; @@ -940,6 +938,8 @@ private: //! nullopt. std::optional<int> GetSnapshotBaseHeight() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + std::array<ThresholdConditionCache, VERSIONBITS_NUM_BITS> m_warningcache GUARDED_BY(::cs_main); + //! Return true if a chainstate is considered usable. //! //! This is false when a background validation chainstate has completed its @@ -960,6 +960,7 @@ public: const arith_uint256& MinimumChainWork() const { return *Assert(m_options.minimum_chain_work); } const uint256& AssumedValidBlock() const { return *Assert(m_options.assumed_valid_block); } kernel::Notifications& GetNotifications() const { return m_options.notifications; }; + int StopAtHeight() const { return m_options.stop_at_height; }; /** * Alias for ::cs_main. diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 6dce51fc12..68abdcd81e 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -668,7 +668,8 @@ void BerkeleyDatabase::ReloadDbEnv() env->ReloadDbEnv(); } -BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database, const BerkeleyBatch& batch) +BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database, const BerkeleyBatch& batch, Span<const std::byte> prefix) + : m_key_prefix(prefix.begin(), prefix.end()) { if (!database.m_db.get()) { throw std::runtime_error(STR_INTERNAL_BUG("BerkeleyDatabase does not exist")); @@ -685,19 +686,30 @@ DatabaseCursor::Status BerkeleyCursor::Next(DataStream& ssKey, DataStream& ssVal { if (m_cursor == nullptr) return Status::FAIL; // Read at cursor - SafeDbt datKey; + SafeDbt datKey(m_key_prefix.data(), m_key_prefix.size()); SafeDbt datValue; - int ret = m_cursor->get(datKey, datValue, DB_NEXT); + int ret = -1; + if (m_first && !m_key_prefix.empty()) { + ret = m_cursor->get(datKey, datValue, DB_SET_RANGE); + } else { + ret = m_cursor->get(datKey, datValue, DB_NEXT); + } + m_first = false; if (ret == DB_NOTFOUND) { return Status::DONE; } - if (ret != 0 || datKey.get_data() == nullptr || datValue.get_data() == nullptr) { + if (ret != 0) { return Status::FAIL; } + Span<const std::byte> raw_key = {AsBytePtr(datKey.get_data()), datKey.get_size()}; + if (!m_key_prefix.empty() && std::mismatch(raw_key.begin(), raw_key.end(), m_key_prefix.begin(), m_key_prefix.end()).second != m_key_prefix.end()) { + return Status::DONE; + } + // Convert to streams ssKey.clear(); - ssKey.write({AsBytePtr(datKey.get_data()), datKey.get_size()}); + ssKey.write(raw_key); ssValue.clear(); ssValue.write({AsBytePtr(datValue.get_data()), datValue.get_size()}); return Status::MORE; @@ -716,6 +728,12 @@ std::unique_ptr<DatabaseCursor> BerkeleyBatch::GetNewCursor() return std::make_unique<BerkeleyCursor>(m_database, *this); } +std::unique_ptr<DatabaseCursor> BerkeleyBatch::GetNewPrefixCursor(Span<const std::byte> prefix) +{ + if (!pdb) return nullptr; + return std::make_unique<BerkeleyCursor>(m_database, *this, prefix); +} + bool BerkeleyBatch::TxnBegin() { if (!pdb || activeTxn) @@ -777,6 +795,7 @@ bool BerkeleyBatch::ReadKey(DataStream&& key, DataStream& value) SafeDbt datValue; int ret = pdb->get(activeTxn, datKey, datValue, 0); if (ret == 0 && datValue.get_data() != nullptr) { + value.clear(); value.write({AsBytePtr(datValue.get_data()), datValue.get_size()}); return true; } diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index e8a57e8a5e..8cc03692d6 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -190,9 +190,13 @@ class BerkeleyCursor : public DatabaseCursor { private: Dbc* m_cursor; + std::vector<std::byte> m_key_prefix; + bool m_first{true}; public: - explicit BerkeleyCursor(BerkeleyDatabase& database, const BerkeleyBatch& batch); + // Constructor for cursor for records matching the prefix + // To match all records, an empty prefix may be provided. + explicit BerkeleyCursor(BerkeleyDatabase& database, const BerkeleyBatch& batch, Span<const std::byte> prefix = {}); ~BerkeleyCursor() override; Status Next(DataStream& key, DataStream& value) override; @@ -229,6 +233,7 @@ public: void Close() override; std::unique_ptr<DatabaseCursor> GetNewCursor() override; + std::unique_ptr<DatabaseCursor> GetNewPrefixCursor(Span<const std::byte> prefix) override; bool TxnBegin() override; bool TxnCommit() override; bool TxnAbort() override; diff --git a/src/wallet/db.h b/src/wallet/db.h index b4ccd13a9a..9d684225c3 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -113,6 +113,7 @@ public: virtual bool ErasePrefix(Span<const std::byte> prefix) = 0; virtual std::unique_ptr<DatabaseCursor> GetNewCursor() = 0; + virtual std::unique_ptr<DatabaseCursor> GetNewPrefixCursor(Span<const std::byte> prefix) = 0; virtual bool TxnBegin() = 0; virtual bool TxnCommit() = 0; virtual bool TxnAbort() = 0; diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index 865af0bb23..44c93eed7c 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -255,11 +255,7 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs:: std::vector<unsigned char> k = ParseHex(key); std::vector<unsigned char> v = ParseHex(value); - - DataStream ss_key{k}; - DataStream ss_value{v}; - - if (!batch->Write(ss_key, ss_value)) { + if (!batch->Write(Span{k}, Span{v})) { error = strprintf(_("Error: Unable to write record to new wallet")); ret = false; break; diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 2560dda87c..4cdfadbee2 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -62,7 +62,7 @@ bool VerifyWallets(WalletContext& context) options.require_existing = true; options.verify = false; if (MakeWalletDatabase("", options, status, error_string)) { - util::SettingsValue wallets(util::SettingsValue::VARR); + common::SettingsValue wallets(common::SettingsValue::VARR); wallets.push_back(""); // Default wallet name is "" // Pass write=false because no need to write file and probably // better not to. If unnamed wallet needs to be added next startup diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 43da15b36e..b93419292e 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1298,7 +1298,7 @@ RPCHelpMan importmulti() }, }, RPCArgOptions{.oneline_description="\"requests\""}}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", { {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions after all imports."}, }, diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 5bdd3c9e72..22f0f0b83c 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -513,7 +513,7 @@ RPCHelpMan listunspent() }, {"include_unsafe", RPCArg::Type::BOOL, RPCArg::Default{true}, "Include outputs that are not safe to spend\n" "See description of \"safe\" attribute below."}, - {"query_options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "JSON with query options", + {"query_options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", { {"minimumAmount", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(0)}, "Minimum value of each UTXO in " + CURRENCY_UNIT + ""}, {"maximumAmount", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"unlimited"}, "Maximum value of each UTXO in " + CURRENCY_UNIT + ""}, diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index a5b1f594bf..feee643f26 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -453,9 +453,9 @@ RPCHelpMan settxfee() static std::vector<RPCArg> FundTxDoc(bool solving_data = true) { std::vector<RPCArg> args = { - {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"}, + {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks", RPCArgOptions{.also_positional = true}}, {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n" - "\"" + FeeModes("\"\n\"") + "\""}, + "\"" + FeeModes("\"\n\"") + "\"", RPCArgOptions{.also_positional = true}}, { "replaceable", RPCArg::Type::BOOL, RPCArg::DefaultHint{"wallet default"}, "Marks this transaction as BIP125-replaceable.\n" "Allows this transaction to be replaced by a transaction with higher fees" @@ -758,7 +758,7 @@ RPCHelpMan fundrawtransaction() "Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "for backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}", + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "For backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}", Cat<std::vector<RPCArg>>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::Default{true}, "For a transaction with existing inputs, automatically include more if they are not enough."}, @@ -997,7 +997,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) "* WARNING: before version 0.21, fee_rate was in " + CURRENCY_UNIT + "/kvB. As of 0.21, fee_rate is in " + CURRENCY_ATOM + "/vB. *\n", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The txid to be bumped"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", { {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks\n"}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, @@ -1187,7 +1187,7 @@ RPCHelpMan send() {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n" "\"" + FeeModes("\"\n\"") + "\""}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", Cat<std::vector<RPCArg>>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::DefaultHint{"false when \"inputs\" are specified, true otherwise"},"Automatically include coins from the wallet to cover the target amount.\n"}, @@ -1200,7 +1200,7 @@ RPCHelpMan send() {"change_address", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The bitcoin address to receive the change"}, {"change_position", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"}, {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if change_address is not specified. Options are \"legacy\", \"p2sh-segwit\", \"bech32\" and \"bech32m\"."}, - {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, + {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB.", RPCArgOptions{.also_positional = true}}, {"include_watching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also select inputs which are watch only.\n" "Only solvable inputs can be used. Watch-only destinations are solvable if the public key and/or output script was imported,\n" "e.g. with 'importpubkey' or 'importmulti' with the 'pubkeys' or 'desc' field."}, @@ -1302,11 +1302,11 @@ RPCHelpMan sendall() "\"" + FeeModes("\"\n\"") + "\""}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, { - "options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + "options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", Cat<std::vector<RPCArg>>( { {"add_to_wallet", RPCArg::Type::BOOL, RPCArg::Default{true}, "When false, returns the serialized transaction without broadcasting or adding it to the wallet"}, - {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, + {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB.", RPCArgOptions{.also_positional = true}}, {"include_watching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also select inputs which are watch-only.\n" "Only solvable inputs can be used. Watch-only destinations are solvable if the public key and/or output script was imported,\n" "e.g. with 'importpubkey' or 'importmulti' with the 'pubkeys' or 'desc' field."}, @@ -1635,7 +1635,7 @@ RPCHelpMan walletcreatefundedpsbt() OutputsDoc(), RPCArgOptions{.skip_type_check = true}}, {"locktime", RPCArg::Type::NUM, RPCArg::Default{0}, "Raw locktime. Non-0 value also locktime-activates inputs"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", Cat<std::vector<RPCArg>>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::DefaultHint{"false when \"inputs\" are specified, true otherwise"}, "Automatically include coins from the wallet to cover the target amount.\n"}, diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index a22862bfa9..e0f246e2f2 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -645,7 +645,7 @@ RPCHelpMan simulaterawtransaction() {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""}, }, }, - {"options", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "Options", + {"options", RPCArg::Type::OBJ_NAMED_PARAMS, RPCArg::Optional::OMITTED, "", { {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Whether to include watch-only addresses (see RPC importaddress)"}, }, diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index ab73e67285..e303310273 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -43,6 +43,7 @@ public: void Close() override {} std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<DummyCursor>(); } + std::unique_ptr<DatabaseCursor> GetNewPrefixCursor(Span<const std::byte> prefix) override { return GetNewCursor(); } bool TxnBegin() override { return true; } bool TxnCommit() override { return true; } bool TxnAbort() override { return true; } diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 77e8a4e9c1..8d7fe97bb1 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -9,6 +9,7 @@ #include <logging.h> #include <sync.h> #include <util/fs_helpers.h> +#include <util/check.h> #include <util/strencodings.h> #include <util/translation.h> #include <wallet/db.h> @@ -34,12 +35,31 @@ static void ErrorLogCallback(void* arg, int code, const char* msg) LogPrintf("SQLite Error. Code: %d. Message: %s\n", code, msg); } +static int TraceSqlCallback(unsigned code, void* context, void* param1, void* param2) +{ + auto* db = static_cast<SQLiteDatabase*>(context); + if (code == SQLITE_TRACE_STMT) { + auto* stmt = static_cast<sqlite3_stmt*>(param1); + // To be conservative and avoid leaking potentially secret information + // in the log file, only expand statements that query the database, not + // statements that update the database. + char* expanded{sqlite3_stmt_readonly(stmt) ? sqlite3_expanded_sql(stmt) : nullptr}; + LogPrintf("[%s] SQLite Statement: %s\n", db->Filename(), expanded ? expanded : sqlite3_sql(stmt)); + if (expanded) sqlite3_free(expanded); + } + return SQLITE_OK; +} + static bool BindBlobToStatement(sqlite3_stmt* stmt, int index, Span<const std::byte> blob, const std::string& description) { - int res = sqlite3_bind_blob(stmt, index, blob.data(), blob.size(), SQLITE_STATIC); + // Pass a pointer to the empty string "" below instead of passing the + // blob.data() pointer if the blob.data() pointer is null. Passing a null + // data pointer to bind_blob would cause sqlite to bind the SQL NULL value + // instead of the empty blob value X'', which would mess up SQL comparisons. + int res = sqlite3_bind_blob(stmt, index, blob.data() ? static_cast<const void*>(blob.data()) : "", blob.size(), SQLITE_STATIC); if (res != SQLITE_OK) { LogPrintf("Unable to bind %s to statement: %s\n", description, sqlite3_errstr(res)); sqlite3_clear_bindings(stmt); @@ -235,6 +255,13 @@ void SQLiteDatabase::Open() if (ret != SQLITE_OK) { throw std::runtime_error(strprintf("SQLiteDatabase: Failed to enable extended result codes: %s\n", sqlite3_errstr(ret))); } + // Trace SQL statements if tracing is enabled with -debug=walletdb -loglevel=walletdb:trace + if (LogAcceptCategory(BCLog::WALLETDB, BCLog::Level::Trace)) { + ret = sqlite3_trace_v2(m_db, SQLITE_TRACE_STMT, TraceSqlCallback, this); + if (ret != SQLITE_OK) { + LogPrintf("Failed to enable SQL tracing for %s\n", Filename()); + } + } } if (sqlite3_db_readonly(m_db, "main") != 0) { @@ -409,6 +436,7 @@ bool SQLiteBatch::ReadKey(DataStream&& key, DataStream& value) // Leftmost column in result is index 0 const std::byte* data{AsBytePtr(sqlite3_column_blob(m_read_stmt, 0))}; size_t data_size(sqlite3_column_bytes(m_read_stmt, 0)); + value.clear(); value.write({data, data_size}); sqlite3_clear_bindings(m_read_stmt); @@ -495,6 +523,9 @@ DatabaseCursor::Status SQLiteCursor::Next(DataStream& key, DataStream& value) return Status::FAIL; } + key.clear(); + value.clear(); + // Leftmost column in result is index 0 const std::byte* key_data{AsBytePtr(sqlite3_column_blob(m_cursor_stmt, 0))}; size_t key_data_size(sqlite3_column_bytes(m_cursor_stmt, 0)); @@ -507,6 +538,7 @@ DatabaseCursor::Status SQLiteCursor::Next(DataStream& key, DataStream& value) SQLiteCursor::~SQLiteCursor() { + sqlite3_clear_bindings(m_cursor_stmt); sqlite3_reset(m_cursor_stmt); int res = sqlite3_finalize(m_cursor_stmt); if (res != SQLITE_OK) { @@ -530,6 +562,48 @@ std::unique_ptr<DatabaseCursor> SQLiteBatch::GetNewCursor() return cursor; } +std::unique_ptr<DatabaseCursor> SQLiteBatch::GetNewPrefixCursor(Span<const std::byte> prefix) +{ + if (!m_database.m_db) return nullptr; + + // To get just the records we want, the SQL statement does a comparison of the binary data + // where the data must be greater than or equal to the prefix, and less than + // the prefix incremented by one (when interpreted as an integer) + std::vector<std::byte> start_range(prefix.begin(), prefix.end()); + std::vector<std::byte> end_range(prefix.begin(), prefix.end()); + auto it = end_range.rbegin(); + for (; it != end_range.rend(); ++it) { + if (*it == std::byte(std::numeric_limits<unsigned char>::max())) { + *it = std::byte(0); + continue; + } + *it = std::byte(std::to_integer<unsigned char>(*it) + 1); + break; + } + if (it == end_range.rend()) { + // If the prefix is all 0xff bytes, clear end_range as we won't need it + end_range.clear(); + } + + auto cursor = std::make_unique<SQLiteCursor>(start_range, end_range); + if (!cursor) return nullptr; + + const char* stmt_text = end_range.empty() ? "SELECT key, value FROM main WHERE key >= ?" : + "SELECT key, value FROM main WHERE key >= ? AND key < ?"; + int res = sqlite3_prepare_v2(m_database.m_db, stmt_text, -1, &cursor->m_cursor_stmt, nullptr); + if (res != SQLITE_OK) { + throw std::runtime_error(strprintf( + "SQLiteDatabase: Failed to setup cursor SQL statement: %s\n", sqlite3_errstr(res))); + } + + if (!BindBlobToStatement(cursor->m_cursor_stmt, 1, cursor->m_prefix_range_start, "prefix_start")) return nullptr; + if (!end_range.empty()) { + if (!BindBlobToStatement(cursor->m_cursor_stmt, 2, cursor->m_prefix_range_end, "prefix_end")) return nullptr; + } + + return cursor; +} + bool SQLiteBatch::TxnBegin() { if (!m_database.m_db || sqlite3_get_autocommit(m_database.m_db) == 0) return false; diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index d9de40569b..0378bbb8d6 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -15,12 +15,21 @@ struct bilingual_str; namespace wallet { class SQLiteDatabase; +/** RAII class that provides a database cursor */ class SQLiteCursor : public DatabaseCursor { public: sqlite3_stmt* m_cursor_stmt{nullptr}; + // Copies of the prefix things for the prefix cursor. + // Prevents SQLite from accessing temp variables for the prefix things. + std::vector<std::byte> m_prefix_range_start; + std::vector<std::byte> m_prefix_range_end; explicit SQLiteCursor() {} + explicit SQLiteCursor(std::vector<std::byte> start_range, std::vector<std::byte> end_range) + : m_prefix_range_start(std::move(start_range)), + m_prefix_range_end(std::move(end_range)) + {} ~SQLiteCursor() override; Status Next(DataStream& key, DataStream& value) override; @@ -57,6 +66,7 @@ public: void Close() override; std::unique_ptr<DatabaseCursor> GetNewCursor() override; + std::unique_ptr<DatabaseCursor> GetNewPrefixCursor(Span<const std::byte> prefix) override; bool TxnBegin() override; bool TxnCommit() override; bool TxnAbort() override; diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index 7761308bbc..4cda35ed8d 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -6,13 +6,56 @@ #include <test/util/setup_common.h> #include <util/fs.h> +#include <util/translation.h> +#ifdef USE_BDB #include <wallet/bdb.h> +#endif +#ifdef USE_SQLITE +#include <wallet/sqlite.h> +#endif +#include <wallet/test/util.h> +#include <wallet/walletutil.h> // for WALLET_FLAG_DESCRIPTORS #include <fstream> #include <memory> #include <string> +inline std::ostream& operator<<(std::ostream& os, const std::pair<const SerializeData, SerializeData>& kv) +{ + Span key{kv.first}, value{kv.second}; + os << "(\"" << std::string_view{reinterpret_cast<const char*>(key.data()), key.size()} << "\", \"" + << std::string_view{reinterpret_cast<const char*>(key.data()), key.size()} << "\")"; + return os; +} + namespace wallet { + +static Span<const std::byte> StringBytes(std::string_view str) +{ + return AsBytes<const char>({str.data(), str.size()}); +} + +static SerializeData StringData(std::string_view str) +{ + auto bytes = StringBytes(str); + return SerializeData{bytes.begin(), bytes.end()}; +} + +static void CheckPrefix(DatabaseBatch& batch, Span<const std::byte> prefix, MockableData expected) +{ + std::unique_ptr<DatabaseCursor> cursor = batch.GetNewPrefixCursor(prefix); + MockableData actual; + while (true) { + DataStream key, value; + DatabaseCursor::Status status = cursor->Next(key, value); + if (status == DatabaseCursor::Status::DONE) break; + BOOST_CHECK(status == DatabaseCursor::Status::MORE); + BOOST_CHECK( + actual.emplace(SerializeData(key.begin(), key.end()), SerializeData(value.begin(), value.end())).second); + } + BOOST_CHECK_EQUAL_COLLECTIONS(actual.begin(), actual.end(), expected.begin(), expected.end()); +} + BOOST_FIXTURE_TEST_SUITE(db_tests, BasicTestingSetup) static std::shared_ptr<BerkeleyEnvironment> GetWalletEnv(const fs::path& path, fs::path& database_filename) @@ -78,5 +121,90 @@ BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_free_instance) BOOST_CHECK(env_2_a == env_2_b); } +static std::vector<std::unique_ptr<WalletDatabase>> TestDatabases(const fs::path& path_root) +{ + std::vector<std::unique_ptr<WalletDatabase>> dbs; + DatabaseOptions options; + DatabaseStatus status; + bilingual_str error; +#ifdef USE_BDB + dbs.emplace_back(MakeBerkeleyDatabase(path_root / "bdb", options, status, error)); +#endif +#ifdef USE_SQLITE + dbs.emplace_back(MakeSQLiteDatabase(path_root / "sqlite", options, status, error)); +#endif + dbs.emplace_back(CreateMockableWalletDatabase()); + return dbs; +} + +BOOST_AUTO_TEST_CASE(db_cursor_prefix_range_test) +{ + // Test each supported db + for (const auto& database : TestDatabases(m_path_root)) { + BOOST_ASSERT(database); + + std::vector<std::string> prefixes = {"", "FIRST", "SECOND", "P\xfe\xff", "P\xff\x01", "\xff\xff"}; + + // Write elements to it + std::unique_ptr<DatabaseBatch> handler = database->MakeBatch(); + for (unsigned int i = 0; i < 10; i++) { + for (const auto& prefix : prefixes) { + BOOST_CHECK(handler->Write(std::make_pair(prefix, i), i)); + } + } + + // Now read all the items by prefix and verify that each element gets parsed correctly + for (const auto& prefix : prefixes) { + DataStream s_prefix; + s_prefix << prefix; + std::unique_ptr<DatabaseCursor> cursor = handler->GetNewPrefixCursor(s_prefix); + DataStream key; + DataStream value; + for (int i = 0; i < 10; i++) { + DatabaseCursor::Status status = cursor->Next(key, value); + BOOST_ASSERT(status == DatabaseCursor::Status::MORE); + + std::string key_back; + unsigned int i_back; + key >> key_back >> i_back; + BOOST_CHECK_EQUAL(key_back, prefix); + + unsigned int value_back; + value >> value_back; + BOOST_CHECK_EQUAL(value_back, i_back); + } + + // Let's now read it once more, it should return DONE + BOOST_CHECK(cursor->Next(key, value) == DatabaseCursor::Status::DONE); + } + } +} + +// Lower level DatabaseBase::GetNewPrefixCursor test, to cover cases that aren't +// covered in the higher level test above. The higher level test uses +// serialized strings which are prefixed with string length, so it doesn't test +// truly empty prefixes or prefixes that begin with \xff +BOOST_AUTO_TEST_CASE(db_cursor_prefix_byte_test) +{ + const MockableData::value_type + e{StringData(""), StringData("e")}, + p{StringData("prefix"), StringData("p")}, + ps{StringData("prefixsuffix"), StringData("ps")}, + f{StringData("\xff"), StringData("f")}, + fs{StringData("\xffsuffix"), StringData("fs")}, + ff{StringData("\xff\xff"), StringData("ff")}, + ffs{StringData("\xff\xffsuffix"), StringData("ffs")}; + for (const auto& database : TestDatabases(m_path_root)) { + std::unique_ptr<DatabaseBatch> batch = database->MakeBatch(); + for (const auto& [k, v] : {e, p, ps, f, fs, ff, ffs}) { + batch->Write(MakeUCharSpan(k), MakeUCharSpan(v)); + } + CheckPrefix(*batch, StringBytes(""), {e, p, ps, f, fs, ff, ffs}); + CheckPrefix(*batch, StringBytes("prefix"), {p, ps}); + CheckPrefix(*batch, StringBytes("\xff"), {f, fs, ff, ffs}); + CheckPrefix(*batch, StringBytes("\xff\xff"), {ff, ffs}); + } +} + BOOST_AUTO_TEST_SUITE_END() } // namespace wallet diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index eacb70cd69..069ab25f26 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -92,6 +92,17 @@ CTxDestination getNewDestination(CWallet& w, OutputType output_type) return *Assert(w.GetNewDestination(output_type, "")); } +// BytePrefix compares equality with other byte spans that begin with the same prefix. +struct BytePrefix { Span<const std::byte> prefix; }; +bool operator<(BytePrefix a, Span<const std::byte> b) { return a.prefix < b.subspan(0, std::min(a.prefix.size(), b.size())); } +bool operator<(Span<const std::byte> a, BytePrefix b) { return a.subspan(0, std::min(a.size(), b.prefix.size())) < b.prefix; } + +MockableCursor::MockableCursor(const MockableData& records, bool pass, Span<const std::byte> prefix) +{ + m_pass = pass; + std::tie(m_cursor, m_cursor_end) = records.equal_range(BytePrefix{prefix}); +} + DatabaseCursor::Status MockableCursor::Next(DataStream& key, DataStream& value) { if (!m_pass) { @@ -100,6 +111,8 @@ DatabaseCursor::Status MockableCursor::Next(DataStream& key, DataStream& value) if (m_cursor == m_cursor_end) { return Status::DONE; } + key.clear(); + value.clear(); const auto& [key_data, value_data] = *m_cursor; key.write(key_data); value.write(value_data); @@ -117,6 +130,7 @@ bool MockableBatch::ReadKey(DataStream&& key, DataStream& value) if (it == m_records.end()) { return false; } + value.clear(); value.write(it->second); return true; } @@ -172,7 +186,7 @@ bool MockableBatch::ErasePrefix(Span<const std::byte> prefix) return true; } -std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase(std::map<SerializeData, SerializeData> records) +std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase(MockableData records) { return std::make_unique<MockableDatabase>(records); } diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index b1ad1c959b..2a1fe639de 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -48,14 +48,17 @@ std::string getnewaddress(CWallet& w); /** Returns a new destination, of an specific type, from the wallet */ CTxDestination getNewDestination(CWallet& w, OutputType output_type); +using MockableData = std::map<SerializeData, SerializeData, std::less<>>; + class MockableCursor: public DatabaseCursor { public: - std::map<SerializeData, SerializeData>::const_iterator m_cursor; - std::map<SerializeData, SerializeData>::const_iterator m_cursor_end; + MockableData::const_iterator m_cursor; + MockableData::const_iterator m_cursor_end; bool m_pass; - explicit MockableCursor(const std::map<SerializeData, SerializeData>& records, bool pass) : m_cursor(records.begin()), m_cursor_end(records.end()), m_pass(pass) {} + explicit MockableCursor(const MockableData& records, bool pass) : m_cursor(records.begin()), m_cursor_end(records.end()), m_pass(pass) {} + MockableCursor(const MockableData& records, bool pass, Span<const std::byte> prefix); ~MockableCursor() {} Status Next(DataStream& key, DataStream& value) override; @@ -64,7 +67,7 @@ public: class MockableBatch : public DatabaseBatch { private: - std::map<SerializeData, SerializeData>& m_records; + MockableData& m_records; bool m_pass; bool ReadKey(DataStream&& key, DataStream& value) override; @@ -74,7 +77,7 @@ private: bool ErasePrefix(Span<const std::byte> prefix) override; public: - explicit MockableBatch(std::map<SerializeData, SerializeData>& records, bool pass) : m_records(records), m_pass(pass) {} + explicit MockableBatch(MockableData& records, bool pass) : m_records(records), m_pass(pass) {} ~MockableBatch() {} void Flush() override {} @@ -84,6 +87,9 @@ public: { return std::make_unique<MockableCursor>(m_records, m_pass); } + std::unique_ptr<DatabaseCursor> GetNewPrefixCursor(Span<const std::byte> prefix) override { + return std::make_unique<MockableCursor>(m_records, m_pass, prefix); + } bool TxnBegin() override { return m_pass; } bool TxnCommit() override { return m_pass; } bool TxnAbort() override { return m_pass; } @@ -94,10 +100,10 @@ public: class MockableDatabase : public WalletDatabase { public: - std::map<SerializeData, SerializeData> m_records; + MockableData m_records; bool m_pass{true}; - MockableDatabase(std::map<SerializeData, SerializeData> records = {}) : WalletDatabase(), m_records(records) {} + MockableDatabase(MockableData records = {}) : WalletDatabase(), m_records(records) {} ~MockableDatabase() {}; void Open() override {} @@ -117,7 +123,7 @@ public: std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override { return std::make_unique<MockableBatch>(m_records, m_pass); } }; -std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase(std::map<SerializeData, SerializeData> records = {}); +std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase(MockableData records = {}); MockableDatabase& GetMockableDatabase(CWallet& wallet); } // namespace wallet diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index ab1184556c..73a4b77188 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -83,7 +83,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_ckey, TestingSetup) { SerializeData ckey_record_key; SerializeData ckey_record_value; - std::map<SerializeData, SerializeData> records; + MockableData records; { // Context setup. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 63ace5c47f..62f0f53b01 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -56,9 +56,9 @@ namespace wallet { bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name) { - util::SettingsValue setting_value = chain.getRwSetting("wallet"); + common::SettingsValue setting_value = chain.getRwSetting("wallet"); if (!setting_value.isArray()) setting_value.setArray(); - for (const util::SettingsValue& value : setting_value.getValues()) { + for (const common::SettingsValue& value : setting_value.getValues()) { if (value.isStr() && value.get_str() == wallet_name) return true; } setting_value.push_back(wallet_name); @@ -67,10 +67,10 @@ bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name) bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name) { - util::SettingsValue setting_value = chain.getRwSetting("wallet"); + common::SettingsValue setting_value = chain.getRwSetting("wallet"); if (!setting_value.isArray()) return true; - util::SettingsValue new_value(util::SettingsValue::VARR); - for (const util::SettingsValue& value : setting_value.getValues()) { + common::SettingsValue new_value(common::SettingsValue::VARR); + for (const common::SettingsValue& value : setting_value.getValues()) { if (!value.isStr() || value.get_str() != wallet_name) new_value.push_back(value); } if (new_value.size() == setting_value.size()) return true; @@ -2832,7 +2832,7 @@ bool CWallet::SetAddressPreviouslySpent(WalletBatch& batch, const CTxDestination return false; if (!used) { - if (auto* data{util::FindKey(m_address_book, dest)}) data->previously_spent = false; + if (auto* data{common::FindKey(m_address_book, dest)}) data->previously_spent = false; return batch.WriteAddressPreviouslySpent(dest, false); } @@ -2852,7 +2852,7 @@ void CWallet::LoadAddressReceiveRequest(const CTxDestination& dest, const std::s bool CWallet::IsAddressPreviouslySpent(const CTxDestination& dest) const { - if (auto* data{util::FindKey(m_address_book, dest)}) return data->previously_spent; + if (auto* data{common::FindKey(m_address_book, dest)}) return data->previously_spent; return false; } @@ -3875,9 +3875,7 @@ bool CWallet::MigrateToSQLite(bilingual_str& error) bool began = batch->TxnBegin(); assert(began); // This is a critical error, the new db could not be written to. The original db exists as a backup, but we should not continue execution. for (const auto& [key, value] : records) { - DataStream ss_key{key}; - DataStream ss_value{value}; - if (!batch->Write(ss_key, ss_value)) { + if (!batch->Write(MakeUCharSpan(key), MakeUCharSpan(value))) { batch->TxnAbort(); m_database->Close(); fs::remove(m_database->Filename()); |