diff options
58 files changed, 583 insertions, 248 deletions
diff --git a/build_msvc/common.init.vcxproj b/build_msvc/common.init.vcxproj index c09997d39d..4fd516fff5 100644 --- a/build_msvc/common.init.vcxproj +++ b/build_msvc/common.init.vcxproj @@ -107,10 +107,10 @@ <ClCompile> <WarningLevel>Level3</WarningLevel> <PrecompiledHeader>NotUsing</PrecompiledHeader> - <AdditionalOptions>/utf-8 %(AdditionalOptions)</AdditionalOptions> + <AdditionalOptions>/utf-8 /std:c++17 %(AdditionalOptions)</AdditionalOptions> <DisableSpecificWarnings>4018;4221;4244;4267;4334;4715;4805;4834</DisableSpecificWarnings> <TreatWarningAsError>true</TreatWarningAsError> - <PreprocessorDefinitions>ZMQ_STATIC;NOMINMAX;WIN32;HAVE_CONFIG_H;_CRT_SECURE_NO_WARNINGS;_SCL_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;%(PreprocessorDefinitions)</PreprocessorDefinitions> + <PreprocessorDefinitions>_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;_SILENCE_CXX17_OLD_ALLOCATOR_MEMBERS_DEPRECATION_WARNING;ZMQ_STATIC;NOMINMAX;WIN32;HAVE_CONFIG_H;_CRT_SECURE_NO_WARNINGS;_SCL_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;%(PreprocessorDefinitions)</PreprocessorDefinitions> <AdditionalIncludeDirectories>..\..\src;..\..\src\univalue\include;..\..\src\secp256k1\include;..\..\src\leveldb\include;..\..\src\leveldb\helpers\memenv;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> </ClCompile> <Link> diff --git a/doc/developer-notes.md b/doc/developer-notes.md index bd3daa3202..6ae7e770e8 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -887,7 +887,7 @@ Others are external projects without a tight relationship with our project. Chan be sent upstream, but bugfixes may also be prudent to PR against Bitcoin Core so that they can be integrated quickly. Cosmetic changes should be purely taken upstream. -There is a tool in `test/lint/git-subtree-check.sh` to check a subtree directory for consistency with +There is a tool in `test/lint/git-subtree-check.sh` ([instructions](../test/lint#git-subtree-checksh)) to check a subtree directory for consistency with its upstream repository. Current subtrees include: diff --git a/doc/fuzzing.md b/doc/fuzzing.md index 419b1db44e..c97b8d4d50 100644 --- a/doc/fuzzing.md +++ b/doc/fuzzing.md @@ -121,6 +121,8 @@ $ git clone https://github.com/google/afl $ make -C afl/ $ make -C afl/llvm_mode/ $ ./autogen.sh +# It is possible to compile with afl-gcc and afl-g++ instead of afl-clang. However, running afl-fuzz +# may require more memory via the -m flag. $ CC=$(pwd)/afl/afl-clang-fast CXX=$(pwd)/afl/afl-clang-fast++ ./configure --enable-fuzz $ make # For macOS you may need to ignore x86 compilation checks when running "make". If so, diff --git a/doc/reduce-traffic.md b/doc/reduce-traffic.md index ce77a00dd5..86943b1f72 100644 --- a/doc/reduce-traffic.md +++ b/doc/reduce-traffic.md @@ -23,7 +23,7 @@ longer serving historic blocks (blocks older than one week). Keep in mind that new nodes require other nodes that are willing to serve historic blocks. -Peers with the `noban` permission will never be disconnected, although their traffic counts for +Peers with the `download` permission will never be disconnected, although their traffic counts for calculating the target. ## 2. Disable "listening" (`-listen=0`) @@ -50,7 +50,7 @@ Be reminded of the effects of this setting. Doing so disables the automatic broadcasting of transactions from wallet. Not relaying other's transactions could hurt your privacy if used while a wallet is loaded or if you use the node to broadcast transactions. -- If a peer is whitelisted and "-whitelistforcerelay" is set to "1" (which will - also set "whitelistrelay" to "1"), we will still receive and relay their transactions. +- If a peer has the forcerelay permission, we will still receive and relay + their transactions. - It makes block propagation slower because compact block relay can only be used when transaction relay is enabled. diff --git a/doc/release-notes-19469.md b/doc/release-notes-19469.md new file mode 100644 index 0000000000..974b228cc1 --- /dev/null +++ b/doc/release-notes-19469.md @@ -0,0 +1,6 @@ +Updated RPCs +------------ + +- `getpeerinfo` no longer returns the `banscore` field unless the configuration + option `-deprecatedrpc=banscore` is used. The `banscore` field will be fully + removed in the next major release. (#19469) diff --git a/doc/release-notes.md b/doc/release-notes.md index e73bedfb10..4d2225884e 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -102,6 +102,11 @@ Updated settings - The `-debug=db` logging category, which was deprecated in 0.20 and replaced by `-debug=walletdb` to distinguish it from `coindb`, has been removed. (#19202) +- A `download` permission has been extracted from the `noban` permission. For + compatibility, `noban` implies the `download` permission, but this may change + in future releases. Refer to the help of the affected settings `-whitebind` + and `-whitelist` for more details. (#19191) + Changes to Wallet or GUI related settings can be found in the GUI or Wallet section below. New settings diff --git a/share/examples/bitcoin.conf b/share/examples/bitcoin.conf index 96fb6658a0..90a592cc63 100644 --- a/share/examples/bitcoin.conf +++ b/share/examples/bitcoin.conf @@ -20,8 +20,8 @@ # Bind to given address and always listen on it. Use [host]:port notation for IPv6 #bind=<addr> -# Bind to given address and whitelist peers connecting to it. Use [host]:port notation for IPv6 -#whitebind=<addr> +# Bind to given address and add permission flags to peers connecting to it. Use [host]:port notation for IPv6 +#whitebind=perm@<addr> ############################################################## ## Quick Primer on addnode vs connect ## diff --git a/src/.clang-format b/src/.clang-format index a8f8565f80..ef7a0ef5c7 100644 --- a/src/.clang-format +++ b/src/.clang-format @@ -3,7 +3,6 @@ AccessModifierOffset: -4 AlignAfterOpenBracket: true AlignEscapedNewlinesLeft: true AlignTrailingComments: true -AllowAllArgumentsOnNextLine : true AllowAllParametersOfDeclarationOnNextLine: true AllowShortBlocksOnASingleLine: false AllowShortCaseLabelsOnASingleLine: true diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 9dc3078487..654d019d95 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -11,6 +11,7 @@ FUZZ_TARGETS = \ test/fuzz/asmap \ test/fuzz/asmap_direct \ test/fuzz/banentry_deserialize \ + test/fuzz/banman \ test/fuzz/base_encode_decode \ test/fuzz/bech32 \ test/fuzz/block \ @@ -355,6 +356,12 @@ test_fuzz_banentry_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_banentry_deserialize_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_banentry_deserialize_SOURCES = test/fuzz/deserialize.cpp +test_fuzz_banman_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_banman_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_banman_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_banman_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_banman_SOURCES = test/fuzz/banman.cpp + test_fuzz_base_encode_decode_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_base_encode_decode_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_base_encode_decode_LDADD = $(FUZZ_SUITE_LD_COMMON) diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 835c5d6c65..f3e8a19de2 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -8,6 +8,7 @@ #include <addrman.h> #include <chainparams.h> #include <clientversion.h> +#include <cstdint> #include <hash.h> #include <random.h> #include <streams.h> @@ -36,7 +37,7 @@ template <typename Data> bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data& data) { // Generate random temporary filename - unsigned short randv = 0; + uint16_t randv = 0; GetRandBytes((unsigned char*)&randv, sizeof(randv)); std::string tmpfn = strprintf("%s.%04x", prefix, randv); diff --git a/src/banman.cpp b/src/banman.cpp index 2fabacda0e..8752185a60 100644 --- a/src/banman.cpp +++ b/src/banman.cpp @@ -26,7 +26,7 @@ BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t SweepBanned(); // sweep out unused entries LogPrint(BCLog::NET, "Loaded %d banned node ips/subnets from banlist.dat %dms\n", - banmap.size(), GetTimeMillis() - n_start); + m_banned.size(), GetTimeMillis() - n_start); } else { LogPrintf("Invalid or missing banlist.dat; recreating\n"); SetBannedSetDirty(true); // force write diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index f5125f22db..9afcda4578 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -516,8 +516,8 @@ static void ParseError(const UniValue& error, std::string& strPrint, int& nRet) */ static void GetWalletBalances(UniValue& result) { - std::unique_ptr<BaseRequestHandler> rh{MakeUnique<DefaultRequestHandler>()}; - const UniValue listwallets = ConnectAndCallRPC(rh.get(), "listwallets", /* args=*/{}); + DefaultRequestHandler rh; + const UniValue listwallets = ConnectAndCallRPC(&rh, "listwallets", /* args=*/{}); if (!find_value(listwallets, "error").isNull()) return; const UniValue& wallets = find_value(listwallets, "result"); if (wallets.size() <= 1) return; @@ -525,7 +525,7 @@ static void GetWalletBalances(UniValue& result) UniValue balances(UniValue::VOBJ); for (const UniValue& wallet : wallets.getValues()) { const std::string wallet_name = wallet.get_str(); - const UniValue getbalances = ConnectAndCallRPC(rh.get(), "getbalances", /* args=*/{}, wallet_name); + const UniValue getbalances = ConnectAndCallRPC(&rh, "getbalances", /* args=*/{}, wallet_name); const UniValue& balance = find_value(getbalances, "result")["mine"]["trusted"]; balances.pushKV(wallet_name, balance); } @@ -540,8 +540,8 @@ static UniValue GetNewAddress() { Optional<std::string> wallet_name{}; if (gArgs.IsArgSet("-rpcwallet")) wallet_name = gArgs.GetArg("-rpcwallet", ""); - std::unique_ptr<BaseRequestHandler> rh{MakeUnique<DefaultRequestHandler>()}; - return ConnectAndCallRPC(rh.get(), "getnewaddress", /* args=*/{}, wallet_name); + DefaultRequestHandler rh; + return ConnectAndCallRPC(&rh, "getnewaddress", /* args=*/{}, wallet_name); } /** diff --git a/src/dbwrapper.h b/src/dbwrapper.h index 116d7d8679..215b033708 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -292,18 +292,6 @@ public: // Get an estimate of LevelDB memory usage (in bytes). size_t DynamicMemoryUsage() const; - // not available for LevelDB; provide for compatibility with BDB - bool Flush() - { - return true; - } - - bool Sync() - { - CDBBatch batch(*this); - return WriteBatch(batch, true); - } - CDBIterator *NewIterator() { return new CDBIterator(*this, pdb->NewIterator(iteroptions)); diff --git a/src/init.cpp b/src/init.cpp index 2e39bcd9a5..16f765574f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -396,7 +396,7 @@ void SetupServerArgs(NodeContext& node) gArgs.AddArg("-blocknotify=<cmd>", "Execute command when the best block changes (%s in cmd is replaced by block hash)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); #endif gArgs.AddArg("-blockreconstructionextratxn=<n>", strprintf("Extra transactions to keep in memory for compact block reconstructions (default: %u)", DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - gArgs.AddArg("-blocksonly", strprintf("Whether to reject transactions from network peers. Automatic broadcast and rebroadcast of any transactions from inbound peers is disabled, unless '-whitelistforcerelay' is '1', in which case whitelisted peers' transactions will be relayed. RPC transactions are not affected. (default: %u)", DEFAULT_BLOCKSONLY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + gArgs.AddArg("-blocksonly", strprintf("Whether to reject transactions from network peers. Automatic broadcast and rebroadcast of any transactions from inbound peers is disabled, unless the peer has the 'forcerelay' permission. RPC transactions are not affected. (default: %u)", DEFAULT_BLOCKSONLY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); gArgs.AddArg("-conf=<file>", strprintf("Specify configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); gArgs.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); gArgs.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS); @@ -446,7 +446,7 @@ void SetupServerArgs(NodeContext& node) gArgs.AddArg("-maxreceivebuffer=<n>", strprintf("Maximum per-connection receive buffer, <n>*1000 bytes (default: %u)", DEFAULT_MAXRECEIVEBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); gArgs.AddArg("-maxsendbuffer=<n>", strprintf("Maximum per-connection send buffer, <n>*1000 bytes (default: %u)", DEFAULT_MAXSENDBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); gArgs.AddArg("-maxtimeadjustment", strprintf("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by peers forward or backward by this amount. (default: %u seconds)", DEFAULT_MAX_TIME_ADJUSTMENT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - gArgs.AddArg("-maxuploadtarget=<n>", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h). Limit does not apply to peers with 'noban' permission. 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + gArgs.AddArg("-maxuploadtarget=<n>", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h). Limit does not apply to peers with 'download' permission. 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); gArgs.AddArg("-onion=<ip:port>", "Use separate SOCKS5 proxy to reach peers via Tor hidden services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); gArgs.AddArg("-onlynet=<net>", "Make outgoing connections only through network <net> (ipv4, ipv6 or onion). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); gArgs.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -469,12 +469,12 @@ void SetupServerArgs(NodeContext& node) #else hidden_args.emplace_back("-upnp"); #endif - gArgs.AddArg("-whitebind=<[permissions@]addr>", "Bind to given address and whitelist peers connecting to it. " + gArgs.AddArg("-whitebind=<[permissions@]addr>", "Bind to the given address and add permission flags to the peers connecting to it. " "Use [host]:port notation for IPv6. Allowed permissions: " + Join(NET_PERMISSIONS_DOC, ", ") + ". " - "Specify multiple permissions separated by commas (default: noban,mempool,relay). Can be specified multiple times.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + "Specify multiple permissions separated by commas (default: download,noban,mempool,relay). Can be specified multiple times.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - gArgs.AddArg("-whitelist=<[permissions@]IP address or network>", "Whitelist peers connecting from the given IP address (e.g. 1.2.3.4) or " - "CIDR notated network(e.g. 1.2.3.0/24). Uses same permissions as " + gArgs.AddArg("-whitelist=<[permissions@]IP address or network>", "Add permission flags to the peers connecting from the given IP address (e.g. 1.2.3.4) or " + "CIDR-notated network (e.g. 1.2.3.0/24). Uses the same permissions as " "-whitebind. Can be specified multiple times." , ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); g_wallet_init_interface.AddWalletOptions(); diff --git a/src/miner.cpp b/src/miner.cpp index d9dcbe8a70..41a835f70a 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -109,7 +109,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc if(!pblocktemplate.get()) return nullptr; - pblock = &pblocktemplate->block; // pointer for convenience + CBlock* const pblock = &pblocktemplate->block; // pointer for convenience // Add dummy coinbase tx as first transaction pblock->vtx.emplace_back(); @@ -226,7 +226,7 @@ bool BlockAssembler::TestPackageTransactions(const CTxMemPool::setEntries& packa void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) { - pblock->vtx.emplace_back(iter->GetSharedTx()); + pblocktemplate->block.vtx.emplace_back(iter->GetSharedTx()); pblocktemplate->vTxFees.push_back(iter->GetFee()); pblocktemplate->vTxSigOpsCost.push_back(iter->GetSigOpCost()); nBlockWeight += iter->GetTxWeight(); diff --git a/src/miner.h b/src/miner.h index 69296f9078..096585dfe4 100644 --- a/src/miner.h +++ b/src/miner.h @@ -128,8 +128,6 @@ class BlockAssembler private: // The constructed block template std::unique_ptr<CBlockTemplate> pblocktemplate; - // A convenience pointer that always refers to the CBlock in pblocktemplate - CBlock* pblock; // Configuration parameters for the block size bool fIncludeWitness; diff --git a/src/net.cpp b/src/net.cpp index 21d340b516..244b0094d6 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -42,6 +42,7 @@ static_assert(MINIUPNPC_API_VERSION >= 10, "miniUPnPc API version >= 10 assumed"); #endif +#include <cstdint> #include <unordered_map> #include <math.h> @@ -110,9 +111,9 @@ void CConnman::AddOneShot(const std::string& strDest) vOneShots.push_back(strDest); } -unsigned short GetListenPort() +uint16_t GetListenPort() { - return (unsigned short)(gArgs.GetArg("-port", Params().GetDefaultPort())); + return (uint16_t)(gArgs.GetArg("-port", Params().GetDefaultPort())); } // find 'best' local address for a particular peer @@ -563,15 +564,15 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap) // since pingtime does not update until the ping is complete, which might take a while. // So, if a ping is taking an unusually long time in flight, // the caller can immediately detect that this is happening. - int64_t nPingUsecWait = 0; - if ((0 != nPingNonceSent) && (0 != nPingUsecStart)) { - nPingUsecWait = GetTimeMicros() - nPingUsecStart; + std::chrono::microseconds ping_wait{0}; + if ((0 != nPingNonceSent) && (0 != m_ping_start.load().count())) { + ping_wait = GetTime<std::chrono::microseconds>() - m_ping_start.load(); } // Raw ping time is in microseconds, but show it to user as whole seconds (Bitcoin users should be well used to small numbers with many decimal places by now :) stats.m_ping_usec = nPingUsecTime; stats.m_min_ping_usec = nMinPingUsecTime; - stats.m_ping_wait_usec = nPingUsecWait; + stats.m_ping_wait_usec = count_microseconds(ping_wait); // Leave string empty if addrLocal invalid (not filled in yet) CService addrLocalUnlocked = GetAddrLocal(); @@ -582,9 +583,9 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap) bool CNode::ReceiveMsgBytes(const char *pch, unsigned int nBytes, bool& complete) { complete = false; - int64_t nTimeMicros = GetTimeMicros(); + const auto time = GetTime<std::chrono::microseconds>(); LOCK(cs_vRecv); - nLastRecv = nTimeMicros / 1000000; + nLastRecv = std::chrono::duration_cast<std::chrono::seconds>(time).count(); nRecvBytes += nBytes; while (nBytes > 0) { // absorb network data @@ -596,7 +597,7 @@ bool CNode::ReceiveMsgBytes(const char *pch, unsigned int nBytes, bool& complete if (m_deserializer->Complete()) { // decompose a transport agnostic CNetMessage from the deserializer - CNetMessage msg = m_deserializer->GetMessage(Params().MessageStart(), nTimeMicros); + CNetMessage msg = m_deserializer->GetMessage(Params().MessageStart(), time); //store received bytes per message command //to prevent a memory DOS, only allow valid commands @@ -699,7 +700,8 @@ const uint256& V1TransportDeserializer::GetMessageHash() const return data_hash; } -CNetMessage V1TransportDeserializer::GetMessage(const CMessageHeader::MessageStartChars& message_start, int64_t time) { +CNetMessage V1TransportDeserializer::GetMessage(const CMessageHeader::MessageStartChars& message_start, const std::chrono::microseconds time) +{ // decompose a single CNetMessage from the TransportDeserializer CNetMessage msg(std::move(vRecv)); @@ -1158,9 +1160,9 @@ void CConnman::InactivityCheck(CNode *pnode) LogPrintf("socket receive timeout: %is\n", nTime - pnode->nLastRecv); pnode->fDisconnect = true; } - else if (pnode->nPingNonceSent && pnode->nPingUsecStart + TIMEOUT_INTERVAL * 1000000 < GetTimeMicros()) + else if (pnode->nPingNonceSent && pnode->m_ping_start.load() + std::chrono::seconds{TIMEOUT_INTERVAL} < GetTime<std::chrono::microseconds>()) { - LogPrintf("ping timeout: %fs\n", 0.000001 * (GetTimeMicros() - pnode->nPingUsecStart)); + LogPrintf("ping timeout: %fs\n", 0.000001 * count_microseconds(GetTime<std::chrono::microseconds>() - pnode->m_ping_start.load())); pnode->fDisconnect = true; } else if (!pnode->fSuccessfullyConnected) @@ -2509,11 +2511,6 @@ CConnman::~CConnman() Stop(); } -size_t CConnman::GetAddressCount() const -{ - return addrman.size(); -} - void CConnman::SetServices(const CService &addr, ServiceFlags nServices) { addrman.SetServices(addr, nServices); @@ -2643,7 +2640,7 @@ void CConnman::RecordBytesSent(uint64_t bytes) nMaxOutboundTotalBytesSentInCycle = 0; } - // TODO, exclude peers with noban permission + // TODO, exclude peers with download permission nMaxOutboundTotalBytesSentInCycle += bytes; } @@ -25,8 +25,8 @@ #include <uint256.h> #include <atomic> +#include <cstdint> #include <deque> -#include <stdint.h> #include <thread> #include <memory> #include <condition_variable> @@ -247,7 +247,6 @@ public: }; // Addrman functions - size_t GetAddressCount() const; void SetServices(const CService &addr, ServiceFlags nServices); void MarkAddressGood(const CAddress& addr); void AddNewAddresses(const std::vector<CAddress>& vAddr, const CAddress& addrFrom, int64_t nTimePenalty = 0); @@ -482,7 +481,7 @@ void Discover(); void StartMapPort(); void InterruptMapPort(); void StopMapPort(); -unsigned short GetListenPort(); +uint16_t GetListenPort(); struct CombinerAll { @@ -612,13 +611,13 @@ public: */ class CNetMessage { public: - CDataStream m_recv; // received message data - int64_t m_time = 0; // time (in microseconds) of message receipt. + CDataStream m_recv; //!< received message data + std::chrono::microseconds m_time{0}; //!< time of message receipt bool m_valid_netmagic = false; bool m_valid_header = false; bool m_valid_checksum = false; - uint32_t m_message_size = 0; // size of the payload - uint32_t m_raw_message_size = 0; // used wire size of the message (including header/checksum) + uint32_t m_message_size{0}; //!< size of the payload + uint32_t m_raw_message_size{0}; //!< used wire size of the message (including header/checksum) std::string m_command; CNetMessage(CDataStream&& recv_in) : m_recv(std::move(recv_in)) {} @@ -642,7 +641,7 @@ public: // read and deserialize data virtual int Read(const char *data, unsigned int bytes) = 0; // decomposes a message from the context - virtual CNetMessage GetMessage(const CMessageHeader::MessageStartChars& message_start, int64_t time) = 0; + virtual CNetMessage GetMessage(const CMessageHeader::MessageStartChars& message_start, std::chrono::microseconds time) = 0; virtual ~TransportDeserializer() {} }; @@ -695,7 +694,7 @@ public: if (ret < 0) Reset(); return ret; } - CNetMessage GetMessage(const CMessageHeader::MessageStartChars& message_start, int64_t time) override; + CNetMessage GetMessage(const CMessageHeader::MessageStartChars& message_start, std::chrono::microseconds time) override; }; /** The TransportSerializer prepares messages for the network transport @@ -845,8 +844,8 @@ public: // Ping time measurement: // The pong reply we're expecting, or 0 if no pong expected. std::atomic<uint64_t> nPingNonceSent{0}; - // Time (in usec) the last ping was sent, or 0 if no ping was ever sent. - std::atomic<int64_t> nPingUsecStart{0}; + /** When the last ping was sent, or 0 if no ping was ever sent */ + std::atomic<std::chrono::microseconds> m_ping_start{std::chrono::microseconds{0}}; // Last measured round-trip time. std::atomic<int64_t> nPingUsecTime{0}; // Best measured round-trip time. diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp index da09149856..a75838307c 100644 --- a/src/net_permissions.cpp +++ b/src/net_permissions.cpp @@ -10,10 +10,11 @@ const std::vector<std::string> NET_PERMISSIONS_DOC{ "bloomfilter (allow requesting BIP37 filtered blocks and transactions)", - "noban (do not ban for misbehavior)", + "noban (do not ban for misbehavior; implies download)", "forcerelay (relay transactions that are already in the mempool; implies relay)", "relay (relay even in -blocksonly mode)", "mempool (allow requesting BIP35 mempool contents)", + "download (allow getheaders during IBD, no disconnect after maxuploadtarget limit)", }; namespace { @@ -46,6 +47,7 @@ bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, else if (permission == "noban") NetPermissions::AddFlag(flags, PF_NOBAN); else if (permission == "forcerelay") NetPermissions::AddFlag(flags, PF_FORCERELAY); else if (permission == "mempool") NetPermissions::AddFlag(flags, PF_MEMPOOL); + else if (permission == "download") NetPermissions::AddFlag(flags, PF_DOWNLOAD); else if (permission == "all") NetPermissions::AddFlag(flags, PF_ALL); else if (permission == "relay") NetPermissions::AddFlag(flags, PF_RELAY); else if (permission.length() == 0); // Allow empty entries @@ -72,6 +74,7 @@ std::vector<std::string> NetPermissions::ToStrings(NetPermissionFlags flags) if (NetPermissions::HasFlag(flags, PF_FORCERELAY)) strings.push_back("forcerelay"); if (NetPermissions::HasFlag(flags, PF_RELAY)) strings.push_back("relay"); if (NetPermissions::HasFlag(flags, PF_MEMPOOL)) strings.push_back("mempool"); + if (NetPermissions::HasFlag(flags, PF_DOWNLOAD)) strings.push_back("download"); return strings; } diff --git a/src/net_permissions.h b/src/net_permissions.h index 2cf85120fe..a9633ee2ae 100644 --- a/src/net_permissions.h +++ b/src/net_permissions.h @@ -14,8 +14,7 @@ struct bilingual_str; extern const std::vector<std::string> NET_PERMISSIONS_DOC; -enum NetPermissionFlags -{ +enum NetPermissionFlags { PF_NONE = 0, // Can query bloomfilter even if -peerbloomfilters is false PF_BLOOMFILTER = (1U << 1), @@ -24,14 +23,16 @@ enum NetPermissionFlags // Always relay transactions from this peer, even if already in mempool // Keep parameter interaction: forcerelay implies relay PF_FORCERELAY = (1U << 2) | PF_RELAY, + // Allow getheaders during IBD and block-download after maxuploadtarget limit + PF_DOWNLOAD = (1U << 6), // Can't be banned/disconnected/discouraged for misbehavior - PF_NOBAN = (1U << 4), + PF_NOBAN = (1U << 4) | PF_DOWNLOAD, // Can query the mempool PF_MEMPOOL = (1U << 5), // True if the user did not specifically set fine grained permissions PF_ISIMPLICIT = (1U << 31), - PF_ALL = PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY | PF_NOBAN | PF_MEMPOOL, + PF_ALL = PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY | PF_NOBAN | PF_MEMPOOL | PF_DOWNLOAD, }; class NetPermissions diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f1a89a8936..bfc60b18f9 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -63,8 +63,8 @@ static constexpr int STALE_RELAY_AGE_LIMIT = 30 * 24 * 60 * 60; /// Age after which a block is considered historical for purposes of rate /// limiting block relay. Set to one week, denominated in seconds. static constexpr int HISTORICAL_BLOCK_AGE = 7 * 24 * 60 * 60; -/** Time between pings automatically sent out for latency probing and keepalive (in seconds). */ -static const int PING_INTERVAL = 2 * 60; +/** Time between pings automatically sent out for latency probing and keepalive */ +static constexpr std::chrono::minutes PING_INTERVAL{2}; /** The maximum number of entries in a locator */ static const unsigned int MAX_LOCATOR_SZ = 101; /** The maximum number of entries in an 'inv' protocol message */ @@ -117,7 +117,7 @@ static constexpr std::chrono::hours AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL{24}; /** Average delay between peer address broadcasts */ static constexpr std::chrono::seconds AVG_ADDRESS_BROADCAST_INTERVAL{30}; /** Average delay between trickled inventory transmissions in seconds. - * Blocks and whitelisted receivers bypass this, outbound peers get half this delay. */ + * Blocks and peers with noban permission bypass this, outbound peers get half this delay. */ static const unsigned int INVENTORY_BROADCAST_INTERVAL = 5; /** Maximum number of inventory items to send per transmission. * Limits the impact of low-fee transaction floods. */ @@ -249,7 +249,7 @@ struct CNodeState { bool fCurrentlyConnected; //! Accumulated misbehaviour score for this peer. int nMisbehavior; - //! Whether this peer should be disconnected and marked as discouraged (unless whitelisted with noban). + //! Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission). bool m_should_discourage; //! String name of this peer (debugging/logging purposes). const std::string name; @@ -1438,7 +1438,7 @@ static void RelayAddress(const CAddress& addr, bool fReachable, const CConnman& assert(nRelayNodes <= best.size()); auto sortfunc = [&best, &hasher, nRelayNodes](CNode* pnode) { - if (pnode->nVersion >= CADDR_TIME_VERSION && pnode->IsAddrRelayPeer()) { + if (pnode->IsAddrRelayPeer()) { uint64_t hashKey = CSipHasher(hasher).Write(pnode->GetId()).Finalize(); for (unsigned int i = 0; i < nRelayNodes; i++) { if (hashKey > best[i].first) { @@ -1509,7 +1509,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c if (send && connman->OutboundTargetReached(true) && (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && - !pfrom.HasPermission(PF_NOBAN) // never disconnect nodes with the noban permission + !pfrom.HasPermission(PF_DOWNLOAD) // nodes with the download permission may exceed target ) { LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId()); @@ -1895,8 +1895,8 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman* connman, ChainstateMan // headers to fetch from this peer. if (nodestate->pindexBestKnownBlock && nodestate->pindexBestKnownBlock->nChainWork < nMinimumChainWork) { // This peer has too little work on their headers chain to help - // us sync -- disconnect if using an outbound slot (unless - // whitelisted or addnode). + // us sync -- disconnect if it is an outbound disconnection + // candidate. // Note: We compare their tip to nMinimumChainWork (rather than // ::ChainActive().Tip()) because we won't start block download // until we have a headers chain that has at least @@ -2206,7 +2206,7 @@ void ProcessMessage( CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, - int64_t nTimeReceived, + const std::chrono::microseconds time_received, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, @@ -2349,11 +2349,8 @@ void ProcessMessage( } // Get recent addresses - if (pfrom.fOneShot || pfrom.nVersion >= CADDR_TIME_VERSION || connman->GetAddressCount() < 1000) - { - connman->PushMessage(&pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETADDR)); - pfrom.fGetAddr = true; - } + connman->PushMessage(&pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETADDR)); + pfrom.fGetAddr = true; connman->MarkAddressGood(pfrom.addr); } @@ -2443,9 +2440,6 @@ void ProcessMessage( std::vector<CAddress> vAddr; vRecv >> vAddr; - // Don't want addr from older versions unless seeding - if (pfrom.nVersion < CADDR_TIME_VERSION && connman->GetAddressCount() > 1000) - return; if (!pfrom.IsAddrRelayPeer()) { return; } @@ -2537,9 +2531,10 @@ void ProcessMessage( // block-relay-only peer bool fBlocksOnly = !g_relay_txes || (pfrom.m_tx_relay == nullptr); - // Allow whitelisted peers to send data other than blocks in blocks only mode if whitelistrelay is true - if (pfrom.HasPermission(PF_RELAY)) + // Allow peers with relay permission to send data other than blocks in blocks only mode + if (pfrom.HasPermission(PF_RELAY)) { fBlocksOnly = false; + } LOCK(cs_main); @@ -2739,7 +2734,7 @@ void ProcessMessage( } LOCK(cs_main); - if (::ChainstateActive().IsInitialBlockDownload() && !pfrom.HasPermission(PF_NOBAN)) { + if (::ChainstateActive().IsInitialBlockDownload() && !pfrom.HasPermission(PF_DOWNLOAD)) { LogPrint(BCLog::NET, "Ignoring getheaders from peer=%d because node is in initial block download\n", pfrom.GetId()); return; } @@ -2893,14 +2888,14 @@ void ProcessMessage( } if (pfrom.HasPermission(PF_FORCERELAY)) { - // Always relay transactions received from whitelisted peers, even + // Always relay transactions received from peers with forcerelay permission, even // if they were already in the mempool, // allowing the node to function as a gateway for // nodes hidden behind it. if (!mempool.exists(tx.GetHash())) { - LogPrintf("Not relaying non-mempool transaction %s from whitelisted peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); + LogPrintf("Not relaying non-mempool transaction %s from forcerelay peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); } else { - LogPrintf("Force relaying tx %s from whitelisted peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); + LogPrintf("Force relaying tx %s from peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); RelayTransaction(tx.GetHash(), *connman); } } @@ -3050,7 +3045,7 @@ void ProcessMessage( PartiallyDownloadedBlock& partialBlock = *(*queuedBlockIt)->partialBlock; ReadStatus status = partialBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status == READ_STATUS_INVALID) { - MarkBlockAsReceived(pindex->GetBlockHash()); // Reset in-flight state in case of whitelist + MarkBlockAsReceived(pindex->GetBlockHash()); // Reset in-flight state in case Misbehaving does not result in a disconnect Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block\n", pfrom.GetId())); return; } else if (status == READ_STATUS_FAILED) { @@ -3110,7 +3105,7 @@ void ProcessMessage( } // cs_main if (fProcessBLOCKTXN) - return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, nTimeReceived, chainparams, chainman, mempool, connman, banman, interruptMsgProc); + return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, time_received, chainparams, chainman, mempool, connman, banman, interruptMsgProc); if (fRevertToHeaderProcessing) { // Headers received from HB compact block peers are permitted to be @@ -3183,7 +3178,7 @@ void ProcessMessage( PartiallyDownloadedBlock& partialBlock = *it->second.second->partialBlock; ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); if (status == READ_STATUS_INVALID) { - MarkBlockAsReceived(resp.blockhash); // Reset in-flight state in case of whitelist + MarkBlockAsReceived(resp.blockhash); // Reset in-flight state in case Misbehaving does not result in a disconnect Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block/non-matching block transactions\n", pfrom.GetId())); return; } else if (status == READ_STATUS_FAILED) { @@ -3385,7 +3380,7 @@ void ProcessMessage( } if (msg_type == NetMsgType::PONG) { - int64_t pingUsecEnd = nTimeReceived; + const auto ping_end = time_received; uint64_t nonce = 0; size_t nAvail = vRecv.in_avail(); bool bPingFinished = false; @@ -3399,11 +3394,11 @@ void ProcessMessage( if (nonce == pfrom.nPingNonceSent) { // Matching pong received, this ping is no longer outstanding bPingFinished = true; - int64_t pingUsecTime = pingUsecEnd - pfrom.nPingUsecStart; - if (pingUsecTime > 0) { + const auto ping_time = ping_end - pfrom.m_ping_start.load(); + if (ping_time.count() > 0) { // Successful ping time measurement, replace previous - pfrom.nPingUsecTime = pingUsecTime; - pfrom.nMinPingUsecTime = std::min(pfrom.nMinPingUsecTime.load(), pingUsecTime); + pfrom.nPingUsecTime = count_microseconds(ping_time); + pfrom.nMinPingUsecTime = std::min(pfrom.nMinPingUsecTime.load(), count_microseconds(ping_time)); } else { // This should never happen sProblem = "Timing mishap"; @@ -3860,7 +3855,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // RPC ping request by user pingSend = true; } - if (pto->nPingNonceSent == 0 && pto->nPingUsecStart + PING_INTERVAL * 1000000 < GetTimeMicros()) { + if (pto->nPingNonceSent == 0 && pto->m_ping_start.load() + PING_INTERVAL < GetTime<std::chrono::microseconds>()) { // Ping automatically sent as a latency probe & keepalive. pingSend = true; } @@ -3870,7 +3865,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) GetRandBytes((unsigned char*)&nonce, sizeof(nonce)); } pto->fPingQueued = false; - pto->nPingUsecStart = GetTimeMicros(); + pto->m_ping_start = GetTime<std::chrono::microseconds>(); if (pto->nVersion > BIP0031_VERSION) { pto->nPingNonceSent = nonce; connman->PushMessage(pto, msgMaker.Make(NetMsgType::PING, nonce)); @@ -4264,9 +4259,9 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // Check for headers sync timeouts if (state.fSyncStarted && state.nHeadersSyncTimeout < std::numeric_limits<int64_t>::max()) { // Detect whether this is a stalling initial-headers-sync peer - if (pindexBestHeader->GetBlockTime() <= GetAdjustedTime() - 24*60*60) { + if (pindexBestHeader->GetBlockTime() <= GetAdjustedTime() - 24 * 60 * 60) { if (nNow > state.nHeadersSyncTimeout && nSyncStarted == 1 && (nPreferredDownload - state.fPreferredDownload >= 1)) { - // Disconnect a (non-whitelisted) peer if it is our only sync peer, + // Disconnect a peer (without the noban permission) if it is our only sync peer, // and we have others we could be using instead. // Note: If all our peers are inbound, then we won't // disconnect our sync peer for stalling; we have bigger @@ -4276,7 +4271,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) pto->fDisconnect = true; return true; } else { - LogPrintf("Timeout downloading headers from whitelisted peer=%d, not disconnecting\n", pto->GetId()); + LogPrintf("Timeout downloading headers from noban peer=%d, not disconnecting\n", pto->GetId()); // Reset the headers sync state so that we have a // chance to try downloading from a different peer. // Note: this will also result in at least one more diff --git a/src/netaddress.cpp b/src/netaddress.cpp index f79425a52e..6744391616 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -3,6 +3,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <cstdint> #include <netaddress.h> #include <hash.h> #include <util/strencodings.h> @@ -627,15 +628,15 @@ CService::CService() : port(0) { } -CService::CService(const CNetAddr& cip, unsigned short portIn) : CNetAddr(cip), port(portIn) +CService::CService(const CNetAddr& cip, uint16_t portIn) : CNetAddr(cip), port(portIn) { } -CService::CService(const struct in_addr& ipv4Addr, unsigned short portIn) : CNetAddr(ipv4Addr), port(portIn) +CService::CService(const struct in_addr& ipv4Addr, uint16_t portIn) : CNetAddr(ipv4Addr), port(portIn) { } -CService::CService(const struct in6_addr& ipv6Addr, unsigned short portIn) : CNetAddr(ipv6Addr), port(portIn) +CService::CService(const struct in6_addr& ipv6Addr, uint16_t portIn) : CNetAddr(ipv6Addr), port(portIn) { } @@ -663,7 +664,7 @@ bool CService::SetSockAddr(const struct sockaddr *paddr) } } -unsigned short CService::GetPort() const +uint16_t CService::GetPort() const { return port; } diff --git a/src/netaddress.h b/src/netaddress.h index 552abc4119..c201012154 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -12,7 +12,7 @@ #include <compat.h> #include <serialize.h> -#include <stdint.h> +#include <cstdint> #include <string> #include <vector> @@ -143,10 +143,10 @@ class CService : public CNetAddr public: CService(); - CService(const CNetAddr& ip, unsigned short port); - CService(const struct in_addr& ipv4Addr, unsigned short port); + CService(const CNetAddr& ip, uint16_t port); + CService(const struct in_addr& ipv4Addr, uint16_t port); explicit CService(const struct sockaddr_in& addr); - unsigned short GetPort() const; + uint16_t GetPort() const; bool GetSockAddr(struct sockaddr* paddr, socklen_t *addrlen) const; bool SetSockAddr(const struct sockaddr* paddr); friend bool operator==(const CService& a, const CService& b); @@ -157,7 +157,7 @@ class CService : public CNetAddr std::string ToStringPort() const; std::string ToStringIPPort() const; - CService(const struct in6_addr& ipv6Addr, unsigned short port); + CService(const struct in6_addr& ipv6Addr, uint16_t port); explicit CService(const struct sockaddr_in6& addr); SERIALIZE_METHODS(CService, obj) { READWRITE(obj.ip, Using<BigEndianFormatter<2>>(obj.port)); } diff --git a/src/netbase.cpp b/src/netbase.cpp index 9fe03c6a24..3a3b5f3e66 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -12,6 +12,7 @@ #include <util/system.h> #include <atomic> +#include <cstdint> #ifndef WIN32 #include <fcntl.h> @@ -798,11 +799,11 @@ bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int ProxyCredentials random_auth; static std::atomic_int counter(0); random_auth.username = random_auth.password = strprintf("%i", counter++); - if (!Socks5(strDest, (unsigned short)port, &random_auth, hSocket)) { + if (!Socks5(strDest, (uint16_t)port, &random_auth, hSocket)) { return false; } } else { - if (!Socks5(strDest, (unsigned short)port, 0, hSocket)) { + if (!Socks5(strDest, (uint16_t)port, 0, hSocket)) { return false; } } diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 3c5e8d0d07..65f226a925 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -683,6 +683,7 @@ void BitcoinGUI::removeWallet(WalletModel* walletModel) m_wallet_selector->removeItem(index); if (m_wallet_selector->count() == 0) { setWalletActionsEnabled(false); + overviewAction->setChecked(true); } else if (m_wallet_selector->count() == 1) { m_wallet_selector_label_action->setVisible(false); m_wallet_selector_action->setVisible(false); diff --git a/src/qt/optionsmodel.h b/src/qt/optionsmodel.h index 6ca5ac9d75..14fdf9046e 100644 --- a/src/qt/optionsmodel.h +++ b/src/qt/optionsmodel.h @@ -6,6 +6,7 @@ #define BITCOIN_QT_OPTIONSMODEL_H #include <amount.h> +#include <cstdint> #include <qt/guiconstants.h> #include <QAbstractListModel> @@ -15,7 +16,7 @@ class Node; } extern const char *DEFAULT_GUI_PROXY_HOST; -static constexpr unsigned short DEFAULT_GUI_PROXY_PORT = 9050; +static constexpr uint16_t DEFAULT_GUI_PROXY_PORT = 9050; /** * Convert configured prune target MiB to displayed GB. Round up to avoid underestimating max disk usage. diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index ab2c37e6f8..71094f7112 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -467,6 +467,7 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty // Install event filter for up and down arrow ui->lineEdit->installEventFilter(this); + ui->lineEdit->setMaxLength(16 * 1024 * 1024); ui->messagesWidget->installEventFilter(this); connect(ui->clearButton, &QPushButton::clicked, this, &RPCConsole::clear); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 98d4b2ce81..c6c78a983a 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -798,10 +798,8 @@ static CBlock GetBlockChecked(const CBlockIndex* pblockindex) if (!ReadBlockFromDisk(block, pblockindex, Params().GetConsensus())) { // Block not found on disk. This could be because we have the block - // header in our index but don't have the block (for example if a - // non-whitelisted node sends us an unrequested long chain of valid - // blocks, we add the headers to our index, but don't accept the - // block). + // header in our index but not yet have the block or did not accept the + // block. throw JSONRPCError(RPC_MISC_ERROR, "Block not found on disk"); } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 393442e946..9981ea35df 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -112,7 +112,7 @@ static UniValue getpeerinfo(const JSONRPCRequest& request) {RPCResult::Type::BOOL, "inbound", "Inbound (true) or Outbound (false)"}, {RPCResult::Type::BOOL, "addnode", "Whether connection was due to addnode/-connect or if it was an automatic/inbound connection"}, {RPCResult::Type::NUM, "startingheight", "The starting height (block) of the peer"}, - {RPCResult::Type::NUM, "banscore", "The ban score"}, + {RPCResult::Type::NUM, "banscore", "The ban score (DEPRECATED, returned only if config option -deprecatedrpc=banscore is passed)"}, {RPCResult::Type::NUM, "synced_headers", "The last header we have in common with this peer"}, {RPCResult::Type::NUM, "synced_blocks", "The last block we have in common with this peer"}, {RPCResult::Type::ARR, "inflight", "", @@ -191,7 +191,10 @@ static UniValue getpeerinfo(const JSONRPCRequest& request) obj.pushKV("addnode", stats.m_manual_connection); obj.pushKV("startingheight", stats.nStartingHeight); if (fStateStats) { - obj.pushKV("banscore", statestats.nMisbehavior); + if (IsDeprecatedRPCEnabled("banscore")) { + // banscore is deprecated in v0.21 for removal in v0.22 + obj.pushKV("banscore", statestats.nMisbehavior); + } obj.pushKV("synced_headers", statestats.nSyncHeight); obj.pushKV("synced_blocks", statestats.nCommonHeight); UniValue heights(UniValue::VARR); diff --git a/src/serialize.h b/src/serialize.h index 71c2cfa164..7a94e704b2 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -9,13 +9,13 @@ #include <compat/endian.h> #include <algorithm> +#include <cstdint> #include <cstring> #include <ios> #include <limits> #include <map> #include <memory> #include <set> -#include <stdint.h> #include <string> #include <string.h> #include <utility> @@ -272,7 +272,7 @@ template<typename Stream> inline void Unserialize(Stream& s, bool& a) { char f=s inline unsigned int GetSizeOfCompactSize(uint64_t nSize) { if (nSize < 253) return sizeof(unsigned char); - else if (nSize <= std::numeric_limits<unsigned short>::max()) return sizeof(unsigned char) + sizeof(unsigned short); + else if (nSize <= std::numeric_limits<uint16_t>::max()) return sizeof(unsigned char) + sizeof(uint16_t); else if (nSize <= std::numeric_limits<unsigned int>::max()) return sizeof(unsigned char) + sizeof(unsigned int); else return sizeof(unsigned char) + sizeof(uint64_t); } @@ -286,7 +286,7 @@ void WriteCompactSize(Stream& os, uint64_t nSize) { ser_writedata8(os, nSize); } - else if (nSize <= std::numeric_limits<unsigned short>::max()) + else if (nSize <= std::numeric_limits<uint16_t>::max()) { ser_writedata8(os, 253); ser_writedata16(os, nSize); diff --git a/src/test/fuzz/banman.cpp b/src/test/fuzz/banman.cpp new file mode 100644 index 0000000000..fc4a1d9261 --- /dev/null +++ b/src/test/fuzz/banman.cpp @@ -0,0 +1,88 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <banman.h> +#include <fs.h> +#include <netaddress.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <util/system.h> + +#include <cstdint> +#include <limits> +#include <string> +#include <vector> + +namespace { +int64_t ConsumeBanTimeOffset(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + // Avoid signed integer overflow by capping to int32_t max: + // banman.cpp:137:73: runtime error: signed integer overflow: 1591700817 + 9223372036854775807 cannot be represented in type 'long' + return fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(std::numeric_limits<int64_t>::min(), std::numeric_limits<int32_t>::max()); +} +} // namespace + +void initialize() +{ + InitializeFuzzingContext(); +} + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + const fs::path banlist_file = GetDataDir() / "fuzzed_banlist.dat"; + fs::remove(banlist_file); + { + BanMan ban_man{banlist_file, nullptr, ConsumeBanTimeOffset(fuzzed_data_provider)}; + while (fuzzed_data_provider.ConsumeBool()) { + switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 11)) { + case 0: { + ban_man.Ban(ConsumeNetAddr(fuzzed_data_provider), + ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool()); + break; + } + case 1: { + ban_man.Ban(ConsumeSubNet(fuzzed_data_provider), + ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool()); + break; + } + case 2: { + ban_man.ClearBanned(); + break; + } + case 4: { + ban_man.IsBanned(ConsumeNetAddr(fuzzed_data_provider)); + break; + } + case 5: { + ban_man.IsBanned(ConsumeSubNet(fuzzed_data_provider)); + break; + } + case 6: { + ban_man.Unban(ConsumeNetAddr(fuzzed_data_provider)); + break; + } + case 7: { + ban_man.Unban(ConsumeSubNet(fuzzed_data_provider)); + break; + } + case 8: { + banmap_t banmap; + ban_man.GetBanned(banmap); + break; + } + case 9: { + ban_man.DumpBanlist(); + break; + } + case 11: { + ban_man.Discourage(ConsumeNetAddr(fuzzed_data_provider)); + break; + } + } + } + } + fs::remove(banlist_file); +} diff --git a/src/test/fuzz/http_request.cpp b/src/test/fuzz/http_request.cpp index ebf89749e9..36d44e361f 100644 --- a/src/test/fuzz/http_request.cpp +++ b/src/test/fuzz/http_request.cpp @@ -7,6 +7,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <util/strencodings.h> #include <event2/buffer.h> #include <event2/event.h> @@ -48,7 +49,14 @@ void test_one_input(const std::vector<uint8_t>& buffer) assert(evbuf != nullptr); const std::vector<uint8_t> http_buffer = ConsumeRandomLengthByteVector(fuzzed_data_provider, 4096); evbuffer_add(evbuf, http_buffer.data(), http_buffer.size()); - if (evhttp_parse_firstline_(evreq, evbuf) != 1 || evhttp_parse_headers_(evreq, evbuf) != 1) { + // Avoid constructing requests that will be interpreted by libevent as PROXY requests to avoid triggering + // a nullptr dereference. The dereference (req->evcon->http_server) takes place in evhttp_parse_request_line + // and is a consequence of our hacky but necessary use of the internal function evhttp_parse_firstline_ in + // this fuzzing harness. The workaround is not aesthetically pleasing, but it successfully avoids the troublesome + // code path. " http:// HTTP/1.1\n" was a crashing input prior to this workaround. + const std::string http_buffer_str = ToLower({http_buffer.begin(), http_buffer.end()}); + if (http_buffer_str.find(" http://") != std::string::npos || http_buffer_str.find(" https://") != std::string::npos || + evhttp_parse_firstline_(evreq, evbuf) != 1 || evhttp_parse_headers_(evreq, evbuf) != 1) { evbuffer_free(evbuf); evhttp_request_free(evreq); return; diff --git a/src/test/fuzz/netaddress.cpp b/src/test/fuzz/netaddress.cpp index d8d53566c7..2901c704f6 100644 --- a/src/test/fuzz/netaddress.cpp +++ b/src/test/fuzz/netaddress.cpp @@ -5,41 +5,13 @@ #include <netaddress.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> #include <cassert> #include <cstdint> #include <netinet/in.h> #include <vector> -namespace { -CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept -{ - const Network network = fuzzed_data_provider.PickValueInArray({Network::NET_IPV4, Network::NET_IPV6, Network::NET_INTERNAL, Network::NET_ONION}); - if (network == Network::NET_IPV4) { - const in_addr v4_addr = { - .s_addr = fuzzed_data_provider.ConsumeIntegral<uint32_t>()}; - return CNetAddr{v4_addr}; - } else if (network == Network::NET_IPV6) { - if (fuzzed_data_provider.remaining_bytes() < 16) { - return CNetAddr{}; - } - in6_addr v6_addr = {}; - memcpy(v6_addr.s6_addr, fuzzed_data_provider.ConsumeBytes<uint8_t>(16).data(), 16); - return CNetAddr{v6_addr, fuzzed_data_provider.ConsumeIntegral<uint32_t>()}; - } else if (network == Network::NET_INTERNAL) { - CNetAddr net_addr; - net_addr.SetInternal(fuzzed_data_provider.ConsumeBytesAsString(32)); - return net_addr; - } else if (network == Network::NET_ONION) { - CNetAddr net_addr; - net_addr.SetSpecial(fuzzed_data_provider.ConsumeBytesAsString(32)); - return net_addr; - } else { - assert(false); - } -} -}; // namespace - void test_one_input(const std::vector<uint8_t>& buffer) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); diff --git a/src/test/fuzz/p2p_transport_deserializer.cpp b/src/test/fuzz/p2p_transport_deserializer.cpp index 57393fed45..6fba2bfaba 100644 --- a/src/test/fuzz/p2p_transport_deserializer.cpp +++ b/src/test/fuzz/p2p_transport_deserializer.cpp @@ -30,7 +30,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) pch += handled; n_bytes -= handled; if (deserializer.Complete()) { - const int64_t m_time = std::numeric_limits<int64_t>::max(); + const std::chrono::microseconds m_time{std::numeric_limits<int64_t>::max()}; const CNetMessage msg = deserializer.GetMessage(Params().MessageStart(), m_time); assert(msg.m_command.size() <= CMessageHeader::COMMAND_SIZE); assert(msg.m_raw_message_size <= buffer.size()); diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 2fa751b987..fa8d67059c 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -34,7 +34,7 @@ void ProcessMessage( CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, - int64_t nTimeReceived, + const std::chrono::microseconds time_received, const CChainParams& chainparams, ChainstateManager& chainman, CTxMemPool& mempool, @@ -87,7 +87,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) connman.AddTestNode(p2p_node); g_setup->m_node.peer_logic->InitializeNode(&p2p_node); try { - ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), + ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream, GetTime<std::chrono::microseconds>(), Params(), *g_setup->m_node.chainman, *g_setup->m_node.mempool, g_setup->m_node.connman.get(), g_setup->m_node.banman.get(), std::atomic<bool>{false}); diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 1c1b2cd254..8cf91ef940 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -8,8 +8,11 @@ #include <amount.h> #include <arith_uint256.h> #include <attributes.h> +#include <chainparamsbase.h> #include <coins.h> #include <consensus/consensus.h> +#include <netaddress.h> +#include <netbase.h> #include <primitives/transaction.h> #include <script/script.h> #include <script/standard.h> @@ -17,6 +20,7 @@ #include <streams.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> +#include <test/util/setup_common.h> #include <txmempool.h> #include <uint256.h> #include <version.h> @@ -228,4 +232,36 @@ NODISCARD inline std::vector<uint8_t> ConsumeFixedLengthByteVector(FuzzedDataPro return result; } +CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + const Network network = fuzzed_data_provider.PickValueInArray({Network::NET_IPV4, Network::NET_IPV6, Network::NET_INTERNAL, Network::NET_ONION}); + CNetAddr net_addr; + if (network == Network::NET_IPV4) { + const in_addr v4_addr = { + .s_addr = fuzzed_data_provider.ConsumeIntegral<uint32_t>()}; + net_addr = CNetAddr{v4_addr}; + } else if (network == Network::NET_IPV6) { + if (fuzzed_data_provider.remaining_bytes() >= 16) { + in6_addr v6_addr = {}; + memcpy(v6_addr.s6_addr, fuzzed_data_provider.ConsumeBytes<uint8_t>(16).data(), 16); + net_addr = CNetAddr{v6_addr, fuzzed_data_provider.ConsumeIntegral<uint32_t>()}; + } + } else if (network == Network::NET_INTERNAL) { + net_addr.SetInternal(fuzzed_data_provider.ConsumeBytesAsString(32)); + } else if (network == Network::NET_ONION) { + net_addr.SetSpecial(fuzzed_data_provider.ConsumeBytesAsString(32)); + } + return net_addr; +} + +CSubNet ConsumeSubNet(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + return {ConsumeNetAddr(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<int32_t>()}; +} + +void InitializeFuzzingContext(const std::string& chain_name = CBaseChainParams::REGTEST) +{ + static const BasicTestingSetup basic_testing_setup{chain_name, {"-nodebuglogfile"}}; +} + #endif // BITCOIN_TEST_FUZZ_UTIL_H diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 84bf593497..ab42be21bd 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -6,6 +6,7 @@ #include <addrman.h> #include <chainparams.h> #include <clientversion.h> +#include <cstdint> #include <net.h> #include <netbase.h> #include <serialize.h> @@ -83,10 +84,10 @@ BOOST_FIXTURE_TEST_SUITE(net_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(cnode_listen_port) { // test default - unsigned short port = GetListenPort(); + uint16_t port = GetListenPort(); BOOST_CHECK(port == Params().GetDefaultPort()); // test set port - unsigned short altPort = 12345; + uint16_t altPort = 12345; BOOST_CHECK(gArgs.SoftSetArg("-port", ToString(altPort))); port = GetListenPort(); BOOST_CHECK(port == altPort); diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 0fbf257f0e..ea3e633cc2 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -383,7 +383,7 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) BOOST_CHECK(!NetWhitebindPermissions::TryParse("bloom,forcerelay,oopsie@1.2.3.4:32", whitebindPermissions, error)); BOOST_CHECK(error.original.find("Invalid P2P permission") != std::string::npos); - // Check whitelist error + // Check netmask error BOOST_CHECK(!NetWhitelistPermissions::TryParse("bloom,forcerelay,noban@1.2.3.4:32", whitelistPermissions, error)); BOOST_CHECK(error.original.find("Invalid netmask specified in -whitelist") != std::string::npos); @@ -397,12 +397,13 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) BOOST_CHECK(NetWhitelistPermissions::TryParse("bloom,forcerelay,noban,relay,mempool@1.2.3.4/32", whitelistPermissions, error)); const auto strings = NetPermissions::ToStrings(PF_ALL); - BOOST_CHECK_EQUAL(strings.size(), 5U); + BOOST_CHECK_EQUAL(strings.size(), 6U); BOOST_CHECK(std::find(strings.begin(), strings.end(), "bloomfilter") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "forcerelay") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "relay") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "noban") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "mempool") != strings.end()); + BOOST_CHECK(std::find(strings.begin(), strings.end(), "download") != strings.end()); } BOOST_AUTO_TEST_CASE(netbase_dont_resolve_strings_with_embedded_nul_characters) diff --git a/src/util/time.h b/src/util/time.h index b00c25f67c..af934e423b 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -15,10 +15,15 @@ void UninterruptibleSleep(const std::chrono::microseconds& n); /** * Helper to count the seconds of a duration. * - * All durations should be using std::chrono and calling this should generally be avoided in code. Though, it is still - * preferred to an inline t.count() to protect against a reliance on the exact type of t. + * All durations should be using std::chrono and calling this should generally + * be avoided in code. Though, it is still preferred to an inline t.count() to + * protect against a reliance on the exact type of t. + * + * This helper is used to convert durations before passing them over an + * interface that doesn't support std::chrono (e.g. RPC, debug log, or the GUI) */ inline int64_t count_seconds(std::chrono::seconds t) { return t.count(); } +inline int64_t count_microseconds(std::chrono::microseconds t) { return t.count(); } /** * DEPRECATED diff --git a/src/validation.cpp b/src/validation.cpp index f28b2a4c82..edc623b205 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1421,12 +1421,12 @@ void static InvalidChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(c pindexBestHeader = ::ChainActive().Tip(); } - LogPrintf("%s: invalid block=%s height=%d log2_work=%.8g date=%s\n", __func__, + LogPrintf("%s: invalid block=%s height=%d log2_work=%f date=%s\n", __func__, pindexNew->GetBlockHash().ToString(), pindexNew->nHeight, log(pindexNew->nChainWork.getdouble())/log(2.0), FormatISO8601DateTime(pindexNew->GetBlockTime())); CBlockIndex *tip = ::ChainActive().Tip(); assert (tip); - LogPrintf("%s: current best=%s height=%d log2_work=%.8g date=%s\n", __func__, + LogPrintf("%s: current best=%s height=%d log2_work=%f date=%s\n", __func__, tip->GetBlockHash().ToString(), ::ChainActive().Height(), log(tip->nChainWork.getdouble())/log(2.0), FormatISO8601DateTime(tip->GetBlockTime())); CheckForkWarningConditions(); @@ -2476,7 +2476,7 @@ void static UpdateTip(const CBlockIndex* pindexNew, const CChainParams& chainPar if (nUpgraded > 0) AppendWarning(warning_messages, strprintf(_("%d of last 100 blocks have unexpected version"), nUpgraded)); } - LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%.8g tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)%s\n", __func__, + LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%f tx=%lu date='%s' progress=%f cache=%.1fMiB(%utxo)%s\n", __func__, pindexNew->GetBlockHash().ToString(), pindexNew->nHeight, pindexNew->nVersion, log(pindexNew->nChainWork.getdouble())/log(2.0), (unsigned long)pindexNew->nChainTx, FormatISO8601DateTime(pindexNew->GetBlockTime()), diff --git a/src/validation.h b/src/validation.h index a148dacb7c..9b49627135 100644 --- a/src/validation.h +++ b/src/validation.h @@ -845,7 +845,7 @@ public: * validationinterface callback. * * @param[in] pblock The block we want to process. - * @param[in] fForceProcessing Process this block even if unrequested; used for non-network block sources and whitelisted peers. + * @param[in] fForceProcessing Process this block even if unrequested; used for non-network block sources. * @param[out] fNewBlock A boolean which is set to indicate if the block was first received via this call * @returns If the block was processed, independently of block validity */ diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index c9ea6c2ad9..3b752ca936 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1547,7 +1547,7 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue& if (!w_desc.descriptor->GetOutputType()) { warnings.push_back("Unknown output type, cannot set descriptor to active."); } else { - pwallet->SetActiveScriptPubKeyMan(spk_manager->GetID(), *w_desc.descriptor->GetOutputType(), internal); + pwallet->AddActiveScriptPubKeyMan(spk_manager->GetID(), *w_desc.descriptor->GetOutputType(), internal); } } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 38d94335a3..51715462c5 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -905,20 +905,22 @@ bool LegacyScriptPubKeyMan::AddWatchOnly(const CScript& dest, int64_t nCreateTim return AddWatchOnly(dest); } -void LegacyScriptPubKeyMan::SetHDChain(const CHDChain& chain, bool memonly) +void LegacyScriptPubKeyMan::LoadHDChain(const CHDChain& chain) { LOCK(cs_KeyStore); - // memonly == true means we are loading the wallet file - // memonly == false means that the chain is actually being changed - if (!memonly) { - // Store the new chain - if (!WalletBatch(m_storage.GetDatabase()).WriteHDChain(chain)) { - throw std::runtime_error(std::string(__func__) + ": writing chain failed"); - } - // When there's an old chain, add it as an inactive chain as we are now rotating hd chains - if (!m_hd_chain.seed_id.IsNull()) { - AddInactiveHDChain(m_hd_chain); - } + m_hd_chain = chain; +} + +void LegacyScriptPubKeyMan::AddHDChain(const CHDChain& chain) +{ + LOCK(cs_KeyStore); + // Store the new chain + if (!WalletBatch(m_storage.GetDatabase()).WriteHDChain(chain)) { + throw std::runtime_error(std::string(__func__) + ": writing chain failed"); + } + // When there's an old chain, add it as an inactive chain as we are now rotating hd chains + if (!m_hd_chain.seed_id.IsNull()) { + AddInactiveHDChain(m_hd_chain); } m_hd_chain = chain; @@ -1172,7 +1174,7 @@ void LegacyScriptPubKeyMan::SetHDSeed(const CPubKey& seed) CHDChain newHdChain; newHdChain.nVersion = m_storage.CanSupportFeature(FEATURE_HD_SPLIT) ? CHDChain::VERSION_HD_CHAIN_SPLIT : CHDChain::VERSION_HD_BASE; newHdChain.seed_id = seed.GetID(); - SetHDChain(newHdChain, false); + AddHDChain(newHdChain); NotifyCanGetAddressesChanged(); WalletBatch batch(m_storage.GetDatabase()); m_storage.UnsetBlankWalletFlag(batch); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 9fa2a68284..a96d971734 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -422,8 +422,10 @@ public: //! Generate a new key CPubKey GenerateNewKey(WalletBatch& batch, CHDChain& hd_chain, bool internal = false) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore); - /* Set the HD chain model (chain child index counters) */ - void SetHDChain(const CHDChain& chain, bool memonly); + /* Set the HD chain model (chain child index counters) and writes it to the database */ + void AddHDChain(const CHDChain& chain); + //! Load a HD chain model (used by LoadWallet) + void LoadHDChain(const CHDChain& chain); const CHDChain& GetHDChain() const { return m_hd_chain; } void AddInactiveHDChain(const CHDChain& chain); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 5c565a3d38..0f7c3e5b39 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -791,4 +791,37 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletFromFile, TestChain100Setup) TestUnloadWallet(std::move(wallet)); } +BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup) +{ + auto chain = interfaces::MakeChain(m_node); + auto wallet = TestLoadWallet(*chain); + CKey key; + key.MakeNewKey(true); + AddKey(*wallet, key); + + std::string error; + m_coinbase_txns.push_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); + auto block_tx = TestSimpleSpend(*m_coinbase_txns[0], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey())); + CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); + + SyncWithValidationInterfaceQueue(); + + { + auto block_hash = block_tx.GetHash(); + auto prev_hash = m_coinbase_txns[0]->GetHash(); + + LOCK(wallet->cs_wallet); + BOOST_CHECK(wallet->HasWalletSpend(prev_hash)); + BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_hash), 1); + + std::vector<uint256> vHashIn{ block_hash }, vHashOut; + BOOST_CHECK_EQUAL(wallet->ZapSelectTx(vHashIn, vHashOut), DBErrors::LOAD_OK); + + BOOST_CHECK(!wallet->HasWalletSpend(prev_hash)); + BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_hash), 0); + } + + TestUnloadWallet(std::move(wallet)); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 29ff7bbef1..256fd753db 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1422,19 +1422,28 @@ bool CWallet::IsWalletFlagSet(uint64_t flag) const return (m_wallet_flags & flag); } -bool CWallet::SetWalletFlags(uint64_t overwriteFlags, bool memonly) +bool CWallet::LoadWalletFlags(uint64_t flags) { LOCK(cs_wallet); - m_wallet_flags = overwriteFlags; - if (((overwriteFlags & KNOWN_WALLET_FLAGS) >> 32) ^ (overwriteFlags >> 32)) { + if (((flags & KNOWN_WALLET_FLAGS) >> 32) ^ (flags >> 32)) { // contains unknown non-tolerable wallet flags return false; } - if (!memonly && !WalletBatch(*database).WriteWalletFlags(m_wallet_flags)) { + m_wallet_flags = flags; + + return true; +} + +bool CWallet::AddWalletFlags(uint64_t flags) +{ + LOCK(cs_wallet); + // We should never be writing unknown onon-tolerable wallet flags + assert(!(((flags & KNOWN_WALLET_FLAGS) >> 32) ^ (flags >> 32))); + if (!WalletBatch(*database).WriteWalletFlags(flags)) { throw std::runtime_error(std::string(__func__) + ": writing wallet flags failed"); } - return true; + return LoadWalletFlags(flags); } int64_t CWalletTx::GetTxTime() const @@ -3120,9 +3129,11 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256 { AssertLockHeld(cs_wallet); DBErrors nZapSelectTxRet = WalletBatch(*database, "cr+").ZapSelectTx(vHashIn, vHashOut); - for (uint256 hash : vHashOut) { + for (const uint256& hash : vHashOut) { const auto& it = mapWallet.find(hash); wtxOrdered.erase(it->second.m_it_wtxOrdered); + for (const auto& txin : it->second.tx->vin) + mapTxSpends.erase(txin.prevout); mapWallet.erase(it); NotifyTransactionChanged(this, hash, CT_DELETED); } @@ -3796,7 +3807,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, // ensure this wallet.dat can only be opened by clients supporting HD with chain split and expects no default key walletInstance->SetMinVersion(FEATURE_LATEST); - walletInstance->SetWalletFlags(wallet_creation_flags, false); + walletInstance->AddWalletFlags(wallet_creation_flags); // Only create LegacyScriptPubKeyMan when not descriptor wallet if (!walletInstance->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { @@ -4417,12 +4428,21 @@ void CWallet::SetupDescriptorScriptPubKeyMans() spk_manager->SetupDescriptorGeneration(master_key, t); uint256 id = spk_manager->GetID(); m_spk_managers[id] = std::move(spk_manager); - SetActiveScriptPubKeyMan(id, t, internal); + AddActiveScriptPubKeyMan(id, t, internal); } } } -void CWallet::SetActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal, bool memonly) +void CWallet::AddActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal) +{ + WalletBatch batch(*database); + if (!batch.WriteActiveScriptPubKeyMan(static_cast<uint8_t>(type), id, internal)) { + throw std::runtime_error(std::string(__func__) + ": writing active ScriptPubKeyMan id failed"); + } + LoadActiveScriptPubKeyMan(id, type, internal); +} + +void CWallet::LoadActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal) { WalletLogPrintf("Setting spkMan to active: id = %s, type = %d, internal = %d\n", id.ToString(), static_cast<int>(type), static_cast<int>(internal)); auto& spk_mans = internal ? m_internal_spk_managers : m_external_spk_managers; @@ -4430,12 +4450,6 @@ void CWallet::SetActiveScriptPubKeyMan(uint256 id, OutputType type, bool interna spk_man->SetInternal(internal); spk_mans[type] = spk_man; - if (!memonly) { - WalletBatch batch(*database); - if (!batch.WriteActiveScriptPubKeyMan(static_cast<uint8_t>(type), id, internal)) { - throw std::runtime_error(std::string(__func__) + ": writing active ScriptPubKeyMan id failed"); - } - } NotifyCanGetAddressesChanged(); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 32d8481cd8..8cb2a64484 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1176,7 +1176,9 @@ public: /** overwrite all flags by the given uint64_t returns false if unknown, non-tolerable flags are present */ - bool SetWalletFlags(uint64_t overwriteFlags, bool memOnly); + bool AddWalletFlags(uint64_t flags); + /** Loads the flags into the wallet. (used by LoadWallet) */ + bool LoadWalletFlags(uint64_t flags); /** Determine if we are a legacy wallet */ bool IsLegacy() const; @@ -1254,12 +1256,17 @@ public: //! Instantiate a descriptor ScriptPubKeyMan from the WalletDescriptor and load it void LoadDescriptorScriptPubKeyMan(uint256 id, WalletDescriptor& desc); - //! Sets the active ScriptPubKeyMan for the specified type and internal + //! Adds the active ScriptPubKeyMan for the specified type and internal. Writes it to the wallet file //! @param[in] id The unique id for the ScriptPubKeyMan //! @param[in] type The OutputType this ScriptPubKeyMan provides addresses for //! @param[in] internal Whether this ScriptPubKeyMan provides change addresses - //! @param[in] memonly Whether to record this update to the database. Set to true for wallet loading, normally false when actually updating the wallet. - void SetActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal, bool memonly = false); + void AddActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal); + + //! Loads an active ScriptPubKeyMan for the specified type and internal. (used by LoadWallet) + //! @param[in] id The unique id for the ScriptPubKeyMan + //! @param[in] type The OutputType this ScriptPubKeyMan provides addresses for + //! @param[in] internal Whether this ScriptPubKeyMan provides change addresses + void LoadActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal); //! Create new DescriptorScriptPubKeyMans and add them to the wallet void SetupDescriptorScriptPubKeyMans(); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 98ccc3198a..9d53f0df9c 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -539,11 +539,11 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, } else if (strType == DBKeys::HDCHAIN) { CHDChain chain; ssValue >> chain; - pwallet->GetOrCreateLegacyScriptPubKeyMan()->SetHDChain(chain, true); + pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadHDChain(chain); } else if (strType == DBKeys::FLAGS) { uint64_t flags; ssValue >> flags; - if (!pwallet->SetWalletFlags(flags, true)) { + if (!pwallet->LoadWalletFlags(flags)) { strErr = "Error reading wallet database: Unknown non-tolerable wallet flags found"; return false; } @@ -752,10 +752,10 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Set the active ScriptPubKeyMans for (auto spk_man_pair : wss.m_active_external_spks) { - pwallet->SetActiveScriptPubKeyMan(spk_man_pair.second, spk_man_pair.first, /* internal */ false, /* memonly */ true); + pwallet->LoadActiveScriptPubKeyMan(spk_man_pair.second, spk_man_pair.first, /* internal */ false); } for (auto spk_man_pair : wss.m_active_internal_spks) { - pwallet->SetActiveScriptPubKeyMan(spk_man_pair.second, spk_man_pair.first, /* internal */ true, /* memonly */ true); + pwallet->LoadActiveScriptPubKeyMan(spk_man_pair.second, spk_man_pair.first, /* internal */ true); } // Set the descriptor caches diff --git a/test/functional/feature_bip68_sequence.py b/test/functional/feature_bip68_sequence.py index 549e8b2029..19cdc10935 100755 --- a/test/functional/feature_bip68_sequence.py +++ b/test/functional/feature_bip68_sequence.py @@ -30,7 +30,10 @@ class BIP68Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 self.extra_args = [ - ["-acceptnonstdtxn=1"], + [ + "-acceptnonstdtxn=1", + "-peertimeout=9999", # bump because mocktime might cause a disconnect otherwise + ], ["-acceptnonstdtxn=0"], ] diff --git a/test/functional/feature_maxuploadtarget.py b/test/functional/feature_maxuploadtarget.py index 7eabf86cad..5538d6d3b4 100755 --- a/test/functional/feature_maxuploadtarget.py +++ b/test/functional/feature_maxuploadtarget.py @@ -35,7 +35,11 @@ class MaxUploadTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 - self.extra_args = [["-maxuploadtarget=800", "-acceptnonstdtxn=1"]] + self.extra_args = [[ + "-maxuploadtarget=800", + "-acceptnonstdtxn=1", + "-peertimeout=9999", # bump because mocktime might cause a disconnect otherwise + ]] self.supports_cli = False # Cache for utxos, as the listunspent may take a long time later in the test @@ -137,8 +141,8 @@ class MaxUploadTest(BitcoinTestFramework): self.nodes[0].disconnect_p2ps() - self.log.info("Restarting node 0 with noban permission and 1MB maxuploadtarget") - self.restart_node(0, ["-whitelist=noban@127.0.0.1", "-maxuploadtarget=1"]) + self.log.info("Restarting node 0 with download permission and 1MB maxuploadtarget") + self.restart_node(0, ["-whitelist=download@127.0.0.1", "-maxuploadtarget=1"]) # Reconnect to self.nodes[0] self.nodes[0].add_p2p_connection(TestP2PConn()) @@ -151,9 +155,12 @@ class MaxUploadTest(BitcoinTestFramework): getdata_request.inv = [CInv(MSG_BLOCK, big_old_block)] self.nodes[0].p2p.send_and_ping(getdata_request) - assert_equal(len(self.nodes[0].getpeerinfo()), 1) #node is still connected because of the noban permission - self.log.info("Peer still connected after trying to download old block (noban permission)") + self.log.info("Peer still connected after trying to download old block (download permission)") + peer_info = self.nodes[0].getpeerinfo() + assert_equal(len(peer_info), 1) # node is still connected + assert_equal(peer_info[0]['permissions'], ['download']) + if __name__ == '__main__': MaxUploadTest().main() diff --git a/test/functional/p2p_blocksonly.py b/test/functional/p2p_blocksonly.py index c155dda664..f42a343042 100755 --- a/test/functional/p2p_blocksonly.py +++ b/test/functional/p2p_blocksonly.py @@ -57,29 +57,30 @@ class P2PBlocksOnly(BitcoinTestFramework): self.nodes[0].p2p.wait_for_tx(txid) assert_equal(self.nodes[0].getmempoolinfo()['size'], 1) - self.log.info('Check that txs from whitelisted peers are not rejected and relayed to others') - self.log.info("Restarting node 0 with whitelist permission and blocksonly") + self.log.info('Check that txs from forcerelay peers are not rejected and relayed to others') + self.log.info("Restarting node 0 with forcerelay permission and blocksonly") self.restart_node(0, ["-persistmempool=0", "-whitelist=127.0.0.1", "-whitelistforcerelay", "-blocksonly"]) - assert_equal(self.nodes[0].getrawmempool(),[]) + assert_equal(self.nodes[0].getrawmempool(), []) first_peer = self.nodes[0].add_p2p_connection(P2PInterface()) second_peer = self.nodes[0].add_p2p_connection(P2PInterface()) peer_1_info = self.nodes[0].getpeerinfo()[0] assert_equal(peer_1_info['whitelisted'], True) - assert_equal(peer_1_info['permissions'], ['noban', 'forcerelay', 'relay', 'mempool']) + assert_equal(peer_1_info['permissions'], ['noban', 'forcerelay', 'relay', 'mempool', 'download']) peer_2_info = self.nodes[0].getpeerinfo()[1] assert_equal(peer_2_info['whitelisted'], True) - assert_equal(peer_2_info['permissions'], ['noban', 'forcerelay', 'relay', 'mempool']) + assert_equal(peer_2_info['permissions'], ['noban', 'forcerelay', 'relay', 'mempool', 'download']) assert_equal(self.nodes[0].testmempoolaccept([sigtx])[0]['allowed'], True) txid = self.nodes[0].testmempoolaccept([sigtx])[0]['txid'] - self.log.info('Check that the tx from whitelisted first_peer is relayed to others (ie.second_peer)') + self.log.info('Check that the tx from forcerelay first_peer is relayed to others (ie.second_peer)') with self.nodes[0].assert_debug_log(["received getdata"]): first_peer.send_message(msg_tx(FromHex(CTransaction(), sigtx))) - self.log.info('Check that the whitelisted peer is still connected after sending the transaction') + self.log.info('Check that the forcerelay peer is still connected after sending the transaction') assert_equal(first_peer.is_connected, True) second_peer.wait_for_tx(txid) assert_equal(self.nodes[0].getmempoolinfo()['size'], 1) - self.log.info("Whitelisted peer's transaction is accepted and relayed") + self.log.info("Forcerelay peer's transaction is accepted and relayed") + if __name__ == '__main__': P2PBlocksOnly().main() diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py index bea202855d..32a795e345 100755 --- a/test/functional/p2p_permissions.py +++ b/test/functional/p2p_permissions.py @@ -39,7 +39,8 @@ class P2PPermissionsTests(BitcoinTestFramework): self.checkpermission( # default permissions (no specific permissions) ["-whitelist=127.0.0.1"], - ["relay", "noban", "mempool"], + # Make sure the default values in the command line documentation match the ones here + ["relay", "noban", "mempool", "download"], True) self.checkpermission( @@ -51,7 +52,7 @@ class P2PPermissionsTests(BitcoinTestFramework): self.checkpermission( # relay permission removed (no specific permissions) ["-whitelist=127.0.0.1", "-whitelistrelay=0"], - ["noban", "mempool"], + ["noban", "mempool", "download"], True) self.checkpermission( @@ -59,7 +60,7 @@ class P2PPermissionsTests(BitcoinTestFramework): # Legacy parameter interaction which set whitelistrelay to true # if whitelistforcerelay is true ["-whitelist=127.0.0.1", "-whitelistforcerelay"], - ["forcerelay", "relay", "noban", "mempool"], + ["forcerelay", "relay", "noban", "mempool", "download"], True) # Let's make sure permissions are merged correctly @@ -70,32 +71,32 @@ class P2PPermissionsTests(BitcoinTestFramework): self.checkpermission( ["-whitelist=noban@127.0.0.1"], # Check parameter interaction forcerelay should activate relay - ["noban", "bloomfilter", "forcerelay", "relay"], + ["noban", "bloomfilter", "forcerelay", "relay", "download"], False) self.replaceinconfig(1, "whitebind=bloomfilter,forcerelay@" + ip_port, "bind=127.0.0.1") self.checkpermission( # legacy whitelistrelay should be ignored ["-whitelist=noban,mempool@127.0.0.1", "-whitelistrelay"], - ["noban", "mempool"], + ["noban", "mempool", "download"], False) self.checkpermission( # legacy whitelistforcerelay should be ignored ["-whitelist=noban,mempool@127.0.0.1", "-whitelistforcerelay"], - ["noban", "mempool"], + ["noban", "mempool", "download"], False) self.checkpermission( # missing mempool permission to be considered legacy whitelisted ["-whitelist=noban@127.0.0.1"], - ["noban"], + ["noban", "download"], False) self.checkpermission( # all permission added ["-whitelist=all@127.0.0.1"], - ["forcerelay", "noban", "mempool", "bloomfilter", "relay"], + ["forcerelay", "noban", "mempool", "bloomfilter", "relay", "download"], False) self.stop_node(1) @@ -107,9 +108,9 @@ class P2PPermissionsTests(BitcoinTestFramework): block_op_true = self.nodes[0].getblock(self.nodes[0].generatetoaddress(100, ADDRESS_BCRT1_P2WSH_OP_TRUE)[0]) self.sync_all() - self.log.debug("Create a connection from a whitelisted wallet that rebroadcasts raw txs") + self.log.debug("Create a connection from a forcerelay peer that rebroadcasts raw txs") # A python mininode is needed to send the raw transaction directly. If a full node was used, it could only - # rebroadcast via the inv-getdata mechanism. However, even for whitelisted connections, a full node would + # rebroadcast via the inv-getdata mechanism. However, even for forcerelay connections, a full node would # currently not request a txid that is already in the mempool. self.restart_node(1, extra_args=["-whitelist=forcerelay@127.0.0.1"]) p2p_rebroadcast_wallet = self.nodes[1].add_p2p_connection(P2PDataStore()) @@ -134,7 +135,7 @@ class P2PPermissionsTests(BitcoinTestFramework): self.log.debug("Check that node[1] will send the tx to node[0] even though it is already in the mempool") connect_nodes(self.nodes[1], 0) - with self.nodes[1].assert_debug_log(["Force relaying tx {} from whitelisted peer=0".format(txid)]): + with self.nodes[1].assert_debug_log(["Force relaying tx {} from peer=0".format(txid)]): p2p_rebroadcast_wallet.send_txs_and_test([tx], self.nodes[1]) wait_until(lambda: txid in self.nodes[0].getrawmempool()) @@ -145,7 +146,7 @@ class P2PPermissionsTests(BitcoinTestFramework): [tx], self.nodes[1], success=False, - reject_reason='Not relaying non-mempool transaction {} from whitelisted peer=0'.format(txid), + reject_reason='Not relaying non-mempool transaction {} from forcerelay peer=0'.format(txid), ) def checkpermission(self, args, expectedPermissions, whitelisted): diff --git a/test/functional/p2p_ping.py b/test/functional/p2p_ping.py new file mode 100755 index 0000000000..e00af88cc4 --- /dev/null +++ b/test/functional/p2p_ping.py @@ -0,0 +1,123 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 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 ping message +""" + +import time + +from test_framework.messages import ( + msg_pong, +) +from test_framework.mininode import ( + P2PInterface, + wait_until, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + +PING_INTERVAL = 2 * 60 + + +class msg_pong_corrupt(msg_pong): + def serialize(self): + return b"" + + +class NodePongAdd1(P2PInterface): + def on_ping(self, message): + self.send_message(msg_pong(message.nonce + 1)) + + +class NodeNoPong(P2PInterface): + def on_ping(self, message): + pass + + +class PingPongTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + self.extra_args = [['-peertimeout=3']] + + def check_peer_info(self, *, pingtime, minping, pingwait): + stats = self.nodes[0].getpeerinfo()[0] + assert_equal(stats.pop('pingtime', None), pingtime) + assert_equal(stats.pop('minping', None), minping) + assert_equal(stats.pop('pingwait', None), pingwait) + + def mock_forward(self, delta): + self.mock_time += delta + self.nodes[0].setmocktime(self.mock_time) + + def run_test(self): + self.mock_time = int(time.time()) + self.mock_forward(0) + + self.log.info('Check that ping is sent after connection is established') + no_pong_node = self.nodes[0].add_p2p_connection(NodeNoPong()) + self.mock_forward(3) + assert no_pong_node.last_message.pop('ping').nonce != 0 + self.check_peer_info(pingtime=None, minping=None, pingwait=3) + + self.log.info('Reply without nonce cancels ping') + with self.nodes[0].assert_debug_log(['pong peer=0: Short payload']): + no_pong_node.send_and_ping(msg_pong_corrupt()) + self.check_peer_info(pingtime=None, minping=None, pingwait=None) + + self.log.info('Reply without ping') + with self.nodes[0].assert_debug_log([ + 'pong peer=0: Unsolicited pong without ping, 0 expected, 0 received, 8 bytes', + ]): + no_pong_node.send_and_ping(msg_pong()) + self.check_peer_info(pingtime=None, minping=None, pingwait=None) + + self.log.info('Reply with wrong nonce does not cancel ping') + assert 'ping' not in no_pong_node.last_message + with self.nodes[0].assert_debug_log(['pong peer=0: Nonce mismatch']): + # mock time PING_INTERVAL ahead to trigger node into sending a ping + self.mock_forward(PING_INTERVAL + 1) + wait_until(lambda: 'ping' in no_pong_node.last_message) + self.mock_forward(9) + # Send the wrong pong + no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce - 1)) + self.check_peer_info(pingtime=None, minping=None, pingwait=9) + + self.log.info('Reply with zero nonce does cancel ping') + with self.nodes[0].assert_debug_log(['pong peer=0: Nonce zero']): + no_pong_node.send_and_ping(msg_pong(0)) + self.check_peer_info(pingtime=None, minping=None, pingwait=None) + + self.log.info('Check that ping is properly reported on RPC') + assert 'ping' not in no_pong_node.last_message + # mock time PING_INTERVAL ahead to trigger node into sending a ping + self.mock_forward(PING_INTERVAL + 1) + wait_until(lambda: 'ping' in no_pong_node.last_message) + ping_delay = 29 + self.mock_forward(ping_delay) + wait_until(lambda: 'ping' in no_pong_node.last_message) + no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce)) + self.check_peer_info(pingtime=ping_delay, minping=ping_delay, pingwait=None) + + self.log.info('Check that minping is decreased after a fast roundtrip') + # mock time PING_INTERVAL ahead to trigger node into sending a ping + self.mock_forward(PING_INTERVAL + 1) + wait_until(lambda: 'ping' in no_pong_node.last_message) + ping_delay = 9 + self.mock_forward(ping_delay) + wait_until(lambda: 'ping' in no_pong_node.last_message) + no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce)) + self.check_peer_info(pingtime=ping_delay, minping=ping_delay, pingwait=None) + + self.log.info('Check that peer is disconnected after ping timeout') + assert 'ping' not in no_pong_node.last_message + self.nodes[0].ping() + wait_until(lambda: 'ping' in no_pong_node.last_message) + with self.nodes[0].assert_debug_log(['ping timeout: 1201.000000s']): + self.mock_forward(20 * 60 + 1) + time.sleep(4) # peertimeout + 1 + + +if __name__ == '__main__': + PingPongTest().main() diff --git a/test/functional/p2p_unrequested_blocks.py b/test/functional/p2p_unrequested_blocks.py index c323168848..71b0b0f63a 100755 --- a/test/functional/p2p_unrequested_blocks.py +++ b/test/functional/p2p_unrequested_blocks.py @@ -4,7 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test processing of unrequested blocks. -Setup: two nodes, node0+node1, not connected to each other. Node1 will have +Setup: two nodes, node0 + node1, not connected to each other. Node1 will have nMinimumChainWork set to 0x10, so it won't process low-work unrequested blocks. We have one P2PInterface connection to node0 called test_node, and one to node1 @@ -71,18 +71,10 @@ class AcceptBlockTest(BitcoinTestFramework): self.extra_args = [[], ["-minimumchainwork=0x10"]] def setup_network(self): - # Node0 will be used to test behavior of processing unrequested blocks - # from peers which are not whitelisted, while Node1 will be used for - # the whitelisted case. - # Node2 will be used for non-whitelisted peers to test the interaction - # with nMinimumChainWork. self.setup_nodes() def run_test(self): - # Setup the p2p connections - # test_node connects to node0 (not whitelisted) test_node = self.nodes[0].add_p2p_connection(P2PInterface()) - # min_work_node connects to node1 (whitelisted) min_work_node = self.nodes[1].add_p2p_connection(P2PInterface()) # 1. Have nodes mine a block (leave IBD) @@ -226,7 +218,7 @@ class AcceptBlockTest(BitcoinTestFramework): self.nodes[0].getblock(all_blocks[286].hash) assert_equal(self.nodes[0].getbestblockhash(), all_blocks[286].hash) assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, all_blocks[287].hash) - self.log.info("Successfully reorged to longer chain from non-whitelisted peer") + self.log.info("Successfully reorged to longer chain") # 8. Create a chain which is invalid at a height longer than the # current chain, but which has more blocks on top of that diff --git a/test/functional/rpc_getpeerinfo_banscore_deprecation.py b/test/functional/rpc_getpeerinfo_banscore_deprecation.py new file mode 100755 index 0000000000..b830248e1e --- /dev/null +++ b/test/functional/rpc_getpeerinfo_banscore_deprecation.py @@ -0,0 +1,24 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 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 deprecation of getpeerinfo RPC banscore field.""" + +from test_framework.test_framework import BitcoinTestFramework + + +class GetpeerinfoBanscoreDeprecationTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.extra_args = [[], ["-deprecatedrpc=banscore"]] + + def run_test(self): + self.log.info("Test getpeerinfo by default no longer returns a banscore field") + assert "banscore" not in self.nodes[0].getpeerinfo()[0].keys() + + self.log.info("Test getpeerinfo returns banscore with -deprecatedrpc=banscore") + assert "banscore" in self.nodes[1].getpeerinfo()[0].keys() + + +if __name__ == "__main__": + GetpeerinfoBanscoreDeprecationTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 2fa48006f4..867a9909a8 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -236,6 +236,7 @@ BASE_SCRIPTS = [ 'mempool_compatibility.py', 'rpc_deriveaddresses.py', 'rpc_deriveaddresses.py --usecli', + 'p2p_ping.py', 'rpc_scantxoutset.py', 'feature_logging.py', 'p2p_node_network_limited.py', @@ -243,6 +244,7 @@ BASE_SCRIPTS = [ 'feature_blocksdir.py', 'feature_config_args.py', 'rpc_getdescriptorinfo.py', + 'rpc_getpeerinfo_banscore_deprecation.py', 'rpc_help.py', 'feature_help.py', 'feature_shutdown.py', diff --git a/test/lint/README.md b/test/lint/README.md index 6b95cc3540..d15c061288 100644 --- a/test/lint/README.md +++ b/test/lint/README.md @@ -23,6 +23,12 @@ maintained: * for `src/crypto/ctaes`: https://github.com/bitcoin-core/ctaes.git (branch master) * for `src/crc32c`: https://github.com/google/crc32c.git (branch master) +To do so, add the upstream repository as remote: + +``` +git remote add --fetch secp256k1 https://github.com/bitcoin-core/secp256k1.git +``` + Usage: `git-subtree-check.sh DIR (COMMIT)` `COMMIT` may be omitted, in which case `HEAD` is used. diff --git a/test/lint/git-subtree-check.sh b/test/lint/git-subtree-check.sh index caa7affc63..5a0500df25 100755 --- a/test/lint/git-subtree-check.sh +++ b/test/lint/git-subtree-check.sh @@ -81,7 +81,7 @@ fi # get the tree in the subtree commit referred to if [ "d$(git cat-file -t $rev 2>/dev/null)" != dcommit ]; then - echo "subtree commit $rev unavailable: cannot compare" >&2 + echo "subtree commit $rev unavailable: cannot compare. Did you add and fetch the remote?" >&2 exit fi tree_subtree=$(git show -s --format="%T" $rev) |