diff options
30 files changed, 498 insertions, 206 deletions
diff --git a/contrib/devtools/pixie.py b/contrib/devtools/pixie.py index 8cf06a799a..64660968ad 100644 --- a/contrib/devtools/pixie.py +++ b/contrib/devtools/pixie.py @@ -217,7 +217,7 @@ def _parse_verneed(section: Section, strings: bytes, eh: ELFHeader) -> Dict[int, result = {} while True: verneed = Verneed(data, ofs, eh) - aofs = verneed.vn_aux + aofs = ofs + verneed.vn_aux while True: vernaux = Vernaux(data, aofs, eh, strings) result[vernaux.vna_other] = vernaux.name diff --git a/contrib/devtools/symbol-check.py b/contrib/devtools/symbol-check.py index 7a5a42c5d2..56e4313d78 100755 --- a/contrib/devtools/symbol-check.py +++ b/contrib/devtools/symbol-check.py @@ -41,8 +41,16 @@ import pixie # MAX_VERSIONS = { 'GCC': (4,8,0), -'GLIBC': (2,17), -'LIBATOMIC': (1,0) +'GLIBC': { + pixie.EM_386: (2,17), + pixie.EM_X86_64: (2,17), + pixie.EM_ARM: (2,17), + pixie.EM_AARCH64:(2,17), + pixie.EM_PPC64: (2,17), + pixie.EM_RISCV: (2,27), +}, +'LIBATOMIC': (1,0), +'V': (0,5,0), # xkb (bitcoin-qt only) } # See here for a description of _IO_stdin_used: # https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=634261#109 @@ -78,14 +86,6 @@ ELF_ALLOWED_LIBRARIES = { 'libfreetype.so.6', # font parsing 'libdl.so.2' # programming interface to dynamic linker } -ARCH_MIN_GLIBC_VER = { -pixie.EM_386: (2,1), -pixie.EM_X86_64: (2,2,5), -pixie.EM_ARM: (2,4), -pixie.EM_AARCH64:(2,17), -pixie.EM_PPC64: (2,17), -pixie.EM_RISCV: (2,27) -} MACHO_ALLOWED_LIBRARIES = { # bitcoind and bitcoin-qt @@ -161,7 +161,10 @@ def check_version(max_versions, version, arch) -> bool: ver = tuple([int(x) for x in ver.split('.')]) if not lib in max_versions: return False - return ver <= max_versions[lib] or lib == 'GLIBC' and ver <= ARCH_MIN_GLIBC_VER[arch] + if isinstance(max_versions[lib], tuple): + return ver <= max_versions[lib] + else: + return ver <= max_versions[lib][arch] def check_imported_symbols(filename) -> bool: elf = pixie.load(filename) diff --git a/doc/files.md b/doc/files.md index 353efe348d..e670d77ae5 100644 --- a/doc/files.md +++ b/doc/files.md @@ -56,7 +56,8 @@ Subdirectory | File(s) | Description `indexes/coinstats/db/` | LevelDB database | Coinstats index; *optional*, used if `-coinstatsindex=1` `wallets/` | | [Contains wallets](#multi-wallet-environment); can be specified by `-walletdir` option; if `wallets/` subdirectory does not exist, wallets reside in the [data directory](#data-directory-location) `./` | `anchors.dat` | Anchor IP address database, created on shutdown and deleted at startup. Anchors are last known outgoing block-relay-only peers that are tried to re-connect to on startup -`./` | `banlist.dat` | Stores the IPs/subnets of banned nodes +`./` | `banlist.dat` | Stores the addresses/subnets of banned nodes (deprecated). `bitcoind` or `bitcoin-qt` no longer save the banlist to this file, but read it on startup if `banlist.json` is not present. +`./` | `banlist.json` | Stores the addresses/subnets of banned nodes. `./` | `bitcoin.conf` | User-defined [configuration settings](bitcoin-conf.md) for `bitcoind` or `bitcoin-qt`. File is not written to by the software and must be created manually. Path can be specified by `-conf` option `./` | `bitcoind.pid` | Stores the process ID (PID) of `bitcoind` or `bitcoin-qt` while running; created at start and deleted on shutdown; can be specified by `-pid` option `./` | `debug.log` | Contains debug information and general logging generated by `bitcoind` or `bitcoin-qt`; can be specified by `-debuglogfile` option @@ -109,7 +110,7 @@ Subdirectory | File | Description ## Legacy subdirectories and files -These subdirectories and files are no longer used by the Bitcoin Core: +These subdirectories and files are no longer used by Bitcoin Core: Path | Description | Repository notes ---------------|-------------|----------------- diff --git a/doc/release-notes-20833.md b/doc/release-notes-20833.md deleted file mode 100644 index 9a02bbd275..0000000000 --- a/doc/release-notes-20833.md +++ /dev/null @@ -1,12 +0,0 @@ -Updated RPCs ------------- - -- The `testmempoolaccept` RPC now accepts multiple transactions (still experimental at the moment, - API may be unstable). This is intended for testing transaction packages with dependency - relationships; it is not recommended for batch-validating independent transactions. In addition to - mempool policy, package policies apply: the list cannot contain more than 25 transactions or have a - total size exceeding 101K virtual bytes, and cannot conflict with (spend the same inputs as) each other or - the mempool, even if it would be a valid BIP125 replace-by-fee. There are some known limitations to - the accuracy of the test accept: it's possible for `testmempoolaccept` to return "allowed"=True for a - group of transactions, but "too-long-mempool-chain" if they are actually submitted. (#20833) - diff --git a/doc/release-notes-20867.md b/doc/release-notes-20867.md deleted file mode 100644 index 60eed6838f..0000000000 --- a/doc/release-notes-20867.md +++ /dev/null @@ -1,11 +0,0 @@ -Wallet ------- - -- We now support up to 20 keys in `multi()` and `sortedmulti()` descriptors - under `wsh()`. (#20867) - -Updated RPCs ------------- - -- `addmultisigaddress` and `createmultisig` now support up to 20 keys for - Segwit addresses. diff --git a/doc/release-notes.md b/doc/release-notes.md index cb7adde470..dc28ccb9ed 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -1,3 +1,5 @@ +# Release notes now being edited on https://github.com/bitcoin-core/bitcoin-devwiki/wiki/22.0-Release-Notes-draft + *After branching off for a major version release of Bitcoin Core, use this template to create the initial release notes draft.* @@ -121,6 +123,18 @@ Updated RPCs - `getnodeaddresses` now also accepts a "network" argument (ipv4, ipv6, onion, or i2p) to return only addresses of the specified network. (#21843) +- The `testmempoolaccept` RPC now accepts multiple transactions (still experimental at the moment, + API may be unstable). This is intended for testing transaction packages with dependency + relationships; it is not recommended for batch-validating independent transactions. In addition to + mempool policy, package policies apply: the list cannot contain more than 25 transactions or have a + total size exceeding 101K virtual bytes, and cannot conflict with (spend the same inputs as) each other or + the mempool, even if it would be a valid BIP125 replace-by-fee. There are some known limitations to + the accuracy of the test accept: it's possible for `testmempoolaccept` to return "allowed"=True for a + group of transactions, but "too-long-mempool-chain" if they are actually submitted. (#20833) + +- `addmultisigaddress` and `createmultisig` now support up to 20 keys for + Segwit addresses. (#20867) + Changes to Wallet or GUI related RPCs can be found in the GUI or Wallet section below. New RPCs @@ -152,6 +166,10 @@ Tools and Utilities like `-onlynet=<network>` or to upgrade to this release of Bitcoin Core 22.0 that supports Tor v3 only. (#21595) +- A new `-rpcwaittimeout` argument to `bitcoin-cli` sets the timeout + in seconds to use with `-rpcwait`. If the timeout expires, + `bitcoin-cli` will report a failure. (#21056) + Wallet ------ @@ -167,6 +185,9 @@ Wallet Note that the resulting transaction may become invalid if one of the unsafe inputs disappears. If that happens, the transaction must be funded with different inputs and republished. (#21359) +- We now support up to 20 keys in `multi()` and `sortedmulti()` descriptors + under `wsh()`. (#20867) + GUI changes ----------- diff --git a/src/addrdb.cpp b/src/addrdb.cpp index bf2f6c7614..b8fd019bab 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -11,13 +11,72 @@ #include <cstdint> #include <hash.h> #include <logging/timer.h> +#include <netbase.h> #include <random.h> #include <streams.h> #include <tinyformat.h> +#include <univalue.h> +#include <util/settings.h> #include <util/system.h> +CBanEntry::CBanEntry(const UniValue& json) + : nVersion(json["version"].get_int()), nCreateTime(json["ban_created"].get_int64()), + nBanUntil(json["banned_until"].get_int64()) +{ +} + +UniValue CBanEntry::ToJson() const +{ + UniValue json(UniValue::VOBJ); + json.pushKV("version", nVersion); + json.pushKV("ban_created", nCreateTime); + json.pushKV("banned_until", nBanUntil); + return json; +} + namespace { +static const char* BANMAN_JSON_ADDR_KEY = "address"; + +/** + * Convert a `banmap_t` object to a JSON array. + * @param[in] bans Bans list to convert. + * @return a JSON array, similar to the one returned by the `listbanned` RPC. Suitable for + * passing to `BanMapFromJson()`. + */ +UniValue BanMapToJson(const banmap_t& bans) +{ + UniValue bans_json(UniValue::VARR); + for (const auto& it : bans) { + const auto& address = it.first; + const auto& ban_entry = it.second; + UniValue j = ban_entry.ToJson(); + j.pushKV(BANMAN_JSON_ADDR_KEY, address.ToString()); + bans_json.push_back(j); + } + return bans_json; +} + +/** + * Convert a JSON array to a `banmap_t` object. + * @param[in] bans_json JSON to convert, must be as returned by `BanMapToJson()`. + * @param[out] bans Bans list to create from the JSON. + * @throws std::runtime_error if the JSON does not have the expected fields or they contain + * unparsable values. + */ +void BanMapFromJson(const UniValue& bans_json, banmap_t& bans) +{ + for (const auto& ban_entry_json : bans_json.getValues()) { + CSubNet subnet; + const auto& subnet_str = ban_entry_json[BANMAN_JSON_ADDR_KEY].get_str(); + if (!LookupSubNet(subnet_str, subnet)) { + throw std::runtime_error( + strprintf("Cannot parse banned address or subnet: %s", subnet_str)); + } + bans.insert_or_assign(subnet, CBanEntry{ban_entry_json}); + } +} + template <typename Stream, typename Data> bool SerializeDB(Stream& stream, const Data& data) { @@ -119,18 +178,54 @@ bool DeserializeFileDB(const fs::path& path, Data& data, int version) } } // namespace -CBanDB::CBanDB(fs::path ban_list_path) : m_ban_list_path(std::move(ban_list_path)) +CBanDB::CBanDB(fs::path ban_list_path) + : m_banlist_dat(ban_list_path.string() + ".dat"), + m_banlist_json(ban_list_path.string() + ".json") { } bool CBanDB::Write(const banmap_t& banSet) { - return SerializeFileDB("banlist", m_ban_list_path, banSet, CLIENT_VERSION); + std::vector<std::string> errors; + if (util::WriteSettings(m_banlist_json, {{JSON_KEY, BanMapToJson(banSet)}}, errors)) { + return true; + } + + for (const auto& err : errors) { + error("%s", err); + } + return false; } -bool CBanDB::Read(banmap_t& banSet) +bool CBanDB::Read(banmap_t& banSet, bool& dirty) { - return DeserializeFileDB(m_ban_list_path, banSet, CLIENT_VERSION); + // If the JSON banlist does not exist, then try to read the non-upgraded banlist.dat. + if (!fs::exists(m_banlist_json)) { + // If this succeeds then we need to flush to disk in order to create the JSON banlist. + dirty = true; + return DeserializeFileDB(m_banlist_dat, banSet, CLIENT_VERSION); + } + + dirty = false; + + std::map<std::string, util::SettingsValue> settings; + std::vector<std::string> errors; + + if (!util::ReadSettings(m_banlist_json, settings, errors)) { + for (const auto& err : errors) { + LogPrintf("Cannot load banlist %s: %s\n", m_banlist_json.string(), err); + } + return false; + } + + try { + BanMapFromJson(settings[JSON_KEY], banSet); + } catch (const std::runtime_error& e) { + LogPrintf("Cannot parse banlist %s: %s\n", m_banlist_json.string(), e.what()); + return false; + } + + return true; } CAddrDB::CAddrDB() diff --git a/src/addrdb.h b/src/addrdb.h index 8953ebb169..399103c991 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -9,6 +9,7 @@ #include <fs.h> #include <net_types.h> // For banmap_t #include <serialize.h> +#include <univalue.h> #include <string> #include <vector> @@ -36,6 +37,13 @@ public: nCreateTime = nCreateTimeIn; } + /** + * Create a ban entry from JSON. + * @param[in] json A JSON representation of a ban entry, as created by `ToJson()`. + * @throw std::runtime_error if the JSON does not have the expected fields. + */ + explicit CBanEntry(const UniValue& json); + SERIALIZE_METHODS(CBanEntry, obj) { uint8_t ban_reason = 2; //! For backward compatibility @@ -48,6 +56,12 @@ public: nCreateTime = 0; nBanUntil = 0; } + + /** + * Generate a JSON representation of this ban entry. + * @return JSON suitable for passing to the `CBanEntry(const UniValue&)` constructor. + */ + UniValue ToJson() const; }; /** Access to the (IP) address database (peers.dat) */ @@ -62,15 +76,30 @@ public: static bool Read(CAddrMan& addr, CDataStream& ssPeers); }; -/** Access to the banlist database (banlist.dat) */ +/** Access to the banlist databases (banlist.json and banlist.dat) */ class CBanDB { private: - const fs::path m_ban_list_path; + /** + * JSON key under which the data is stored in the json database. + */ + static constexpr const char* JSON_KEY = "banned_nets"; + + const fs::path m_banlist_dat; + const fs::path m_banlist_json; public: explicit CBanDB(fs::path ban_list_path); bool Write(const banmap_t& banSet); - bool Read(banmap_t& banSet); + + /** + * Read the banlist from disk. + * @param[out] banSet The loaded list. Set if `true` is returned, otherwise it is left + * in an undefined state. + * @param[out] dirty Indicates whether the loaded list needs flushing to disk. Set if + * `true` is returned, otherwise it is left in an undefined state. + * @return true on success + */ + bool Read(banmap_t& banSet, bool& dirty); }; /** diff --git a/src/banman.cpp b/src/banman.cpp index bb97fc4809..d2437e6733 100644 --- a/src/banman.cpp +++ b/src/banman.cpp @@ -18,20 +18,18 @@ BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist…").translated); int64_t n_start = GetTimeMillis(); - m_is_dirty = false; - banmap_t banmap; - if (m_ban_db.Read(banmap)) { - SetBanned(banmap); // thread save setter - SetBannedSetDirty(false); // no need to write down, just read data - SweepBanned(); // sweep out unused entries + if (m_ban_db.Read(m_banned, m_is_dirty)) { + SweepBanned(); // sweep out unused entries - LogPrint(BCLog::NET, "Loaded %d banned node ips/subnets from banlist.dat %dms\n", - m_banned.size(), GetTimeMillis() - n_start); + LogPrint(BCLog::NET, "Loaded %d banned node addresses/subnets %dms\n", m_banned.size(), + GetTimeMillis() - n_start); } else { - LogPrintf("Recreating banlist.dat\n"); - SetBannedSetDirty(true); // force write - DumpBanlist(); + LogPrintf("Recreating the banlist database\n"); + m_banned = {}; + m_is_dirty = true; } + + DumpBanlist(); } BanMan::~BanMan() @@ -53,8 +51,8 @@ void BanMan::DumpBanlist() SetBannedSetDirty(false); } - LogPrint(BCLog::NET, "Flushed %d banned node ips/subnets to banlist.dat %dms\n", - banmap.size(), GetTimeMillis() - n_start); + LogPrint(BCLog::NET, "Flushed %d banned node addresses/subnets to disk %dms\n", banmap.size(), + GetTimeMillis() - n_start); } void BanMan::ClearBanned() @@ -167,13 +165,6 @@ void BanMan::GetBanned(banmap_t& banmap) banmap = m_banned; //create a thread safe copy } -void BanMan::SetBanned(const banmap_t& banmap) -{ - LOCK(m_cs_banned); - m_banned = banmap; - m_is_dirty = true; -} - void BanMan::SweepBanned() { int64_t now = GetTime(); @@ -188,7 +179,7 @@ void BanMan::SweepBanned() m_banned.erase(it++); m_is_dirty = true; notify_ui = true; - LogPrint(BCLog::NET, "%s: Removed banned node ip/subnet from banlist.dat: %s\n", __func__, sub_net.ToString()); + LogPrint(BCLog::NET, "Removed banned node address/subnet: %s\n", sub_net.ToString()); } else ++it; } diff --git a/src/banman.h b/src/banman.h index f6bfbd1e49..8c75d4037e 100644 --- a/src/banman.h +++ b/src/banman.h @@ -17,7 +17,8 @@ // NOTE: When adjusting this, update rpcnet:setban's help ("24h") static constexpr unsigned int DEFAULT_MISBEHAVING_BANTIME = 60 * 60 * 24; // Default 24-hour ban -// How often to dump addresses to banlist.dat + +/// How often to dump banned addresses/subnets to disk. static constexpr std::chrono::minutes DUMP_BANS_INTERVAL{15}; class CClientUIInterface; @@ -30,7 +31,7 @@ class CSubNet; // If an address or subnet is banned, we never accept incoming connections from // it and never create outgoing connections to it. We won't gossip its address // to other peers in addr messages. Banned addresses and subnets are stored to -// banlist.dat on shutdown and reloaded on startup. Banning can be used to +// disk on shutdown and reloaded on startup. Banning can be used to // prevent connections with spy nodes or other griefers. // // 2. Discouragement. If a peer misbehaves enough (see Misbehaving() in @@ -79,7 +80,6 @@ public: void DumpBanlist(); private: - void SetBanned(const banmap_t& banmap); bool BannedSetIsDirty(); //!set the "dirty" flag for the banlist void SetBannedSetDirty(bool dirty = true); diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index c5840b2098..adc485983f 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -40,6 +40,7 @@ UrlDecodeFn* const URL_DECODE = urlDecode; static const char DEFAULT_RPCCONNECT[] = "127.0.0.1"; static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900; +static constexpr int DEFAULT_WAIT_CLIENT_TIMEOUT = 0; static const bool DEFAULT_NAMED=false; static const int CONTINUE_EXECUTION=-1; static constexpr int8_t UNKNOWN_NETWORK{-1}; @@ -73,6 +74,7 @@ static void SetupCliArgs(ArgsManager& argsman) argsman.AddArg("-rpcport=<port>", strprintf("Connect to JSON-RPC on <port> (default: %u, testnet: %u, signet: %u, regtest: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort(), signetBaseParams->RPCPort(), regtestBaseParams->RPCPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS); argsman.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-rpcwait", "Wait for RPC server to start", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-rpcwaittimeout=<n>", strprintf("Timeout in seconds to wait for the RPC server to start, or 0 for no timeout. (default: %d)", DEFAULT_WAIT_CLIENT_TIMEOUT), ArgsManager::ALLOW_INT, OptionsCategory::OPTIONS); argsman.AddArg("-rpcwallet=<walletname>", "Send RPC for non-default wallet on RPC server (needs to exactly match corresponding -wallet option passed to bitcoind). This changes the RPC endpoint used, e.g. http://127.0.0.1:8332/wallet/<walletname>", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-stdin", "Read extra arguments from standard input, one per line until EOF/Ctrl-D (recommended for sensitive information such as passphrases). When combined with -stdinrpcpass, the first line from standard input is used for the RPC password.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-stdinrpcpass", "Read RPC password from standard input as a single line. When combined with -stdin, the first line from standard input is used for the RPC password. When combined with -stdinwalletpassphrase, -stdinrpcpass consumes the first line, and -stdinwalletpassphrase consumes the second.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); @@ -794,6 +796,9 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str UniValue response(UniValue::VOBJ); // Execute and handle connection failures with -rpcwait. const bool fWait = gArgs.GetBoolArg("-rpcwait", false); + const int timeout = gArgs.GetArg("-rpcwaittimeout", DEFAULT_WAIT_CLIENT_TIMEOUT); + const int64_t deadline = GetTime<std::chrono::seconds>().count() + timeout; + do { try { response = CallRPC(rh, strMethod, args, rpcwallet); @@ -804,11 +809,12 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str } } break; // Connection succeeded, no need to retry. - } catch (const CConnectionFailed&) { - if (fWait) { - UninterruptibleSleep(std::chrono::milliseconds{1000}); + } catch (const CConnectionFailed& e) { + const int64_t now = GetTime<std::chrono::seconds>().count(); + if (fWait && (timeout <= 0 || now < deadline)) { + UninterruptibleSleep(std::chrono::seconds{1}); } else { - throw; + throw CConnectionFailed(strprintf("timeout on transient error: %s", e.what())); } } } while (fWait); diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 45c049c3be..b3984a43bb 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -83,7 +83,7 @@ public: bool Enqueue(WorkItem* item) { LOCK(cs); - if (queue.size() >= maxDepth) { + if (!running || queue.size() >= maxDepth) { return false; } queue.emplace_back(std::unique_ptr<WorkItem>(item)); @@ -99,7 +99,7 @@ public: WAIT_LOCK(cs, lock); while (running && queue.empty()) cond.wait(lock); - if (!running) + if (!running && queue.empty()) break; i = std::move(queue.front()); queue.pop_front(); @@ -448,8 +448,6 @@ void StopHTTPServer() thread.join(); } g_thread_http_workers.clear(); - delete workQueue; - workQueue = nullptr; } // Unlisten sockets, these are what make the event loop running, which means // that after this and all connections are closed the event loop will quit. @@ -469,6 +467,10 @@ void StopHTTPServer() event_base_free(eventBase); eventBase = nullptr; } + if (workQueue) { + delete workQueue; + workQueue = nullptr; + } LogPrint(BCLog::HTTP, "Stopped HTTP server\n"); } diff --git a/src/init.cpp b/src/init.cpp index 4dc82811f9..da0447ca79 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1161,7 +1161,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.addrman); node.addrman = std::make_unique<CAddrMan>(); assert(!node.banman); - node.banman = std::make_unique<BanMan>(gArgs.GetDataDirNet() / "banlist.dat", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME)); + node.banman = std::make_unique<BanMan>(gArgs.GetDataDirNet() / "banlist", &uiInterface, args.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME)); assert(!node.connman); node.connman = std::make_unique<CConnman>(GetRand(std::numeric_limits<uint64_t>::max()), GetRand(std::numeric_limits<uint64_t>::max()), *node.addrman, args.GetBoolArg("-networkactive", true)); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b2112cfd2e..9bb3dfdcf3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -836,12 +836,27 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) return; } if (nodestate->fProvidesHeaderAndIDs) { + int num_outbound_hb_peers = 0; for (std::list<NodeId>::iterator it = lNodesAnnouncingHeaderAndIDs.begin(); it != lNodesAnnouncingHeaderAndIDs.end(); it++) { if (*it == nodeid) { lNodesAnnouncingHeaderAndIDs.erase(it); lNodesAnnouncingHeaderAndIDs.push_back(nodeid); return; } + CNodeState *state = State(*it); + if (state != nullptr && !state->m_is_inbound) ++num_outbound_hb_peers; + } + if (nodestate->m_is_inbound) { + // If we're adding an inbound HB peer, make sure we're not removing + // our last outbound HB peer in the process. + if (lNodesAnnouncingHeaderAndIDs.size() >= 3 && num_outbound_hb_peers == 1) { + CNodeState *remove_node = State(lNodesAnnouncingHeaderAndIDs.front()); + if (remove_node != nullptr && !remove_node->m_is_inbound) { + // Put the HB outbound peer in the second slot, so that it + // doesn't get removed. + std::swap(lNodesAnnouncingHeaderAndIDs.front(), *std::next(lNodesAnnouncingHeaderAndIDs.begin())); + } + } } m_connman.ForNode(nodeid, [this](CNode* pfrom) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index faf99dea81..7c7bf68178 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -5,12 +5,13 @@ #ifndef BITCOIN_NODE_BLOCKSTORAGE_H #define BITCOIN_NODE_BLOCKSTORAGE_H -#include <cstdint> -#include <vector> - #include <fs.h> #include <protocol.h> // For CMessageHeader::MessageStartChars +#include <atomic> +#include <cstdint> +#include <vector> + class ArgsManager; class BlockValidationState; class CBlock; diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 57178d015d..5668ead1fb 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -191,7 +191,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_AUTO_TEST_CASE(peer_discouragement) { const CChainParams& chainparams = Params(); - auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); + auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, false); @@ -285,7 +285,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) BOOST_AUTO_TEST_CASE(DoS_bantime) { const CChainParams& chainparams = Params(); - auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); + auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman); auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, false); diff --git a/src/test/fuzz/banman.cpp b/src/test/fuzz/banman.cpp index 759a70a857..cca41e79ae 100644 --- a/src/test/fuzz/banman.cpp +++ b/src/test/fuzz/banman.cpp @@ -9,8 +9,10 @@ #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> #include <test/util/setup_common.h> +#include <util/readwritefile.h> #include <util/system.h> +#include <cassert> #include <cstdint> #include <limits> #include <string> @@ -38,8 +40,20 @@ FUZZ_TARGET_INIT(banman, initialize_banman) int limit_max_ops{300}; FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; SetMockTime(ConsumeTime(fuzzed_data_provider)); - const fs::path banlist_file = gArgs.GetDataDirNet() / "fuzzed_banlist.dat"; - fs::remove(banlist_file); + fs::path banlist_file = gArgs.GetDataDirNet() / "fuzzed_banlist"; + + const bool start_with_corrupted_banlist{fuzzed_data_provider.ConsumeBool()}; + if (start_with_corrupted_banlist) { + const std::string sfx{fuzzed_data_provider.ConsumeBool() ? ".dat" : ".json"}; + assert(WriteBinaryFile(banlist_file.string() + sfx, + fuzzed_data_provider.ConsumeRandomLengthString())); + } else { + const bool force_read_and_write_to_err{fuzzed_data_provider.ConsumeBool()}; + if (force_read_and_write_to_err) { + banlist_file = fs::path{"path"} / "to" / "inaccessible" / "fuzzed_banlist"; + } + } + { BanMan ban_man{banlist_file, nullptr, ConsumeBanTimeOffset(fuzzed_data_provider)}; while (--limit_max_ops >= 0 && fuzzed_data_provider.ConsumeBool()) { @@ -80,5 +94,6 @@ FUZZ_TARGET_INIT(banman, initialize_banman) }); } } - fs::remove(banlist_file); + fs::remove(banlist_file.string() + ".dat"); + fs::remove(banlist_file.string() + ".json"); } diff --git a/src/test/fuzz/base_encode_decode.cpp b/src/test/fuzz/base_encode_decode.cpp index 4470e13a61..2b4f15115b 100644 --- a/src/test/fuzz/base_encode_decode.cpp +++ b/src/test/fuzz/base_encode_decode.cpp @@ -14,7 +14,12 @@ #include <string> #include <vector> -FUZZ_TARGET(base_encode_decode) +void initialize_base_encode_decode() +{ + static const ECCVerifyHandle verify_handle; +} + +FUZZ_TARGET_INIT(base_encode_decode, initialize_base_encode_decode) { const std::string random_encoded_string(buffer.begin(), buffer.end()); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index e105e85e47..f71d9148b6 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -196,7 +196,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const } m_node.addrman = std::make_unique<CAddrMan>(); - m_node.banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); + m_node.banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); m_node.connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman); // Deterministic randomness for tests. m_node.peerman = PeerManager::make(chainparams, *m_node.connman, *m_node.addrman, m_node.banman.get(), *m_node.scheduler, *m_node.chainman, diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 94915a1373..e03835eaff 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4322,6 +4322,11 @@ static RPCHelpMan walletprocesspsbt() std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return NullUniValue; + const CWallet& wallet{*pwallet}; + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + wallet.BlockUntilSyncedToCurrentChain(); + RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VBOOL, UniValue::VSTR}); // Unserialize the transaction @@ -4338,7 +4343,7 @@ static RPCHelpMan walletprocesspsbt() bool sign = request.params[1].isNull() ? true : request.params[1].get_bool(); bool bip32derivs = request.params[3].isNull() ? true : request.params[3].get_bool(); bool complete = true; - const TransactionError err = pwallet->FillPSBT(psbtx, complete, nHashType, sign, bip32derivs); + const TransactionError err{wallet.FillPSBT(psbtx, complete, nHashType, sign, bip32derivs)}; if (err != TransactionError::OK) { throw JSONRPCTransactionError(err); } @@ -4436,6 +4441,11 @@ static RPCHelpMan walletcreatefundedpsbt() std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return NullUniValue; + CWallet& wallet{*pwallet}; + // Make sure the results are valid at least up to the most recent block + // the user could have gotten from another RPC command prior to now + wallet.BlockUntilSyncedToCurrentChain(); + RPCTypeCheck(request.params, { UniValue::VARR, UniValueType(), // ARR or OBJ, checked later @@ -4447,7 +4457,7 @@ static RPCHelpMan walletcreatefundedpsbt() CAmount fee; int change_position; - bool rbf = pwallet->m_signal_rbf; + bool rbf{wallet.m_signal_rbf}; const UniValue &replaceable_arg = request.params[3]["replaceable"]; if (!replaceable_arg.isNull()) { RPCTypeCheckArgument(replaceable_arg, UniValue::VBOOL); @@ -4458,7 +4468,7 @@ static RPCHelpMan walletcreatefundedpsbt() // Automatically select coins, unless at least one is manually selected. Can // be overridden by options.add_inputs. coin_control.m_add_inputs = rawTx.vin.size() == 0; - FundTransaction(*pwallet, rawTx, fee, change_position, request.params[3], coin_control, /* override_min_fee */ true); + FundTransaction(wallet, rawTx, fee, change_position, request.params[3], coin_control, /* override_min_fee */ true); // Make a blank psbt PartiallySignedTransaction psbtx(rawTx); @@ -4466,7 +4476,7 @@ static RPCHelpMan walletcreatefundedpsbt() // Fill transaction with out data but don't sign bool bip32derivs = request.params[4].isNull() ? true : request.params[4].get_bool(); bool complete = true; - const TransactionError err = pwallet->FillPSBT(psbtx, complete, 1, false, bip32derivs); + const TransactionError err{wallet.FillPSBT(psbtx, complete, 1, false, bip32derivs)}; if (err != TransactionError::OK) { throw JSONRPCTransactionError(err); } diff --git a/test/functional/feature_coinstatsindex.py b/test/functional/feature_coinstatsindex.py index cf270b25ee..7488e3cb42 100755 --- a/test/functional/feature_coinstatsindex.py +++ b/test/functional/feature_coinstatsindex.py @@ -281,6 +281,7 @@ class CoinStatsIndexTest(BitcoinTestFramework): # Add another block, so we don't depend on reconsiderblock remembering which # blocks were touched by invalidateblock index_node.generate(1) + self.sync_all() # Ensure that removing and re-adding blocks yields consistent results block = index_node.getblockhash(99) diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py index 0bb04ae267..b99b64e16d 100755 --- a/test/functional/feature_rbf.py +++ b/test/functional/feature_rbf.py @@ -7,17 +7,20 @@ from decimal import Decimal from test_framework.blocktools import COINBASE_MATURITY -from test_framework.messages import COIN, COutPoint, CTransaction, CTxIn, CTxOut +from test_framework.messages import COIN, COutPoint, CTransaction, CTxIn, CTxOut, BIP125_SEQUENCE_NUMBER from test_framework.script import CScript, OP_DROP from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_raises_rpc_error, satoshi_round from test_framework.script_util import DUMMY_P2WPKH_SCRIPT, DUMMY_2_P2WPKH_SCRIPT +from test_framework.wallet import MiniWallet MAX_REPLACEMENT_LIMIT = 100 + def txToHex(tx): return tx.serialize().hex() + def make_utxo(node, amount, confirmed=True, scriptPubKey=DUMMY_P2WPKH_SCRIPT): """Create a txout with a given amount and scriptPubKey @@ -26,12 +29,12 @@ def make_utxo(node, amount, confirmed=True, scriptPubKey=DUMMY_P2WPKH_SCRIPT): confirmed - txouts created will be confirmed in the blockchain; unconfirmed otherwise. """ - fee = 1*COIN - while node.getbalance() < satoshi_round((amount + fee)/COIN): + fee = 1 * COIN + while node.getbalance() < satoshi_round((amount + fee) / COIN): node.generate(COINBASE_MATURITY) new_addr = node.getnewaddress() - txid = node.sendtoaddress(new_addr, satoshi_round((amount+fee)/COIN)) + txid = node.sendtoaddress(new_addr, satoshi_round((amount + fee) / COIN)) tx1 = node.getrawtransaction(txid, 1) txid = int(txid, 16) i, _ = next(filter(lambda vout: new_addr == vout[1]['scriptPubKey']['address'], enumerate(tx1['vout']))) @@ -78,10 +81,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): self.skip_if_no_wallet() def run_test(self): - # Leave IBD - self.nodes[0].generate(1) - - make_utxo(self.nodes[0], 1*COIN) + make_utxo(self.nodes[0], 1 * COIN) # Ensure nodes are synced self.sync_all() @@ -123,7 +123,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): def test_simple_doublespend(self): """Simple doublespend""" - tx0_outpoint = make_utxo(self.nodes[0], int(1.1*COIN)) + tx0_outpoint = make_utxo(self.nodes[0], int(1.1 * COIN)) # make_utxo may have generated a bunch of blocks, so we need to sync # before we can spend the coins generated, or else the resulting @@ -165,14 +165,14 @@ class ReplaceByFeeTest(BitcoinTestFramework): def test_doublespend_chain(self): """Doublespend of a long chain""" - initial_nValue = 50*COIN + initial_nValue = 50 * COIN tx0_outpoint = make_utxo(self.nodes[0], initial_nValue) prevout = tx0_outpoint remaining_value = initial_nValue chain_txids = [] - while remaining_value > 10*COIN: - remaining_value -= 1*COIN + while remaining_value > 10 * COIN: + remaining_value -= 1 * COIN tx = CTransaction() tx.vin = [CTxIn(prevout, nSequence=0)] tx.vout = [CTxOut(remaining_value, CScript([1, OP_DROP] * 15 + [1]))] @@ -205,10 +205,10 @@ class ReplaceByFeeTest(BitcoinTestFramework): def test_doublespend_tree(self): """Doublespend of a big tree of transactions""" - initial_nValue = 50*COIN + initial_nValue = 50 * COIN tx0_outpoint = make_utxo(self.nodes[0], initial_nValue) - def branch(prevout, initial_value, max_txs, tree_width=5, fee=0.0001*COIN, _total_txs=None): + def branch(prevout, initial_value, max_txs, tree_width=5, fee=0.0001 * COIN, _total_txs=None): if _total_txs is None: _total_txs = [0] if _total_txs[0] >= max_txs: @@ -239,7 +239,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): _total_txs=_total_txs): yield x - fee = int(0.0001*COIN) + fee = int(0.0001 * COIN) n = MAX_REPLACEMENT_LIMIT tree_txs = list(branch(tx0_outpoint, initial_nValue, n, fee=fee)) assert_equal(len(tree_txs), n) @@ -267,8 +267,8 @@ class ReplaceByFeeTest(BitcoinTestFramework): # Try again, but with more total transactions than the "max txs # double-spent at once" anti-DoS limit. - for n in (MAX_REPLACEMENT_LIMIT+1, MAX_REPLACEMENT_LIMIT*2): - fee = int(0.0001*COIN) + for n in (MAX_REPLACEMENT_LIMIT + 1, MAX_REPLACEMENT_LIMIT * 2): + fee = int(0.0001 * COIN) tx0_outpoint = make_utxo(self.nodes[0], initial_nValue) tree_txs = list(branch(tx0_outpoint, initial_nValue, n, fee=fee)) assert_equal(len(tree_txs), n) @@ -286,7 +286,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): def test_replacement_feeperkb(self): """Replacement requires fee-per-KB to be higher""" - tx0_outpoint = make_utxo(self.nodes[0], int(1.1*COIN)) + tx0_outpoint = make_utxo(self.nodes[0], int(1.1 * COIN)) tx1a = CTransaction() tx1a.vin = [CTxIn(tx0_outpoint, nSequence=0)] @@ -298,7 +298,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): # rejected. tx1b = CTransaction() tx1b.vin = [CTxIn(tx0_outpoint, nSequence=0)] - tx1b.vout = [CTxOut(int(0.001*COIN), CScript([b'a'*999000]))] + tx1b.vout = [CTxOut(int(0.001 * COIN), CScript([b'a' * 999000]))] tx1b_hex = txToHex(tx1b) # This will raise an exception due to insufficient fee @@ -306,8 +306,8 @@ class ReplaceByFeeTest(BitcoinTestFramework): def test_spends_of_conflicting_outputs(self): """Replacements that spend conflicting tx outputs are rejected""" - utxo1 = make_utxo(self.nodes[0], int(1.2*COIN)) - utxo2 = make_utxo(self.nodes[0], 3*COIN) + utxo1 = make_utxo(self.nodes[0], int(1.2 * COIN)) + utxo2 = make_utxo(self.nodes[0], 3 * COIN) tx1a = CTransaction() tx1a.vin = [CTxIn(utxo1, nSequence=0)] @@ -346,8 +346,8 @@ class ReplaceByFeeTest(BitcoinTestFramework): def test_new_unconfirmed_inputs(self): """Replacements that add new unconfirmed inputs are rejected""" - confirmed_utxo = make_utxo(self.nodes[0], int(1.1*COIN)) - unconfirmed_utxo = make_utxo(self.nodes[0], int(0.1*COIN), False) + confirmed_utxo = make_utxo(self.nodes[0], int(1.1 * COIN)) + unconfirmed_utxo = make_utxo(self.nodes[0], int(0.1 * COIN), False) tx1 = CTransaction() tx1.vin = [CTxIn(confirmed_utxo)] @@ -369,13 +369,13 @@ class ReplaceByFeeTest(BitcoinTestFramework): # transactions # Start by creating a single transaction with many outputs - initial_nValue = 10*COIN + initial_nValue = 10 * COIN utxo = make_utxo(self.nodes[0], initial_nValue) - fee = int(0.0001*COIN) - split_value = int((initial_nValue-fee)/(MAX_REPLACEMENT_LIMIT+1)) + fee = int(0.0001 * COIN) + split_value = int((initial_nValue - fee) / (MAX_REPLACEMENT_LIMIT + 1)) outputs = [] - for _ in range(MAX_REPLACEMENT_LIMIT+1): + for _ in range(MAX_REPLACEMENT_LIMIT + 1): outputs.append(CTxOut(split_value, CScript([1]))) splitting_tx = CTransaction() @@ -387,7 +387,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): txid = int(txid, 16) # Now spend each of those outputs individually - for i in range(MAX_REPLACEMENT_LIMIT+1): + for i in range(MAX_REPLACEMENT_LIMIT + 1): tx_i = CTransaction() tx_i.vin = [CTxIn(COutPoint(txid, i), nSequence=0)] tx_i.vout = [CTxOut(split_value - fee, DUMMY_P2WPKH_SCRIPT)] @@ -397,9 +397,9 @@ class ReplaceByFeeTest(BitcoinTestFramework): # Now create doublespend of the whole lot; should fail. # Need a big enough fee to cover all spending transactions and have # a higher fee rate - double_spend_value = (split_value-100*fee)*(MAX_REPLACEMENT_LIMIT+1) + double_spend_value = (split_value - 100 * fee) * (MAX_REPLACEMENT_LIMIT + 1) inputs = [] - for i in range(MAX_REPLACEMENT_LIMIT+1): + for i in range(MAX_REPLACEMENT_LIMIT + 1): inputs.append(CTxIn(COutPoint(txid, i), nSequence=0)) double_tx = CTransaction() double_tx.vin = inputs @@ -418,7 +418,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): def test_opt_in(self): """Replacing should only work if orig tx opted in""" - tx0_outpoint = make_utxo(self.nodes[0], int(1.1*COIN)) + tx0_outpoint = make_utxo(self.nodes[0], int(1.1 * COIN)) # Create a non-opting in transaction tx1a = CTransaction() @@ -439,7 +439,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): # This will raise an exception assert_raises_rpc_error(-26, "txn-mempool-conflict", self.nodes[0].sendrawtransaction, tx1b_hex, 0) - tx1_outpoint = make_utxo(self.nodes[0], int(1.1*COIN)) + tx1_outpoint = make_utxo(self.nodes[0], int(1.1 * COIN)) # Create a different non-opting in transaction tx2a = CTransaction() @@ -467,7 +467,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): tx3a = CTransaction() tx3a.vin = [CTxIn(COutPoint(tx1a_txid, 0), nSequence=0xffffffff), CTxIn(COutPoint(tx2a_txid, 0), nSequence=0xfffffffd)] - tx3a.vout = [CTxOut(int(0.9*COIN), CScript([b'c'])), CTxOut(int(0.9*COIN), CScript([b'd']))] + tx3a.vout = [CTxOut(int(0.9 * COIN), CScript([b'c'])), CTxOut(int(0.9 * COIN), CScript([b'd']))] tx3a_hex = txToHex(tx3a) tx3a_txid = self.nodes[0].sendrawtransaction(tx3a_hex, 0) @@ -495,7 +495,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): # correctly used by replacement logic # 1. Check that feeperkb uses modified fees - tx0_outpoint = make_utxo(self.nodes[0], int(1.1*COIN)) + tx0_outpoint = make_utxo(self.nodes[0], int(1.1 * COIN)) tx1a = CTransaction() tx1a.vin = [CTxIn(tx0_outpoint, nSequence=0)] @@ -506,14 +506,14 @@ class ReplaceByFeeTest(BitcoinTestFramework): # Higher fee, but the actual fee per KB is much lower. tx1b = CTransaction() tx1b.vin = [CTxIn(tx0_outpoint, nSequence=0)] - tx1b.vout = [CTxOut(int(0.001*COIN), CScript([b'a'*740000]))] + tx1b.vout = [CTxOut(int(0.001 * COIN), CScript([b'a' * 740000]))] tx1b_hex = txToHex(tx1b) # Verify tx1b cannot replace tx1a. assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx1b_hex, 0) # Use prioritisetransaction to set tx1a's fee to 0. - self.nodes[0].prioritisetransaction(txid=tx1a_txid, fee_delta=int(-0.1*COIN)) + self.nodes[0].prioritisetransaction(txid=tx1a_txid, fee_delta=int(-0.1 * COIN)) # Now tx1b should be able to replace tx1a tx1b_txid = self.nodes[0].sendrawtransaction(tx1b_hex, 0) @@ -521,7 +521,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): assert tx1b_txid in self.nodes[0].getrawmempool() # 2. Check that absolute fee checks use modified fee. - tx1_outpoint = make_utxo(self.nodes[0], int(1.1*COIN)) + tx1_outpoint = make_utxo(self.nodes[0], int(1.1 * COIN)) tx2a = CTransaction() tx2a.vin = [CTxIn(tx1_outpoint, nSequence=0)] @@ -540,7 +540,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx2b_hex, 0) # Now prioritise tx2b to have a higher modified fee - self.nodes[0].prioritisetransaction(txid=tx2b.hash, fee_delta=int(0.1*COIN)) + self.nodes[0].prioritisetransaction(txid=tx2b.hash, fee_delta=int(0.1 * COIN)) # tx2b should now be accepted tx2b_txid = self.nodes[0].sendrawtransaction(tx2b_hex, 0) @@ -550,11 +550,11 @@ class ReplaceByFeeTest(BitcoinTestFramework): def test_rpc(self): us0 = self.nodes[0].listunspent()[0] ins = [us0] - outs = {self.nodes[0].getnewaddress() : Decimal(1.0000000)} + outs = {self.nodes[0].getnewaddress(): Decimal(1.0000000)} rawtx0 = self.nodes[0].createrawtransaction(ins, outs, 0, True) rawtx1 = self.nodes[0].createrawtransaction(ins, outs, 0, False) - json0 = self.nodes[0].decoderawtransaction(rawtx0) - json1 = self.nodes[0].decoderawtransaction(rawtx1) + json0 = self.nodes[0].decoderawtransaction(rawtx0) + json1 = self.nodes[0].decoderawtransaction(rawtx1) assert_equal(json0["vin"][0]["sequence"], 4294967293) assert_equal(json1["vin"][0]["sequence"], 4294967295) @@ -562,74 +562,68 @@ class ReplaceByFeeTest(BitcoinTestFramework): frawtx2a = self.nodes[0].fundrawtransaction(rawtx2, {"replaceable": True}) frawtx2b = self.nodes[0].fundrawtransaction(rawtx2, {"replaceable": False}) - json0 = self.nodes[0].decoderawtransaction(frawtx2a['hex']) - json1 = self.nodes[0].decoderawtransaction(frawtx2b['hex']) + json0 = self.nodes[0].decoderawtransaction(frawtx2a['hex']) + json1 = self.nodes[0].decoderawtransaction(frawtx2b['hex']) assert_equal(json0["vin"][0]["sequence"], 4294967293) assert_equal(json1["vin"][0]["sequence"], 4294967294) def test_no_inherited_signaling(self): - # Send tx from which to conflict outputs later - base_txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10")) - self.nodes[0].generate(1) - self.sync_blocks() + wallet = MiniWallet(self.nodes[0]) + wallet.scan_blocks(start=76, num=1) + confirmed_utxo = wallet.get_utxo() # Create an explicitly opt-in parent transaction - optin_parent_tx = self.nodes[0].createrawtransaction([{ - 'txid': base_txid, - 'vout': 0, - "sequence": 0xfffffffd, - }], {self.nodes[0].getnewaddress(): Decimal("9.99998")}) - - optin_parent_tx = self.nodes[0].signrawtransactionwithwallet(optin_parent_tx) - - # Broadcast parent tx - optin_parent_txid = self.nodes[0].sendrawtransaction(hexstring=optin_parent_tx["hex"], maxfeerate=0) - assert optin_parent_txid in self.nodes[0].getrawmempool() - - replacement_parent_tx = self.nodes[0].createrawtransaction([{ - 'txid': base_txid, - 'vout': 0, - "sequence": 0xfffffffd, - }], {self.nodes[0].getnewaddress(): Decimal("9.90000")}) - - replacement_parent_tx = self.nodes[0].signrawtransactionwithwallet(replacement_parent_tx) + optin_parent_tx = wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=confirmed_utxo, + sequence=BIP125_SEQUENCE_NUMBER, + fee_rate=Decimal('0.01'), + ) + assert_equal(True, self.nodes[0].getmempoolentry(optin_parent_tx['txid'])['bip125-replaceable']) + + replacement_parent_tx = wallet.create_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=confirmed_utxo, + sequence=BIP125_SEQUENCE_NUMBER, + fee_rate=Decimal('0.02'), + ) # Test if parent tx can be replaced. - res = self.nodes[0].testmempoolaccept(rawtxs=[replacement_parent_tx['hex']], maxfeerate=0)[0] + res = self.nodes[0].testmempoolaccept(rawtxs=[replacement_parent_tx['hex']])[0] # Parent can be replaced. assert_equal(res['allowed'], True) # Create an opt-out child tx spending the opt-in parent - optout_child_tx = self.nodes[0].createrawtransaction([{ - 'txid': optin_parent_txid, - 'vout': 0, - "sequence": 0xffffffff, - }], {self.nodes[0].getnewaddress(): Decimal("9.99990")}) - - optout_child_tx = self.nodes[0].signrawtransactionwithwallet(optout_child_tx) - - # Broadcast child tx - optout_child_txid = self.nodes[0].sendrawtransaction(hexstring=optout_child_tx["hex"], maxfeerate=0) - assert optout_child_txid in self.nodes[0].getrawmempool() - - replacement_child_tx = self.nodes[0].createrawtransaction([{ - 'txid': optin_parent_txid, - 'vout': 0, - "sequence": 0xffffffff, - }], {self.nodes[0].getnewaddress(): Decimal("9.00000")}) - - replacement_child_tx = self.nodes[0].signrawtransactionwithwallet(replacement_child_tx) + parent_utxo = wallet.get_utxo(txid=optin_parent_tx['txid']) + optout_child_tx = wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=parent_utxo, + sequence=0xffffffff, + fee_rate=Decimal('0.01'), + ) + + # Reports true due to inheritance + assert_equal(True, self.nodes[0].getmempoolentry(optout_child_tx['txid'])['bip125-replaceable']) + + replacement_child_tx = wallet.create_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=parent_utxo, + sequence=0xffffffff, + fee_rate=Decimal('0.02'), + mempool_valid=False, + ) # Broadcast replacement child tx # BIP 125 : # 1. The original transactions signal replaceability explicitly or through inheritance as described in the above # Summary section. - # The original transaction (`optout_child_tx`) doesn't signal RBF but its parent (`optin_parent_txid`) does. + # The original transaction (`optout_child_tx`) doesn't signal RBF but its parent (`optin_parent_tx`) does. # The replacement transaction (`replacement_child_tx`) should be able to replace the original transaction. # See CVE-2021-31876 for further explanations. - assert optin_parent_txid in self.nodes[0].getrawmempool() + assert_equal(True, self.nodes[0].getmempoolentry(optin_parent_tx['txid'])['bip125-replaceable']) assert_raises_rpc_error(-26, 'txn-mempool-conflict', self.nodes[0].sendrawtransaction, replacement_child_tx["hex"], 0) + if __name__ == '__main__': ReplaceByFeeTest().main() diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index 30cd499b3f..22eec59600 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -10,10 +10,12 @@ from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than_or_equal, assert_raises_process_error, assert_raises_rpc_error, get_auth_cookie, ) +import time # The block reward of coinbaseoutput.nValue (50) BTC/block matures after # COINBASE_MATURITY (100) blocks. Therefore, after mining 101 blocks we expect @@ -248,6 +250,12 @@ class TestBitcoinCli(BitcoinTestFramework): self.nodes[0].wait_for_rpc_connection() assert_equal(blocks, BLOCKS + 25) + self.log.info("Test -rpcwait option waits at most -rpcwaittimeout seconds for startup") + self.stop_node(0) # stop the node so we time out + start_time = time.time() + assert_raises_process_error(1, "Could not connect to the server", self.nodes[0].cli('-rpcwait', '-rpcwaittimeout=5').echo) + assert_greater_than_or_equal(time.time(), start_time + 5) + if __name__ == '__main__': TestBitcoinCli().main() diff --git a/test/functional/p2p_compactblocks_hb.py b/test/functional/p2p_compactblocks_hb.py new file mode 100755 index 0000000000..a3d30a6f04 --- /dev/null +++ b/test/functional/p2p_compactblocks_hb.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test compact blocks HB selection logic.""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + + +class CompactBlocksConnectionTest(BitcoinTestFramework): + """Test class for verifying selection of HB peer connections.""" + + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 6 + + def peer_info(self, from_node, to_node): + """Query from_node for its getpeerinfo about to_node.""" + for peerinfo in self.nodes[from_node].getpeerinfo(): + if "(testnode%i)" % to_node in peerinfo['subver']: + return peerinfo + return None + + def setup_network(self): + self.setup_nodes() + # Start network with everyone disconnected + self.sync_all() + + def relay_block_through(self, peer): + """Relay a new block through peer peer, and return HB status between 1 and [2,3,4,5].""" + self.connect_nodes(peer, 0) + self.nodes[0].generate(1) + self.sync_blocks() + self.disconnect_nodes(peer, 0) + status_to = [self.peer_info(1, i)['bip152_hb_to'] for i in range(2, 6)] + status_from = [self.peer_info(i, 1)['bip152_hb_from'] for i in range(2, 6)] + assert_equal(status_to, status_from) + return status_to + + def run_test(self): + self.log.info("Testing reserved high-bandwidth mode slot for outbound peer...") + + # Connect everyone to node 0, and mine some blocks to get all nodes out of IBD. + for i in range(1, 6): + self.connect_nodes(i, 0) + self.nodes[0].generate(2) + self.sync_blocks() + for i in range(1, 6): + self.disconnect_nodes(i, 0) + + # Construct network topology: + # - Node 0 is the block producer + # - Node 1 is the "target" node being tested + # - Nodes 2-5 are intermediaries. + # - Node 1 has an outbound connection to node 2 + # - Node 1 has inbound connections from nodes 3-5 + self.connect_nodes(3, 1) + self.connect_nodes(4, 1) + self.connect_nodes(5, 1) + self.connect_nodes(1, 2) + + # Mine blocks subsequently relaying through nodes 3,4,5 (inbound to node 1) + for nodeid in range(3, 6): + status = self.relay_block_through(nodeid) + assert_equal(status, [False, nodeid >= 3, nodeid >= 4, nodeid >= 5]) + + # And again through each. This should not change HB status. + for nodeid in range(3, 6): + status = self.relay_block_through(nodeid) + assert_equal(status, [False, True, True, True]) + + # Now relay one block through peer 2 (outbound from node 1), so it should take HB status + # from one of the inbounds. + status = self.relay_block_through(2) + assert_equal(status[0], True) + assert_equal(sum(status), 3) + + # Now relay again through nodes 3,4,5. Since 2 is outbound, it should remain HB. + for nodeid in range(3, 6): + status = self.relay_block_through(nodeid) + assert status[0] + assert status[nodeid - 2] + assert_equal(sum(status), 3) + + # Reconnect peer 2, and retry. Now the three inbounds should be HB again. + self.disconnect_nodes(1, 2) + self.connect_nodes(1, 2) + for nodeid in range(3, 6): + status = self.relay_block_through(nodeid) + assert not status[0] + assert status[nodeid - 2] + assert_equal(status, [False, True, True, True]) + + +if __name__ == '__main__': + CompactBlocksConnectionTest().main() diff --git a/test/functional/rpc_setban.py b/test/functional/rpc_setban.py index fd5f8aa098..36873f964b 100755 --- a/test/functional/rpc_setban.py +++ b/test/functional/rpc_setban.py @@ -22,7 +22,7 @@ class SetBanTests(BitcoinTestFramework): # Node 0 connects to Node 1, check that the noban permission is not granted self.connect_nodes(0, 1) peerinfo = self.nodes[1].getpeerinfo()[0] - assert(not 'noban' in peerinfo['permissions']) + assert not "noban" in peerinfo["permissions"] # Node 0 get banned by Node 1 self.nodes[1].setban("127.0.0.1", "add") @@ -36,27 +36,40 @@ class SetBanTests(BitcoinTestFramework): self.restart_node(1, ['-whitelist=127.0.0.1']) self.connect_nodes(0, 1) peerinfo = self.nodes[1].getpeerinfo()[0] - assert('noban' in peerinfo['permissions']) + assert "noban" in peerinfo["permissions"] # If we remove the ban, Node 0 should be able to reconnect even without noban permission self.nodes[1].setban("127.0.0.1", "remove") self.restart_node(1, []) self.connect_nodes(0, 1) peerinfo = self.nodes[1].getpeerinfo()[0] - assert(not 'noban' in peerinfo['permissions']) + assert not "noban" in peerinfo["permissions"] self.log.info("Test that a non-IP address can be banned/unbanned") node = self.nodes[1] tor_addr = "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion" ip_addr = "1.2.3.4" - assert(not self.is_banned(node, tor_addr)) - assert(not self.is_banned(node, ip_addr)) + assert not self.is_banned(node, tor_addr) + assert not self.is_banned(node, ip_addr) + node.setban(tor_addr, "add") - assert(self.is_banned(node, tor_addr)) - assert(not self.is_banned(node, ip_addr)) + assert self.is_banned(node, tor_addr) + assert not self.is_banned(node, ip_addr) + + self.log.info("Test the ban list is preserved through restart") + + self.restart_node(1) + assert self.is_banned(node, tor_addr) + assert not self.is_banned(node, ip_addr) + node.setban(tor_addr, "remove") - assert(not self.is_banned(self.nodes[1], tor_addr)) - assert(not self.is_banned(node, ip_addr)) + assert not self.is_banned(self.nodes[1], tor_addr) + assert not self.is_banned(node, ip_addr) + + self.restart_node(1) + assert not self.is_banned(node, tor_addr) + assert not self.is_banned(node, ip_addr) + if __name__ == '__main__': SetBanTests().main() diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index f35ea6c122..e91b44e776 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -181,11 +181,6 @@ def create_raw_transaction(node, txid, to_address, *, amount): signed_psbt = wrpc.walletprocesspsbt(psbt) psbt = signed_psbt['psbt'] final_psbt = node.finalizepsbt(psbt) - if not final_psbt["complete"]: - node.log.info(f'final_psbt={final_psbt}') - for w in node.listwallets(): - wrpc = node.get_wallet_rpc(w) - node.log.info(f'listunspent={wrpc.listunspent()}') assert_equal(final_psbt["complete"], True) return final_psbt['hex'] diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 5a9736a7a3..3d79174356 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -39,7 +39,7 @@ MAX_BLOOM_HASH_FUNCS = 50 COIN = 100000000 # 1 btc in satoshis MAX_MONEY = 21000000 * COIN -BIP125_SEQUENCE_NUMBER = 0xfffffffd # Sequence number that is BIP 125 opt-in and BIP 68-opt-out +BIP125_SEQUENCE_NUMBER = 0xfffffffd # Sequence number that is rbf-opt-in (BIP 125) and csv-opt-out (BIP 68) MAX_PROTOCOL_MESSAGE_LENGTH = 4000000 # Maximum length of incoming protocol messages MAX_HEADERS_RESULTS = 2000 # Number of headers sent in one getheaders result diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index a89a26caea..40360c54a0 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -194,6 +194,10 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): help="Run test using legacy wallets", dest='descriptors') self.add_options(parser) + # Running TestShell in a Jupyter notebook causes an additional -f argument + # To keep TestShell from failing with an "unrecognized argument" error, we add a dummy "-f" argument + # source: https://stackoverflow.com/questions/48796169/how-to-fix-ipykernel-launcher-py-error-unrecognized-arguments-in-jupyter/56349168#56349168 + parser.add_argument("-f", "--fff", help="a dummy argument to fool ipython", default="1") self.options = parser.parse_args() self.options.previous_releases_path = previous_releases_path diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index 0962a5cb54..47ec6b0be2 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -88,12 +88,20 @@ class MiniWallet: if out['scriptPubKey']['hex'] == self._scriptPubKey.hex(): self._utxos.append({'txid': tx['txid'], 'vout': out['n'], 'value': out['value']}) - def sign_tx(self, tx): + def sign_tx(self, tx, fixed_length=True): """Sign tx that has been created by MiniWallet in P2PK mode""" assert self._priv_key is not None (sighash, err) = LegacySignatureHash(CScript(self._scriptPubKey), tx, 0, SIGHASH_ALL) assert err is None - tx.vin[0].scriptSig = CScript([self._priv_key.sign_ecdsa(sighash) + bytes(bytearray([SIGHASH_ALL]))]) + # for exact fee calculation, create only signatures with fixed size by default (>49.89% probability): + # 65 bytes: high-R val (33 bytes) + low-S val (32 bytes) + # with the DER header/skeleton data of 6 bytes added, this leads to a target size of 71 bytes + der_sig = b'' + while not len(der_sig) == 71: + der_sig = self._priv_key.sign_ecdsa(sighash) + if not fixed_length: + break + tx.vin[0].scriptSig = CScript([der_sig + bytes(bytearray([SIGHASH_ALL]))]) def generate(self, num_blocks): """Generate blocks with coinbase outputs to the internal address, and append the outputs to the internal list""" @@ -124,23 +132,26 @@ class MiniWallet: else: return self._utxos[index] - def send_self_transfer(self, *, fee_rate=Decimal("0.003"), from_node, utxo_to_spend=None, locktime=0): + def send_self_transfer(self, **kwargs): """Create and send a tx with the specified fee_rate. Fee may be exact or at most one satoshi higher than needed.""" - tx = self.create_self_transfer(fee_rate=fee_rate, from_node=from_node, utxo_to_spend=utxo_to_spend) - self.sendrawtransaction(from_node=from_node, tx_hex=tx['hex']) + tx = self.create_self_transfer(**kwargs) + self.sendrawtransaction(from_node=kwargs['from_node'], tx_hex=tx['hex']) return tx - def create_self_transfer(self, *, fee_rate=Decimal("0.003"), from_node, utxo_to_spend=None, mempool_valid=True, locktime=0): + def create_self_transfer(self, *, fee_rate=Decimal("0.003"), from_node, utxo_to_spend=None, mempool_valid=True, locktime=0, sequence=0): """Create and return a tx with the specified fee_rate. Fee may be exact or at most one satoshi higher than needed.""" self._utxos = sorted(self._utxos, key=lambda k: k['value']) utxo_to_spend = utxo_to_spend or self._utxos.pop() # Pick the largest utxo (if none provided) and hope it covers the fee - vsize = Decimal(96) + if self._priv_key is None: + vsize = Decimal(96) # anyone-can-spend + else: + vsize = Decimal(168) # P2PK (73 bytes scriptSig + 35 bytes scriptPubKey + 60 bytes other) send_value = satoshi_round(utxo_to_spend['value'] - fee_rate * (vsize / 1000)) fee = utxo_to_spend['value'] - send_value assert send_value > 0 tx = CTransaction() - tx.vin = [CTxIn(COutPoint(int(utxo_to_spend['txid'], 16), utxo_to_spend['vout']))] + tx.vin = [CTxIn(COutPoint(int(utxo_to_spend['txid'], 16), utxo_to_spend['vout']), nSequence=sequence)] tx.vout = [CTxOut(int(send_value * COIN), self._scriptPubKey)] tx.nLockTime = locktime if not self._address: @@ -159,10 +170,7 @@ class MiniWallet: tx_info = from_node.testmempoolaccept([tx_hex])[0] assert_equal(mempool_valid, tx_info['allowed']) if mempool_valid: - # TODO: for P2PK, vsize is not constant due to varying scriptSig length, - # so only check this for anyone-can-spend outputs right now - if self._priv_key is None: - assert_equal(tx_info['vsize'], vsize) + assert_equal(tx_info['vsize'], vsize) assert_equal(tx_info['fees']['base'], fee) return {'txid': tx_info['txid'], 'wtxid': tx_info['wtxid'], 'hex': tx_hex, 'tx': tx} diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 76347b052d..ad1acd2e2f 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -174,6 +174,7 @@ BASE_SCRIPTS = [ 'wallet_groups.py --legacy-wallet', 'p2p_addrv2_relay.py', 'wallet_groups.py --descriptors', + 'p2p_compactblocks_hb.py', 'p2p_disconnect_ban.py', 'rpc_decodescript.py', 'rpc_blockchain.py', |