diff options
Diffstat (limited to 'src')
74 files changed, 838 insertions, 671 deletions
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/addrdb.h b/src/addrdb.h index c6d4307d69..8410c3776c 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -17,13 +17,6 @@ class CSubNet; class CAddrMan; class CDataStream; -typedef enum BanReason -{ - BanReasonUnknown = 0, - BanReasonNodeMisbehaving = 1, - BanReasonManuallyAdded = 2 -} BanReason; - class CBanEntry { public: @@ -31,7 +24,6 @@ public: int nVersion; int64_t nCreateTime; int64_t nBanUntil; - uint8_t banReason; CBanEntry() { @@ -44,31 +36,17 @@ public: nCreateTime = nCreateTimeIn; } - explicit CBanEntry(int64_t n_create_time_in, BanReason ban_reason_in) : CBanEntry(n_create_time_in) + SERIALIZE_METHODS(CBanEntry, obj) { - banReason = ban_reason_in; + uint8_t ban_reason = 2; //! For backward compatibility + READWRITE(obj.nVersion, obj.nCreateTime, obj.nBanUntil, ban_reason); } - SERIALIZE_METHODS(CBanEntry, obj) { READWRITE(obj.nVersion, obj.nCreateTime, obj.nBanUntil, obj.banReason); } - void SetNull() { nVersion = CBanEntry::CURRENT_VERSION; nCreateTime = 0; nBanUntil = 0; - banReason = BanReasonUnknown; - } - - std::string banReasonToString() const - { - switch (banReason) { - case BanReasonNodeMisbehaving: - return "node misbehaving"; - case BanReasonManuallyAdded: - return "manually added"; - default: - return "unknown"; - } } }; diff --git a/src/banman.cpp b/src/banman.cpp index 422904bb33..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 @@ -68,28 +68,13 @@ void BanMan::ClearBanned() if (m_client_interface) m_client_interface->BannedListChanged(); } -int BanMan::IsBannedLevel(CNetAddr net_addr) +bool BanMan::IsDiscouraged(const CNetAddr& net_addr) { - // Returns the most severe level of banning that applies to this address. - // 0 - Not banned - // 1 - Automatic misbehavior ban - // 2 - Any other ban - int level = 0; - auto current_time = GetTime(); LOCK(m_cs_banned); - for (const auto& it : m_banned) { - CSubNet sub_net = it.first; - CBanEntry ban_entry = it.second; - - if (current_time < ban_entry.nBanUntil && sub_net.Match(net_addr)) { - if (ban_entry.banReason != BanReasonNodeMisbehaving) return 2; - level = 1; - } - } - return level; + return m_discouraged.contains(net_addr.GetAddrBytes()); } -bool BanMan::IsBanned(CNetAddr net_addr) +bool BanMan::IsBanned(const CNetAddr& net_addr) { auto current_time = GetTime(); LOCK(m_cs_banned); @@ -104,7 +89,7 @@ bool BanMan::IsBanned(CNetAddr net_addr) return false; } -bool BanMan::IsBanned(CSubNet sub_net) +bool BanMan::IsBanned(const CSubNet& sub_net) { auto current_time = GetTime(); LOCK(m_cs_banned); @@ -118,15 +103,21 @@ bool BanMan::IsBanned(CSubNet sub_net) return false; } -void BanMan::Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch) +void BanMan::Ban(const CNetAddr& net_addr, int64_t ban_time_offset, bool since_unix_epoch) { CSubNet sub_net(net_addr); - Ban(sub_net, ban_reason, ban_time_offset, since_unix_epoch); + Ban(sub_net, ban_time_offset, since_unix_epoch); +} + +void BanMan::Discourage(const CNetAddr& net_addr) +{ + LOCK(m_cs_banned); + m_discouraged.insert(net_addr.GetAddrBytes()); } -void BanMan::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch) +void BanMan::Ban(const CSubNet& sub_net, int64_t ban_time_offset, bool since_unix_epoch) { - CBanEntry ban_entry(GetTime(), ban_reason); + CBanEntry ban_entry(GetTime()); int64_t normalized_ban_time_offset = ban_time_offset; bool normalized_since_unix_epoch = since_unix_epoch; @@ -146,8 +137,8 @@ void BanMan::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ba } if (m_client_interface) m_client_interface->BannedListChanged(); - //store banlist to disk immediately if user requested ban - if (ban_reason == BanReasonManuallyAdded) DumpBanlist(); + //store banlist to disk immediately + DumpBanlist(); } bool BanMan::Unban(const CNetAddr& net_addr) diff --git a/src/banman.h b/src/banman.h index 6bea2e75e9..f6bfbd1e49 100644 --- a/src/banman.h +++ b/src/banman.h @@ -6,6 +6,7 @@ #define BITCOIN_BANMAN_H #include <addrdb.h> +#include <bloom.h> #include <fs.h> #include <net_types.h> // For banmap_t #include <sync.h> @@ -23,32 +24,55 @@ class CClientUIInterface; class CNetAddr; class CSubNet; -// Denial-of-service detection/prevention -// The idea is to detect peers that are behaving -// badly and disconnect/ban them, but do it in a -// one-coding-mistake-won't-shatter-the-entire-network -// way. -// IMPORTANT: There should be nothing I can give a -// node that it will forward on that will make that -// node's peers drop it. If there is, an attacker -// can isolate a node and/or try to split the network. -// Dropping a node for sending stuff that is invalid -// now but might be valid in a later version is also -// dangerous, because it can cause a network split -// between nodes running old code and nodes running -// new code. +// Banman manages two related but distinct concepts: +// +// 1. Banning. This is configured manually by the user, through the setban RPC. +// If an address or subnet is banned, we never accept incoming connections from +// it and never create outgoing connections to it. We won't gossip its address +// to other peers in addr messages. Banned addresses and subnets are stored to +// banlist.dat on shutdown and reloaded on startup. Banning can be used to +// prevent connections with spy nodes or other griefers. +// +// 2. Discouragement. If a peer misbehaves enough (see Misbehaving() in +// net_processing.cpp), we'll mark that address as discouraged. We still allow +// incoming connections from them, but they're preferred for eviction when +// we receive new incoming connections. We never make outgoing connections to +// them, and do not gossip their address to other peers. This is implemented as +// a bloom filter. We can (probabilistically) test for membership, but can't +// list all discouraged addresses or unmark them as discouraged. Discouragement +// can prevent our limited connection slots being used up by incompatible +// or broken peers. +// +// Neither banning nor discouragement are protections against denial-of-service +// attacks, since if an attacker has a way to waste our resources and we +// disconnect from them and ban that address, it's trivial for them to +// reconnect from another IP address. +// +// Attempting to automatically disconnect or ban any class of peer carries the +// risk of splitting the network. For example, if we banned/disconnected for a +// transaction that fails a policy check and a future version changes the +// policy check so the transaction is accepted, then that transaction could +// cause the network to split between old nodes and new nodes. class BanMan { public: ~BanMan(); BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time); - void Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset = 0, bool since_unix_epoch = false); - void Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset = 0, bool since_unix_epoch = false); + void Ban(const CNetAddr& net_addr, int64_t ban_time_offset = 0, bool since_unix_epoch = false); + void Ban(const CSubNet& sub_net, int64_t ban_time_offset = 0, bool since_unix_epoch = false); + void Discourage(const CNetAddr& net_addr); void ClearBanned(); - int IsBannedLevel(CNetAddr net_addr); - bool IsBanned(CNetAddr net_addr); - bool IsBanned(CSubNet sub_net); + + //! Return whether net_addr is banned + bool IsBanned(const CNetAddr& net_addr); + + //! Return whether sub_net is exactly banned + bool IsBanned(const CSubNet& sub_net); + + //! Return whether net_addr is discouraged. + bool IsDiscouraged(const CNetAddr& net_addr); + bool Unban(const CNetAddr& net_addr); bool Unban(const CSubNet& sub_net); void GetBanned(banmap_t& banmap); @@ -68,6 +92,7 @@ private: CClientUIInterface* m_client_interface = nullptr; CBanDB m_ban_db; const int64_t m_default_ban_time; + CRollingBloomFilter m_discouraged GUARDED_BY(m_cs_banned) {50000, 0.000001}; }; #endif 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 0427368e70..7b893a08d7 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); @@ -431,8 +431,7 @@ void SetupServerArgs(NodeContext& node) gArgs.AddArg("-addnode=<ip>", "Add a node to connect to and attempt to keep the connection open (see the `addnode` RPC command help for more info). This option can be specified multiple times to add multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); gArgs.AddArg("-asmap=<file>", strprintf("Specify asn mapping used for bucketing of the peers (default: %s). Relative paths will be prefixed by the net-specific datadir location.", DEFAULT_ASMAP_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - gArgs.AddArg("-banscore=<n>", strprintf("Threshold for disconnecting misbehaving peers (default: %u)", DEFAULT_BANSCORE_THRESHOLD), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - gArgs.AddArg("-bantime=<n>", strprintf("Number of seconds to keep misbehaving peers from reconnecting (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + gArgs.AddArg("-bantime=<n>", strprintf("Default duration (in seconds) of manually configured bans (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); gArgs.AddArg("-bind=<addr>", "Bind to given address and always listen on it. Use [host]:port notation for IPv6", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); gArgs.AddArg("-connect=<ip>", "Connect only to the specified node; -noconnect disables automatic connections (the rules for this peer are the same as for -addnode). This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); gArgs.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -446,7 +445,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 +468,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/interfaces/chain.h b/src/interfaces/chain.h index 65695707f7..bbeb0fa801 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -15,6 +15,7 @@ #include <string> #include <vector> +class ArgsManager; class CBlock; class CFeeRate; class CRPCCommand; @@ -322,7 +323,7 @@ std::unique_ptr<Chain> MakeChain(NodeContext& node); //! analysis, or fee estimation. These clients need to expose their own //! MakeXXXClient functions returning their implementations of the ChainClient //! interface. -std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, std::vector<std::string> wallet_filenames); +std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames); } // namespace interfaces diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 5b9a15f268..834a16ecf5 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -146,10 +146,10 @@ public: } return false; } - bool ban(const CNetAddr& net_addr, BanReason reason, int64_t ban_time_offset) override + bool ban(const CNetAddr& net_addr, int64_t ban_time_offset) override { if (m_context.banman) { - m_context.banman->Ban(net_addr, reason, ban_time_offset); + m_context.banman->Ban(net_addr, ban_time_offset); return true; } return false; diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 877a40568f..b88b5bc14e 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -122,7 +122,7 @@ public: virtual bool getBanned(banmap_t& banmap) = 0; //! Ban node. - virtual bool ban(const CNetAddr& net_addr, BanReason reason, int64_t ban_time_offset) = 0; + virtual bool ban(const CNetAddr& net_addr, int64_t ban_time_offset) = 0; //! Unban node. virtual bool unban(const CSubNet& ip) = 0; diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index f6806aed65..7fd24425cf 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -483,10 +483,11 @@ public: class WalletClientImpl : public ChainClient { public: - WalletClientImpl(Chain& chain, std::vector<std::string> wallet_filenames) + WalletClientImpl(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames) : m_wallet_filenames(std::move(wallet_filenames)) { m_context.chain = &chain; + m_context.args = &args; } void registerRpcs() override { @@ -499,7 +500,7 @@ public: } bool verify() override { return VerifyWallets(*m_context.chain, m_wallet_filenames); } bool load() override { return LoadWallets(*m_context.chain, m_wallet_filenames); } - void start(CScheduler& scheduler) override { return StartWallets(scheduler); } + void start(CScheduler& scheduler) override { return StartWallets(scheduler, *Assert(m_context.args)); } void flush() override { return FlushWallets(); } void stop() override { return StopWallets(); } void setMockTime(int64_t time) override { return SetMockTime(time); } @@ -514,7 +515,7 @@ public: ~WalletClientImpl() override { UnloadWallets(); } WalletContext m_context; - std::vector<std::string> m_wallet_filenames; + const std::vector<std::string> m_wallet_filenames; std::vector<std::unique_ptr<Handler>> m_rpc_handlers; std::list<CRPCCommand> m_rpc_commands; }; @@ -523,9 +524,9 @@ public: std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return wallet ? MakeUnique<WalletImpl>(wallet) : nullptr; } -std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, std::vector<std::string> wallet_filenames) +std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames) { - return MakeUnique<WalletClientImpl>(chain, std::move(wallet_filenames)); + return MakeUnique<WalletClientImpl>(chain, args, std::move(wallet_filenames)); } } // namespace interfaces 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 281232d801..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)); @@ -1010,17 +1012,24 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { // on all platforms. Set it again here just to be sure. SetSocketNoDelay(hSocket); - int bannedlevel = m_banman ? m_banman->IsBannedLevel(addr) : 0; - - // Don't accept connections from banned peers, but if our inbound slots aren't almost full, accept - // if the only banning reason was an automatic misbehavior ban. - if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0)) + // Don't accept connections from banned peers. + bool banned = m_banman->IsBanned(addr); + if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && banned) { LogPrint(BCLog::NET, "connection from %s dropped (banned)\n", addr.ToString()); CloseSocket(hSocket); return; } + // Only accept connections from discouraged peers if our inbound slots aren't (almost) full. + bool discouraged = m_banman->IsDiscouraged(addr); + if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && nInbound + 1 >= nMaxInbound && discouraged) + { + LogPrint(BCLog::NET, "connection from %s dropped (discouraged)\n", addr.ToString()); + CloseSocket(hSocket); + return; + } + if (nInbound >= nMaxInbound) { if (!AttemptToEvictConnection()) { @@ -1044,7 +1053,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { pnode->m_permissionFlags = permissionFlags; // If this flag is present, the user probably expect that RPC and QT report it as whitelisted (backward compatibility) pnode->m_legacyWhitelisted = legacyWhitelisted; - pnode->m_prefer_evict = bannedlevel > 0; + pnode->m_prefer_evict = discouraged; m_msgproc->InitializeNode(pnode); LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString()); @@ -1103,12 +1112,9 @@ void CConnman::DisconnectNodes() if (pnode->GetRefCount() <= 0) { bool fDelete = false; { - TRY_LOCK(pnode->cs_inventory, lockInv); - if (lockInv) { - TRY_LOCK(pnode->cs_vSend, lockSend); - if (lockSend) { - fDelete = true; - } + TRY_LOCK(pnode->cs_vSend, lockSend); + if (lockSend) { + fDelete = true; } } if (fDelete) { @@ -1154,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) @@ -2045,10 +2051,10 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai return; } if (!pszDest) { - if (IsLocal(addrConnect) || - FindNode(static_cast<CNetAddr>(addrConnect)) || (m_banman && m_banman->IsBanned(addrConnect)) || - FindNode(addrConnect.ToStringIPPort())) + bool banned_or_discouraged = m_banman && (m_banman->IsDiscouraged(addrConnect) || m_banman->IsBanned(addrConnect)); + if (IsLocal(addrConnect) || FindNode(static_cast<CNetAddr>(addrConnect)) || banned_or_discouraged || FindNode(addrConnect.ToStringIPPort())) { return; + } } else if (FindNode(std::string(pszDest))) return; @@ -2505,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); @@ -2639,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 @@ -803,7 +802,7 @@ public: // There is no final sorting before sending, as they are always sent immediately // and in the order requested. std::vector<uint256> vInventoryBlockToSend GUARDED_BY(cs_inventory); - RecursiveMutex cs_inventory; + Mutex cs_inventory; struct TxRelay { mutable RecursiveMutex cs_filter; @@ -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. @@ -982,18 +981,6 @@ public: } } - void PushBlockInventory(const uint256& hash) - { - LOCK(cs_inventory); - vInventoryBlockToSend.push_back(hash); - } - - void PushBlockHash(const uint256 &hash) - { - LOCK(cs_inventory); - vBlockHashesToAnnounce.push_back(hash); - } - void CloseSocketDisconnect(); void copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap); 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 e004067e75..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, - // Can't be banned for misbehavior - PF_NOBAN = (1U << 4), + // 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_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 e3053869ae..ad349cf1a7 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -38,7 +38,9 @@ static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60; /** Minimum time between orphan transactions expire time checks in seconds */ static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60; /** How long to cache transactions in mapRelay for normal relay */ -static constexpr std::chrono::seconds RELAY_TX_CACHE_TIME{15 * 60}; +static constexpr std::chrono::seconds RELAY_TX_CACHE_TIME = std::chrono::minutes{15}; +/** How long a transaction has to be in the mempool before it can unconditionally be relayed (even when not in mapRelay). */ +static constexpr std::chrono::seconds UNCONDITIONAL_RELAY_DELAY = std::chrono::minutes{2}; /** Headers download timeout expressed in microseconds * Timeout = base + per_header * (expected number of headers) */ static constexpr int64_t HEADERS_DOWNLOAD_TIMEOUT_BASE = 15 * 60 * 1000000; // 15 minutes @@ -63,8 +65,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,11 +119,20 @@ 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. +/** Maximum rate of inventory items to send per second. * Limits the impact of low-fee transaction floods. */ -static constexpr unsigned int INVENTORY_BROADCAST_MAX = 7 * INVENTORY_BROADCAST_INTERVAL; +static constexpr unsigned int INVENTORY_BROADCAST_PER_SECOND = 7; +/** Maximum number of inventory items to send per transmission. */ +static constexpr unsigned int INVENTORY_BROADCAST_MAX = INVENTORY_BROADCAST_PER_SECOND * INVENTORY_BROADCAST_INTERVAL; +/** The number of most recently announced transactions a peer can request. */ +static constexpr unsigned int INVENTORY_MAX_RECENT_RELAY = 3500; +/** Verify that INVENTORY_MAX_RECENT_RELAY is enough to cache everything typically + * relayed before unconditional relay from the mempool kicks in. This is only a + * lower bound, and it should be larger to account for higher inv rate to outbound + * peers, and random variations in the broadcast mechanism. */ +static_assert(INVENTORY_MAX_RECENT_RELAY >= INVENTORY_BROADCAST_PER_SECOND * UNCONDITIONAL_RELAY_DELAY / std::chrono::seconds{1}, "INVENTORY_RELAY_MAX too low"); /** Average delay between feefilter broadcasts in seconds. */ static constexpr unsigned int AVG_FEEFILTER_BROADCAST_INTERVAL = 10 * 60; /** Maximum feefilter broadcast delay after significant change. */ @@ -249,8 +260,8 @@ struct CNodeState { bool fCurrentlyConnected; //! Accumulated misbehaviour score for this peer. int nMisbehavior; - //! Whether this peer should be disconnected and banned (unless whitelisted). - bool fShouldBan; + //! 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; //! The best known block we know this peer has announced. @@ -395,13 +406,16 @@ struct CNodeState { //! Whether this peer is a manual connection bool m_is_manual_connection; + //! A rolling bloom filter of all announced tx CInvs to this peer. + CRollingBloomFilter m_recently_announced_invs = CRollingBloomFilter{INVENTORY_MAX_RECENT_RELAY, 0.000001}; + CNodeState(CAddress addrIn, std::string addrNameIn, bool is_inbound, bool is_manual) : address(addrIn), name(std::move(addrNameIn)), m_is_inbound(is_inbound), m_is_manual_connection (is_manual) { fCurrentlyConnected = false; nMisbehavior = 0; - fShouldBan = false; + m_should_discourage = false; pindexBestKnownBlock = nullptr; hashLastUnknownBlock.SetNull(); pindexLastCommonBlock = nullptr; @@ -422,6 +436,7 @@ struct CNodeState { fSupportsDesiredCmpctVersion = false; m_chain_sync = { 0, nullptr, false, false }; m_last_block_announcement = 0; + m_recently_announced_invs.reset(); } }; @@ -1016,7 +1031,8 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) } /** - * Mark a misbehaving peer to be banned depending upon the value of `-banscore`. + * Increment peer's misbehavior score. If the new value >= DISCOURAGEMENT_THRESHOLD, mark the node + * to be discouraged, meaning the peer might be disconnected and added to the discouragement filter. */ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { @@ -1028,18 +1044,17 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV return; state->nMisbehavior += howmuch; - int banscore = gArgs.GetArg("-banscore", DEFAULT_BANSCORE_THRESHOLD); std::string message_prefixed = message.empty() ? "" : (": " + message); - if (state->nMisbehavior >= banscore && state->nMisbehavior - howmuch < banscore) + if (state->nMisbehavior >= DISCOURAGEMENT_THRESHOLD && state->nMisbehavior - howmuch < DISCOURAGEMENT_THRESHOLD) { - LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) BAN THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); - state->fShouldBan = true; + LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); + state->m_should_discourage = true; } else LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d)%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); } /** - * Potentially ban a node based on the contents of a BlockValidationState object + * Potentially mark a node discouraged based on the contents of a BlockValidationState object * * @param[in] via_compact_block this bool is passed in because net_processing should * punish peers differently depending on whether the data was provided in a compact @@ -1069,7 +1084,7 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s break; } - // Ban outbound (but not inbound) peers if on an invalid chain. + // Discourage outbound (but not inbound) peers if on an invalid chain. // Exempt HB compact block peers and manual connections. if (!via_compact_block && !node_state->m_is_inbound && !node_state->m_is_manual_connection) { Misbehaving(nodeid, 100, message); @@ -1104,7 +1119,7 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s } /** - * Potentially ban a node based on the contents of a TxValidationState object + * Potentially disconnect and discourage a node based on the contents of a TxValidationState object * * @return Returns true if the peer was punished (probably disconnected) */ @@ -1325,9 +1340,10 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB } // Relay inventory, but don't relay old inventory during initial block download. connman->ForEachNode([nNewHeight, &vHashes](CNode* pnode) { + LOCK(pnode->cs_inventory); if (nNewHeight > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : 0)) { for (const uint256& hash : reverse_iterate(vHashes)) { - pnode->PushBlockHash(hash); + pnode->vBlockHashesToAnnounce.push_back(hash); } } }); @@ -1336,7 +1352,7 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB } /** - * Handle invalid block rejection and consequent peer banning, maintain which + * Handle invalid block rejection and consequent peer discouragement, maintain which * peers announce compact blocks. */ void PeerLogicValidation::BlockChecked(const CBlock& block, const BlockValidationState& state) { @@ -1437,7 +1453,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) { @@ -1508,7 +1524,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()); @@ -1604,7 +1620,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c // Trigger the peer node to send a getblocks request for the next batch of inventory if (inv.hash == pfrom.hashContinue) { - // Bypass PushBlockInventory, this must send even if redundant, + // Send immediately. This must send even if redundant, // and we want it right after the last block so they don't // wait for other stuff first. std::vector<CInv> vInv; @@ -1616,30 +1632,28 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c } //! Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). -CTransactionRef static FindTxForGetData(CNode& peer, const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds longlived_mempool_time) LOCKS_EXCLUDED(cs_main) +CTransactionRef static FindTxForGetData(const CNode& peer, const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) LOCKS_EXCLUDED(cs_main) { - // Check if the requested transaction is so recent that we're just - // about to announce it to the peer; if so, they certainly shouldn't - // know we already have it. - { - LOCK(peer.m_tx_relay->cs_tx_inventory); - if (peer.m_tx_relay->setInventoryTxToSend.count(txid)) return {}; + auto txinfo = mempool.info(txid); + if (txinfo.tx) { + // If a TX could have been INVed in reply to a MEMPOOL request, + // or is older than UNCONDITIONAL_RELAY_DELAY, permit the request + // unconditionally. + if ((mempool_req.count() && txinfo.m_time <= mempool_req) || txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY) { + return std::move(txinfo.tx); + } } { LOCK(cs_main); - // Look up transaction in relay pool - auto mi = mapRelay.find(txid); - if (mi != mapRelay.end()) return mi->second; - } - auto txinfo = mempool.info(txid); - if (txinfo.tx) { - // To protect privacy, do not answer getdata using the mempool when - // that TX couldn't have been INVed in reply to a MEMPOOL request, - // or when it's too recent to have expired from mapRelay. - if ((mempool_req.count() && txinfo.m_time <= mempool_req) || txinfo.m_time <= longlived_mempool_time) { - return txinfo.tx; + // Otherwise, the transaction must have been announced recently. + if (State(peer.GetId())->m_recently_announced_invs.contains(txid)) { + // If it was, it can be relayed from either the mempool... + if (txinfo.tx) return std::move(txinfo.tx); + // ... or the relay pool. + auto mi = mapRelay.find(txid); + if (mi != mapRelay.end()) return mi->second; } } @@ -1654,8 +1668,7 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm std::vector<CInv> vNotFound; const CNetMsgMaker msgMaker(pfrom.GetSendVersion()); - // mempool entries added before this time have likely expired from mapRelay - const std::chrono::seconds longlived_mempool_time = GetTime<std::chrono::seconds>() - RELAY_TX_CACHE_TIME; + const std::chrono::seconds now = GetTime<std::chrono::seconds>(); // Get last mempool request time const std::chrono::seconds mempool_req = pfrom.m_tx_relay != nullptr ? pfrom.m_tx_relay->m_last_mempool_req.load() : std::chrono::seconds::min(); @@ -1676,11 +1689,22 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm continue; } - CTransactionRef tx = FindTxForGetData(pfrom, inv.hash, mempool_req, longlived_mempool_time); + CTransactionRef tx = FindTxForGetData(pfrom, inv.hash, mempool_req, now); if (tx) { int nSendFlags = (inv.type == MSG_TX ? SERIALIZE_TRANSACTION_NO_WITNESS : 0); connman->PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *tx)); mempool.RemoveUnbroadcastTx(inv.hash); + // As we're going to send tx, make sure its unconfirmed parents are made requestable. + for (const auto& txin : tx->vin) { + auto txinfo = mempool.info(txin.prevout.hash); + if (txinfo.tx && txinfo.m_time > now - UNCONDITIONAL_RELAY_DELAY) { + // Relaying a transaction with a recent but unconfirmed parent. + if (WITH_LOCK(pfrom.m_tx_relay->cs_tx_inventory, return !pfrom.m_tx_relay->filterInventoryKnown.contains(txin.prevout.hash))) { + LOCK(cs_main); + State(pfrom.GetId())->m_recently_announced_invs.insert(txin.prevout.hash); + } + } + } } else { vNotFound.push_back(inv); } @@ -1894,8 +1918,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 @@ -2205,7 +2229,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, @@ -2348,11 +2372,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); } @@ -2442,9 +2463,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; } @@ -2473,7 +2491,8 @@ void ProcessMessage( if (addr.nTime <= 100000000 || addr.nTime > nNow + 10 * 60) addr.nTime = nNow - 5 * 24 * 60 * 60; pfrom.AddAddressKnown(addr); - if (banman->IsBanned(addr)) continue; // Do not process banned addresses beyond remembering we received them + if (banman->IsDiscouraged(addr)) continue; // Do not process banned/discouraged addresses beyond remembering we received them + if (banman->IsBanned(addr)) continue; bool fReachable = IsReachable(addr); if (addr.nTime > nSince && !pfrom.fGetAddr && vAddr.size() <= 10 && addr.IsRoutable()) { @@ -2535,9 +2554,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); @@ -2663,7 +2683,7 @@ void ProcessMessage( LogPrint(BCLog::NET, " getblocks stopping, pruned or too old block at %d %s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); break; } - pfrom.PushBlockInventory(pindex->GetBlockHash()); + WITH_LOCK(pfrom.cs_inventory, pfrom.vInventoryBlockToSend.push_back(pindex->GetBlockHash())); if (--nLimit <= 0) { // When this block is requested, we'll send an inv that'll @@ -2737,7 +2757,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; } @@ -2891,14 +2911,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); } } @@ -3048,7 +3068,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) { @@ -3108,14 +3128,14 @@ 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 // relayed before full validation (see BIP 152), so we don't want to disconnect // the peer if the header turns out to be for an invalid block. // Note that if a peer tries to build on an invalid chain, that - // will be detected and the peer will be banned. + // will be detected and the peer will be disconnected/discouraged. return ProcessHeadersMessage(pfrom, connman, chainman, mempool, {cmpctblock.header}, chainparams, /*via_compact_block=*/true); } @@ -3181,7 +3201,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) { @@ -3201,7 +3221,7 @@ void ProcessMessage( // 3. the block is otherwise invalid (eg invalid coinbase, // block is too big, too many legacy sigops, etc). // So if CheckBlock failed, #3 is the only possibility. - // Under BIP 152, we don't DoS-ban unless proof of work is + // Under BIP 152, we don't discourage the peer unless proof of work is // invalid (we don't require all the stateless checks to have // been run). This is handled below, so just treat this as // though the block was successfully read, and rely on the @@ -3326,7 +3346,7 @@ void ProcessMessage( std::vector<CAddress> vAddr = connman->GetAddresses(); FastRandomContext insecure_rand; for (const CAddress &addr : vAddr) { - if (!banman->IsBanned(addr)) { + if (!banman->IsDiscouraged(addr) && !banman->IsBanned(addr)) { pfrom.PushAddress(addr, insecure_rand); } } @@ -3383,7 +3403,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; @@ -3397,11 +3417,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"; @@ -3561,25 +3581,26 @@ void ProcessMessage( return; } -bool PeerLogicValidation::CheckIfBanned(CNode& pnode) +bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode& pnode) { AssertLockHeld(cs_main); CNodeState &state = *State(pnode.GetId()); - if (state.fShouldBan) { - state.fShouldBan = false; - if (pnode.HasPermission(PF_NOBAN)) + if (state.m_should_discourage) { + state.m_should_discourage = false; + if (pnode.HasPermission(PF_NOBAN)) { LogPrintf("Warning: not punishing whitelisted peer %s!\n", pnode.addr.ToString()); - else if (pnode.m_manual_connection) + } else if (pnode.m_manual_connection) { LogPrintf("Warning: not punishing manually-connected peer %s!\n", pnode.addr.ToString()); - else if (pnode.addr.IsLocal()) { - // Disconnect but don't ban _this_ local node - LogPrintf("Warning: disconnecting but not banning local peer %s!\n", pnode.addr.ToString()); + } else if (pnode.addr.IsLocal()) { + // Disconnect but don't discourage this local node + LogPrintf("Warning: disconnecting but not discouraging local peer %s!\n", pnode.addr.ToString()); pnode.fDisconnect = true; } else { - // Disconnect and ban all nodes sharing the address + // Disconnect and discourage all nodes sharing the address + LogPrintf("Disconnecting and discouraging peer %s!\n", pnode.addr.ToString()); if (m_banman) { - m_banman->Ban(pnode.addr, BanReasonNodeMisbehaving); + m_banman->Discourage(pnode.addr); } connman->DisconnectNode(pnode.addr); } @@ -3679,7 +3700,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter } LOCK(cs_main); - CheckIfBanned(*pfrom); + MaybeDiscourageAndDisconnect(*pfrom); return fMoreWork; } @@ -3857,7 +3878,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; } @@ -3867,7 +3888,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)); @@ -3882,7 +3903,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) if (!lockMain) return true; - if (CheckIfBanned(*pto)) return true; + if (MaybeDiscourageAndDisconnect(*pto)) return true; CNodeState &state = *State(pto->GetId()); @@ -4080,7 +4101,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // If the peer's chain has this block, don't inv it back. if (!PeerHasHeader(&state, pindex)) { - pto->PushBlockInventory(hashToAnnounce); + pto->vInventoryBlockToSend.push_back(hashToAnnounce); LogPrint(BCLog::NET, "%s: sending inv peer=%d hash=%s\n", __func__, pto->GetId(), hashToAnnounce.ToString()); } @@ -4151,6 +4172,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) if (!pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; } pto->m_tx_relay->filterInventoryKnown.insert(hash); + // Responses to MEMPOOL requests bypass the m_recently_announced_invs filter. vInv.push_back(inv); if (vInv.size() == MAX_INV_SZ) { connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); @@ -4204,6 +4226,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) } if (pto->m_tx_relay->pfilter && !pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; // Send + State(pto->GetId())->m_recently_announced_invs.insert(hash); vInv.push_back(CInv(MSG_TX, hash)); nRelayedTransactions++; { @@ -4261,9 +4284,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 @@ -4273,7 +4296,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/net_processing.h b/src/net_processing.h index 19beca0cc4..3479ef6865 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -23,6 +23,8 @@ static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100; static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100; static const bool DEFAULT_PEERBLOOMFILTERS = false; static const bool DEFAULT_PEERBLOCKFILTERS = false; +/** Threshold for marking a node to be discouraged, e.g. disconnected and added to the discouragement filter. */ +static const int DISCOURAGEMENT_THRESHOLD{100}; class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface { private: @@ -31,7 +33,7 @@ private: ChainstateManager& m_chainman; CTxMemPool& m_mempool; - bool CheckIfBanned(CNode& pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool MaybeDiscourageAndDisconnect(CNode& pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main); public: PeerLogicValidation(CConnman* connman, BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool); 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 e640c07d32..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> @@ -90,6 +90,7 @@ class CNetAddr uint32_t GetMappedAS(const std::vector<bool> &asmap) const; std::vector<unsigned char> GetGroup(const std::vector<bool> &asmap) const; + std::vector<unsigned char> GetAddrBytes() const { return {std::begin(ip), std::end(ip)}; } int GetReachabilityFrom(const CNetAddr *paddrPartner = nullptr) const; explicit CNetAddr(const struct in6_addr& pipv6Addr, const uint32_t scope = 0); @@ -142,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); @@ -156,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/node/coinstats.cpp b/src/node/coinstats.cpp index e3c4c828b6..fb46ea1731 100644 --- a/src/node/coinstats.cpp +++ b/src/node/coinstats.cpp @@ -8,13 +8,23 @@ #include <coins.h> #include <hash.h> #include <serialize.h> -#include <validation.h> #include <uint256.h> #include <util/system.h> +#include <validation.h> #include <map> -static void ApplyStats(CCoinsStats &stats, CHashWriter& ss, const uint256& hash, const std::map<uint32_t, Coin>& outputs) +static uint64_t GetBogoSize(const CScript& scriptPubKey) +{ + return 32 /* txid */ + + 4 /* vout index */ + + 4 /* height + coinbase */ + + 8 /* amount */ + + 2 /* scriptPubKey len */ + + scriptPubKey.size() /* scriptPubKey */; +} + +static void ApplyStats(CCoinsStats& stats, CHashWriter& ss, const uint256& hash, const std::map<uint32_t, Coin>& outputs) { assert(!outputs.empty()); ss << hash; @@ -26,26 +36,38 @@ static void ApplyStats(CCoinsStats &stats, CHashWriter& ss, const uint256& hash, ss << VARINT_MODE(output.second.out.nValue, VarIntMode::NONNEGATIVE_SIGNED); stats.nTransactionOutputs++; stats.nTotalAmount += output.second.out.nValue; - stats.nBogoSize += 32 /* txid */ + 4 /* vout index */ + 4 /* height + coinbase */ + 8 /* amount */ + - 2 /* scriptPubKey len */ + output.second.out.scriptPubKey.size() /* scriptPubKey */; + stats.nBogoSize += GetBogoSize(output.second.out.scriptPubKey); } ss << VARINT(0u); } +static void ApplyStats(CCoinsStats& stats, std::nullptr_t, const uint256& hash, const std::map<uint32_t, Coin>& outputs) +{ + assert(!outputs.empty()); + stats.nTransactions++; + for (const auto& output : outputs) { + stats.nTransactionOutputs++; + stats.nTotalAmount += output.second.out.nValue; + stats.nBogoSize += GetBogoSize(output.second.out.scriptPubKey); + } +} + //! Calculate statistics about the unspent transaction output set -bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const std::function<void()>& interruption_point) +template <typename T> +static bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, const std::function<void()>& interruption_point) { stats = CCoinsStats(); std::unique_ptr<CCoinsViewCursor> pcursor(view->Cursor()); assert(pcursor); - CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION); stats.hashBlock = pcursor->GetBestBlock(); { LOCK(cs_main); stats.nHeight = LookupBlockIndex(stats.hashBlock)->nHeight; } - ss << stats.hashBlock; + + PrepareHash(hash_obj, stats); + uint256 prevkey; std::map<uint32_t, Coin> outputs; while (pcursor->Valid()) { @@ -54,7 +76,7 @@ bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const std::function<void Coin coin; if (pcursor->GetKey(key) && pcursor->GetValue(coin)) { if (!outputs.empty() && key.hash != prevkey) { - ApplyStats(stats, ss, prevkey, outputs); + ApplyStats(stats, hash_obj, prevkey, outputs); outputs.clear(); } prevkey = key.hash; @@ -66,9 +88,38 @@ bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const std::function<void pcursor->Next(); } if (!outputs.empty()) { - ApplyStats(stats, ss, prevkey, outputs); + ApplyStats(stats, hash_obj, prevkey, outputs); } - stats.hashSerialized = ss.GetHash(); + + FinalizeHash(hash_obj, stats); + stats.nDiskSize = view->EstimateSize(); return true; } + +bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, CoinStatsHashType hash_type, const std::function<void()>& interruption_point) +{ + switch (hash_type) { + case(CoinStatsHashType::HASH_SERIALIZED): { + CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION); + return GetUTXOStats(view, stats, ss, interruption_point); + } + case(CoinStatsHashType::NONE): { + return GetUTXOStats(view, stats, nullptr, interruption_point); + } + } // no default case, so the compiler can warn about missing cases + assert(false); +} + +// The legacy hash serializes the hashBlock +static void PrepareHash(CHashWriter& ss, CCoinsStats& stats) +{ + ss << stats.hashBlock; +} +static void PrepareHash(std::nullptr_t, CCoinsStats& stats) {} + +static void FinalizeHash(CHashWriter& ss, CCoinsStats& stats) +{ + stats.hashSerialized = ss.GetHash(); +} +static void FinalizeHash(std::nullptr_t, CCoinsStats& stats) {} diff --git a/src/node/coinstats.h b/src/node/coinstats.h index d9cdaa3036..2a7441c10e 100644 --- a/src/node/coinstats.h +++ b/src/node/coinstats.h @@ -14,6 +14,11 @@ class CCoinsView; +enum class CoinStatsHashType { + HASH_SERIALIZED, + NONE, +}; + struct CCoinsStats { int nHeight{0}; @@ -30,6 +35,6 @@ struct CCoinsStats }; //! Calculate statistics about the unspent transaction output set -bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const std::function<void()>& interruption_point = {}); +bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const CoinStatsHashType hash_type, const std::function<void()>& interruption_point = {}); #endif // BITCOIN_NODE_COINSTATS_H diff --git a/src/protocol.h b/src/protocol.h index 985f44640b..9ab63a30fb 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -371,7 +371,13 @@ public: READWRITE(nVersion); } if ((s.GetType() & SER_DISK) || - (nVersion >= CADDR_TIME_VERSION && !(s.GetType() & SER_GETHASH))) { + (nVersion != INIT_PROTO_VERSION && !(s.GetType() & SER_GETHASH))) { + // The only time we serialize a CAddress object without nTime is in + // the initial VERSION messages which contain two CAddress records. + // At that point, the serialization version is INIT_PROTO_VERSION. + // After the version handshake, serialization version is >= + // MIN_PEER_PROTO_VERSION and all ADDR messages are serialized with + // nTime. READWRITE(obj.nTime); } READWRITE(Using<CustomUintFormatter<8>>(obj.nServices)); diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 3c5e8d0d07..5d6efeeeda 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -112,6 +112,8 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty Q_EMIT consoleShown(rpcConsole); } + modalOverlay = new ModalOverlay(enableWallet, this->centralWidget()); + // Accept D&D of URIs setAcceptDrops(true); @@ -201,7 +203,6 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty openOptionsDialogWithTab(OptionsDialog::TAB_NETWORK); }); - modalOverlay = new ModalOverlay(enableWallet, this->centralWidget()); connect(labelBlocksIcon, &GUIUtil::ClickableLabel::clicked, this, &BitcoinGUI::showModalOverlay); connect(progressBar, &GUIUtil::ClickableProgressBar::clicked, this, &BitcoinGUI::showModalOverlay); #ifdef ENABLE_WALLET @@ -238,6 +239,7 @@ BitcoinGUI::~BitcoinGUI() void BitcoinGUI::createActions() { QActionGroup *tabGroup = new QActionGroup(this); + connect(modalOverlay, &ModalOverlay::triggered, tabGroup, &QActionGroup::setEnabled); overviewAction = new QAction(platformStyle->SingleColorIcon(":/icons/overview"), tr("&Overview"), this); overviewAction->setStatusTip(tr("Show general overview of wallet")); @@ -683,6 +685,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/modaloverlay.cpp b/src/qt/modaloverlay.cpp index 0ba1beaf3e..8070aa627c 100644 --- a/src/qt/modaloverlay.cpp +++ b/src/qt/modaloverlay.cpp @@ -171,6 +171,8 @@ void ModalOverlay::showHide(bool hide, bool userRequested) if ( (layerIsVisible && !hide) || (!layerIsVisible && hide) || (!hide && userClosed && !userRequested)) return; + Q_EMIT triggered(hide); + if (!isVisible() && !hide) setVisible(true); diff --git a/src/qt/modaloverlay.h b/src/qt/modaloverlay.h index 1d84046d3d..7b07777641 100644 --- a/src/qt/modaloverlay.h +++ b/src/qt/modaloverlay.h @@ -25,16 +25,20 @@ public: explicit ModalOverlay(bool enable_wallet, QWidget *parent); ~ModalOverlay(); -public Q_SLOTS: void tipUpdate(int count, const QDateTime& blockDate, double nVerificationProgress); void setKnownBestHeight(int count, const QDateTime& blockDate); - void toggleVisibility(); // will show or hide the modal layer void showHide(bool hide = false, bool userRequested = false); - void closeClicked(); bool isLayerVisible() const { return layerIsVisible; } +public Q_SLOTS: + void toggleVisibility(); + void closeClicked(); + +Q_SIGNALS: + void triggered(bool hidden); + protected: bool eventFilter(QObject * obj, QEvent * ev) override; bool event(QEvent* ev) override; 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 dafd517ca8..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); @@ -1218,7 +1219,7 @@ void RPCConsole::banSelectedNode(int bantime) // Find possible nodes, ban it and clear the selected node const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow); if (stats) { - m_node.ban(stats->nodeStats.addr, BanReasonManuallyAdded, bantime); + m_node.ban(stats->nodeStats.addr, bantime); m_node.disconnectByAddress(stats->nodeStats.addr); } } diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index f88d57c716..443e2d047d 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -62,9 +62,10 @@ void AppTests::appTests() } #endif - BasicTestingSetup test{CBaseChainParams::REGTEST}; // Create a temp data directory to backup the gui settings to - ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference - LogInstance().DisconnectTestLogger(); + fs::create_directories([] { + BasicTestingSetup test{CBaseChainParams::REGTEST}; // Create a temp data directory to backup the gui settings to + return GetDataDir() / "blocks"; + }()); m_app.parameterSetup(); m_app.createOptionsModel(true /* reset settings */); @@ -80,6 +81,7 @@ void AppTests::appTests() m_app.exec(); // Reset global state to avoid interfering with later tests. + LogInstance().DisconnectTestLogger(); AbortShutdown(); UnloadBlockIndex(); WITH_LOCK(::cs_main, g_chainman.Reset()); diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index aefdcd2716..12efca2503 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -40,7 +40,7 @@ Q_IMPORT_PLUGIN(QCocoaIntegrationPlugin); const std::function<void(const std::string&)> G_TEST_LOG_FUN{}; // This is all you need to run all the tests -int main(int argc, char *argv[]) +int main(int argc, char* argv[]) { // Initialize persistent globals with the testing setup state for sanity. // E.g. -datadir in gArgs is set to a temp directory dummy value (instead @@ -70,6 +70,8 @@ int main(int argc, char *argv[]) BitcoinApplication app(*node); app.setApplicationName("Bitcoin-Qt-test"); + node->setupServerArgs(); // Make gArgs available in the NodeContext + node->context()->args->ClearArgs(); // Clear added args again AppTests app_tests(app); if (QTest::qExec(&app_tests) != 0) { fInvalid = true; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 630a8b463f..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"); } @@ -975,7 +973,9 @@ static UniValue gettxoutsetinfo(const JSONRPCRequest& request) RPCHelpMan{"gettxoutsetinfo", "\nReturns statistics about the unspent transaction output set.\n" "Note this call may take some time.\n", - {}, + { + {"hash_type", RPCArg::Type::STR, /* default */ "hash_serialized_2", "Which UTXO set hash should be calculated. Options: 'hash_serialized_2' (the legacy algorithm), 'none'."}, + }, RPCResult{ RPCResult::Type::OBJ, "", "", { @@ -984,7 +984,7 @@ static UniValue gettxoutsetinfo(const JSONRPCRequest& request) {RPCResult::Type::NUM, "transactions", "The number of transactions with unspent outputs"}, {RPCResult::Type::NUM, "txouts", "The number of unspent transaction outputs"}, {RPCResult::Type::NUM, "bogosize", "A meaningless metric for UTXO set size"}, - {RPCResult::Type::STR_HEX, "hash_serialized_2", "The serialized hash"}, + {RPCResult::Type::STR_HEX, "hash_serialized_2", "The serialized hash (only present if 'hash_serialized_2' hash_type is chosen)"}, {RPCResult::Type::NUM, "disk_size", "The estimated size of the chainstate on disk"}, {RPCResult::Type::STR_AMOUNT, "total_amount", "The total amount"}, }}, @@ -999,14 +999,18 @@ static UniValue gettxoutsetinfo(const JSONRPCRequest& request) CCoinsStats stats; ::ChainstateActive().ForceFlushStateToDisk(); + const CoinStatsHashType hash_type = ParseHashType(request.params[0], CoinStatsHashType::HASH_SERIALIZED); + CCoinsView* coins_view = WITH_LOCK(cs_main, return &ChainstateActive().CoinsDB()); - if (GetUTXOStats(coins_view, stats, RpcInterruptionPoint)) { + if (GetUTXOStats(coins_view, stats, hash_type, RpcInterruptionPoint)) { ret.pushKV("height", (int64_t)stats.nHeight); ret.pushKV("bestblock", stats.hashBlock.GetHex()); ret.pushKV("transactions", (int64_t)stats.nTransactions); ret.pushKV("txouts", (int64_t)stats.nTransactionOutputs); ret.pushKV("bogosize", (int64_t)stats.nBogoSize); - ret.pushKV("hash_serialized_2", stats.hashSerialized.GetHex()); + if (hash_type == CoinStatsHashType::HASH_SERIALIZED) { + ret.pushKV("hash_serialized_2", stats.hashSerialized.GetHex()); + } ret.pushKV("disk_size", stats.nDiskSize); ret.pushKV("total_amount", ValueFromAmount(stats.nTotalAmount)); } else { @@ -2317,7 +2321,7 @@ UniValue dumptxoutset(const JSONRPCRequest& request) ::ChainstateActive().ForceFlushStateToDisk(); - if (!GetUTXOStats(&::ChainstateActive().CoinsDB(), stats, RpcInterruptionPoint)) { + if (!GetUTXOStats(&::ChainstateActive().CoinsDB(), stats, CoinStatsHashType::NONE, RpcInterruptionPoint)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to read UTXO set"); } @@ -2378,7 +2382,7 @@ static const CRPCCommand commands[] = { "blockchain", "getmempoolinfo", &getmempoolinfo, {} }, { "blockchain", "getrawmempool", &getrawmempool, {"verbose"} }, { "blockchain", "gettxout", &gettxout, {"txid","n","include_mempool"} }, - { "blockchain", "gettxoutsetinfo", &gettxoutsetinfo, {} }, + { "blockchain", "gettxoutsetinfo", &gettxoutsetinfo, {"hash_type"} }, { "blockchain", "pruneblockchain", &pruneblockchain, {"height"} }, { "blockchain", "savemempool", &savemempool, {} }, { "blockchain", "verifychain", &verifychain, {"checklevel","nblocks"} }, diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index df1e0fe623..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); @@ -614,12 +617,12 @@ static UniValue setban(const JSONRPCRequest& request) absolute = true; if (isSubnet) { - node.banman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute); + node.banman->Ban(subNet, banTime, absolute); if (node.connman) { node.connman->DisconnectNode(subNet); } } else { - node.banman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute); + node.banman->Ban(netAddr, banTime, absolute); if (node.connman) { node.connman->DisconnectNode(netAddr); } @@ -628,7 +631,7 @@ static UniValue setban(const JSONRPCRequest& request) else if(strCommand == "remove") { if (!( isSubnet ? node.banman->Unban(subNet) : node.banman->Unban(netAddr) )) { - throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Unban failed. Requested address/subnet was not previously banned."); + throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Unban failed. Requested address/subnet was not previously manually banned."); } } return NullUniValue; @@ -637,7 +640,7 @@ static UniValue setban(const JSONRPCRequest& request) static UniValue listbanned(const JSONRPCRequest& request) { RPCHelpMan{"listbanned", - "\nList all banned IPs/Subnets.\n", + "\nList all manually banned IPs/Subnets.\n", {}, RPCResult{RPCResult::Type::ARR, "", "", { @@ -646,7 +649,6 @@ static UniValue listbanned(const JSONRPCRequest& request) {RPCResult::Type::STR, "address", ""}, {RPCResult::Type::NUM_TIME, "banned_until", ""}, {RPCResult::Type::NUM_TIME, "ban_created", ""}, - {RPCResult::Type::STR, "ban_reason", ""}, }}, }}, RPCExamples{ @@ -671,7 +673,6 @@ static UniValue listbanned(const JSONRPCRequest& request) rec.pushKV("address", entry.first.ToString()); rec.pushKV("banned_until", banEntry.nBanUntil); rec.pushKV("ban_created", banEntry.nCreateTime); - rec.pushKV("ban_reason", banEntry.banReasonToString()); bannedAddresses.push_back(rec); } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 54ea352a72..ca73c699c9 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -113,6 +113,23 @@ std::vector<unsigned char> ParseHexO(const UniValue& o, std::string strKey) return ParseHexV(find_value(o, strKey), strKey); } +CoinStatsHashType ParseHashType(const UniValue& param, const CoinStatsHashType default_type) +{ + if (param.isNull()) { + return default_type; + } else { + std::string hash_type_input = param.get_str(); + + if (hash_type_input == "hash_serialized_2") { + return CoinStatsHashType::HASH_SERIALIZED; + } else if (hash_type_input == "none") { + return CoinStatsHashType::NONE; + } else { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%d is not a valid hash_type", hash_type_input)); + } + } +} + std::string HelpExampleCli(const std::string& methodname, const std::string& args) { return "> bitcoin-cli " + methodname + " " + args + "\n"; diff --git a/src/rpc/util.h b/src/rpc/util.h index 53dce2c397..96dd1ea74a 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_RPC_UTIL_H #define BITCOIN_RPC_UTIL_H +#include <node/coinstats.h> #include <node/transaction.h> #include <outputtype.h> #include <protocol.h> @@ -77,6 +78,8 @@ extern uint256 ParseHashO(const UniValue& o, std::string strKey); extern std::vector<unsigned char> ParseHexV(const UniValue& v, std::string strName); extern std::vector<unsigned char> ParseHexO(const UniValue& o, std::string strKey); +CoinStatsHashType ParseHashType(const UniValue& param, const CoinStatsHashType default_type); + extern CAmount AmountFromValue(const UniValue& value); extern std::string HelpExampleCli(const std::string& methodname, const std::string& args); extern std::string HelpExampleRpc(const std::string& methodname, const std::string& args); 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/data/script_tests.json b/src/test/data/script_tests.json index c01ef307b7..724789bbf9 100644 --- a/src/test/data/script_tests.json +++ b/src/test/data/script_tests.json @@ -678,7 +678,7 @@ ["0 0x02 0x0000 0", "CHECKMULTISIGVERIFY 1", "", "OK"], ["While not really correctly DER encoded, the empty signature is allowed by"], -["STRICTENC to provide a compact way to provide a delibrately invalid signature."], +["STRICTENC to provide a compact way to provide a deliberately invalid signature."], ["0", "0x21 0x02865c40293a680cb9c020e7b1e106d8c1916d3cef99aa431a56d253e69256dac0 CHECKSIG NOT", "STRICTENC", "OK"], ["0 0", "1 0x21 0x02865c40293a680cb9c020e7b1e106d8c1916d3cef99aa431a56d253e69256dac0 1 CHECKMULTISIG NOT", "STRICTENC", "OK"], diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 348b170536..3d84fa855f 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -232,14 +232,14 @@ BOOST_AUTO_TEST_CASE(DoS_banning) dummyNode1.fSuccessfullyConnected = true; { LOCK(cs_main); - Misbehaving(dummyNode1.GetId(), 100); // Should get banned + Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged } { LOCK2(cs_main, dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); } - BOOST_CHECK(banman->IsBanned(addr1)); - BOOST_CHECK(!banman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned + BOOST_CHECK(banman->IsDiscouraged(addr1)); + BOOST_CHECK(!banman->IsDiscouraged(ip(0xa0b0c001|0x0000ff00))); // Different IP, not discouraged CAddress addr2(ip(0xa0b0c002), NODE_NONE); CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, 1, CAddress(), "", true); @@ -255,8 +255,8 @@ BOOST_AUTO_TEST_CASE(DoS_banning) LOCK2(cs_main, dummyNode2.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode2)); } - BOOST_CHECK(!banman->IsBanned(addr2)); // 2 not banned yet... - BOOST_CHECK(banman->IsBanned(addr1)); // ... but 1 still should be + BOOST_CHECK(!banman->IsDiscouraged(addr2)); // 2 not discouraged yet... + BOOST_CHECK(banman->IsDiscouraged(addr1)); // ... but 1 still should be { LOCK(cs_main); Misbehaving(dummyNode2.GetId(), 50); @@ -265,7 +265,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) LOCK2(cs_main, dummyNode2.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode2)); } - BOOST_CHECK(banman->IsBanned(addr2)); + BOOST_CHECK(banman->IsDiscouraged(addr2)); bool dummy; peerLogic->FinalizeNode(dummyNode1.GetId(), dummy); @@ -279,7 +279,6 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) auto peerLogic = MakeUnique<PeerLogicValidation>(connman.get(), banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool); banman->ClearBanned(); - gArgs.ForceSetArg("-banscore", "111"); // because 11 is my favorite number CAddress addr1(ip(0xa0b0c001), NODE_NONE); CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 3, 1, CAddress(), "", true); dummyNode1.SetSendVersion(PROTOCOL_VERSION); @@ -288,13 +287,13 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) dummyNode1.fSuccessfullyConnected = true; { LOCK(cs_main); - Misbehaving(dummyNode1.GetId(), 100); + Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD - 11); } { LOCK2(cs_main, dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); } - BOOST_CHECK(!banman->IsBanned(addr1)); + BOOST_CHECK(!banman->IsDiscouraged(addr1)); { LOCK(cs_main); Misbehaving(dummyNode1.GetId(), 10); @@ -303,7 +302,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) LOCK2(cs_main, dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); } - BOOST_CHECK(!banman->IsBanned(addr1)); + BOOST_CHECK(!banman->IsDiscouraged(addr1)); { LOCK(cs_main); Misbehaving(dummyNode1.GetId(), 1); @@ -312,8 +311,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore) LOCK2(cs_main, dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); } - BOOST_CHECK(banman->IsBanned(addr1)); - gArgs.ForceSetArg("-banscore", ToString(DEFAULT_BANSCORE_THRESHOLD)); + BOOST_CHECK(banman->IsDiscouraged(addr1)); bool dummy; peerLogic->FinalizeNode(dummyNode1.GetId(), dummy); @@ -338,19 +336,13 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) { LOCK(cs_main); - Misbehaving(dummyNode.GetId(), 100); + Misbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD); } { LOCK2(cs_main, dummyNode.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode)); } - BOOST_CHECK(banman->IsBanned(addr)); - - SetMockTime(nStartTime+60*60); - BOOST_CHECK(banman->IsBanned(addr)); - - SetMockTime(nStartTime+60*60*24+1); - BOOST_CHECK(!banman->IsBanned(addr)); + BOOST_CHECK(banman->IsDiscouraged(addr)); bool dummy; peerLogic->FinalizeNode(dummyNode.GetId(), dummy); diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 5d7065dafb..20132d5782 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -135,7 +135,7 @@ void DoCheck(const std::string& prv, const std::string& pub, int flags, const st // When the descriptor is hardened, evaluate with access to the private keys inside. const FlatSigningProvider& key_provider = (flags & HARDENED) ? keys_priv : keys_pub; - // Evaluate the descriptor selected by `t` in poisition `i`. + // Evaluate the descriptor selected by `t` in position `i`. FlatSigningProvider script_provider, script_provider_cached; std::vector<CScript> spks, spks_cached; DescriptorCache desc_cache; diff --git a/src/test/fuzz/addrdb.cpp b/src/test/fuzz/addrdb.cpp index 524cea83fe..ad6461650f 100644 --- a/src/test/fuzz/addrdb.cpp +++ b/src/test/fuzz/addrdb.cpp @@ -18,18 +18,11 @@ void test_one_input(const std::vector<uint8_t>& buffer) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); const CBanEntry ban_entry = [&] { - switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 3)) { + switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 2)) { case 0: return CBanEntry{fuzzed_data_provider.ConsumeIntegral<int64_t>()}; break; - case 1: - return CBanEntry{fuzzed_data_provider.ConsumeIntegral<int64_t>(), fuzzed_data_provider.PickValueInArray<BanReason>({ - BanReason::BanReasonUnknown, - BanReason::BanReasonNodeMisbehaving, - BanReason::BanReasonManuallyAdded, - })}; - break; - case 2: { + case 1: { const std::optional<CBanEntry> ban_entry = ConsumeDeserializable<CBanEntry>(fuzzed_data_provider); if (ban_entry) { return *ban_entry; @@ -39,5 +32,4 @@ void test_one_input(const std::vector<uint8_t>& buffer) } return CBanEntry{}; }(); - assert(!ban_entry.banReasonToString().empty()); } 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/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 52dd62a145..c186bef7ae 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -278,7 +278,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) CCoinsStats stats; bool expected_code_path = false; try { - (void)GetUTXOStats(&coins_view_cache, stats); + (void)GetUTXOStats(&coins_view_cache, stats, CoinStatsHashType::HASH_SERIALIZED); } catch (const std::logic_error&) { expected_code_path = true; } 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..acadf151c5 100644 --- a/src/validation.h +++ b/src/validation.h @@ -74,7 +74,6 @@ static const int64_t DEFAULT_MAX_TIP_AGE = 24 * 60 * 60; static const bool DEFAULT_CHECKPOINTS_ENABLED = true; static const bool DEFAULT_TXINDEX = false; static const char* const DEFAULT_BLOCKFILTERINDEX = "0"; -static const unsigned int DEFAULT_BANSCORE_THRESHOLD = 100; /** Default for -persistmempool */ static const bool DEFAULT_PERSIST_MEMPOOL = true; /** Default for using fee filter */ @@ -845,7 +844,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/version.h b/src/version.h index d932b512d4..e5d1f5a7f9 100644 --- a/src/version.h +++ b/src/version.h @@ -14,15 +14,8 @@ static const int PROTOCOL_VERSION = 70015; //! initial proto version, to be increased after version/verack negotiation static const int INIT_PROTO_VERSION = 209; -//! In this version, 'getheaders' was introduced. -static const int GETHEADERS_VERSION = 31800; - //! disconnect from peers older than this proto version -static const int MIN_PEER_PROTO_VERSION = GETHEADERS_VERSION; - -//! nTime field added to CAddress, starting with this version; -//! if possible, avoid requesting addresses nodes older than this -static const int CADDR_TIME_VERSION = 31402; +static const int MIN_PEER_PROTO_VERSION = 31800; //! BIP 0031, pong message, is enabled for all versions AFTER this one static const int BIP0031_VERSION = 60000; diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 24954f4a02..54a9aa34e0 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -292,11 +292,10 @@ BerkeleyBatch::SafeDbt::operator Dbt*() return &m_dbt; } -bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, bilingual_str& errorStr) +bool BerkeleyDatabase::Verify(bilingual_str& errorStr) { - std::string walletFile; - std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, walletFile); fs::path walletDir = env->Directory(); + fs::path file_path = walletDir / strFile; LogPrintf("Using BerkeleyDB version %s\n", BerkeleyDatabaseVersion()); LogPrintf("Using wallet %s\n", file_path.string()); @@ -306,19 +305,10 @@ bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, bilingual_str& return false; } - return true; -} - -bool BerkeleyBatch::VerifyDatabaseFile(const fs::path& file_path, bilingual_str& errorStr) -{ - std::string walletFile; - std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, walletFile); - fs::path walletDir = env->Directory(); - - if (fs::exists(walletDir / walletFile)) + if (fs::exists(file_path)) { - if (!env->Verify(walletFile)) { - errorStr = strprintf(_("%s corrupt. Try using the wallet tool bitcoin-wallet to salvage or restoring a backup."), walletFile); + if (!env->Verify(strFile)) { + errorStr = strprintf(_("%s corrupt. Try using the wallet tool bitcoin-wallet to salvage or restoring a backup."), file_path); return false; } } @@ -495,13 +485,11 @@ void BerkeleyEnvironment::ReloadDbEnv() Open(true); } -bool BerkeleyBatch::Rewrite(BerkeleyDatabase& database, const char* pszSkip) +bool BerkeleyDatabase::Rewrite(const char* pszSkip) { - if (database.IsDummy()) { + if (IsDummy()) { return true; } - BerkeleyEnvironment *env = database.env.get(); - const std::string& strFile = database.strFile; while (true) { { LOCK(cs_db); @@ -515,7 +503,7 @@ bool BerkeleyBatch::Rewrite(BerkeleyDatabase& database, const char* pszSkip) LogPrintf("BerkeleyBatch::Rewrite: Rewriting %s...\n", strFile); std::string strFileRes = strFile + ".rewrite"; { // surround usage of db with extra {} - BerkeleyBatch db(database, "r"); + BerkeleyBatch db(*this, "r"); std::unique_ptr<Db> pdbCopy = MakeUnique<Db>(env->dbenv.get(), 0); int ret = pdbCopy->open(nullptr, // Txn pointer @@ -625,51 +613,35 @@ void BerkeleyEnvironment::Flush(bool fShutdown) } } -bool BerkeleyBatch::PeriodicFlush(BerkeleyDatabase& database) +bool BerkeleyDatabase::PeriodicFlush() { - if (database.IsDummy()) { - return true; - } - bool ret = false; - BerkeleyEnvironment *env = database.env.get(); - const std::string& strFile = database.strFile; + // There's nothing to do for dummy databases. Return true. + if (IsDummy()) return true; + + // Don't flush if we can't acquire the lock. TRY_LOCK(cs_db, lockDb); - if (lockDb) - { - // Don't do this if any databases are in use - int nRefCount = 0; - std::map<std::string, int>::iterator mit = env->mapFileUseCount.begin(); - while (mit != env->mapFileUseCount.end()) - { - nRefCount += (*mit).second; - mit++; - } + if (!lockDb) return false; - if (nRefCount == 0) - { - std::map<std::string, int>::iterator mi = env->mapFileUseCount.find(strFile); - if (mi != env->mapFileUseCount.end()) - { - LogPrint(BCLog::WALLETDB, "Flushing %s\n", strFile); - int64_t nStart = GetTimeMillis(); + // Don't flush if any databases are in use + for (const auto& use_count : env->mapFileUseCount) { + if (use_count.second > 0) return false; + } - // Flush wallet file so it's self contained - env->CloseDb(strFile); - env->CheckpointLSN(strFile); + // Don't flush if there haven't been any batch writes for this database. + auto it = env->mapFileUseCount.find(strFile); + if (it == env->mapFileUseCount.end()) return false; - env->mapFileUseCount.erase(mi++); - LogPrint(BCLog::WALLETDB, "Flushed %s %dms\n", strFile, GetTimeMillis() - nStart); - ret = true; - } - } - } + LogPrint(BCLog::WALLETDB, "Flushing %s\n", strFile); + int64_t nStart = GetTimeMillis(); - return ret; -} + // Flush wallet file so it's self contained + env->CloseDb(strFile); + env->CheckpointLSN(strFile); + env->mapFileUseCount.erase(it); -bool BerkeleyDatabase::Rewrite(const char* pszSkip) -{ - return BerkeleyBatch::Rewrite(*this, pszSkip); + LogPrint(BCLog::WALLETDB, "Flushed %s %dms\n", strFile, GetTimeMillis() - nStart); + + return true; } bool BerkeleyDatabase::Backup(const std::string& strDest) const @@ -815,15 +787,13 @@ std::string BerkeleyDatabaseVersion() return DbEnv::version(nullptr, nullptr, nullptr); } -bool BerkeleyBatch::ReadKey(CDataStream& key, CDataStream& value) +bool BerkeleyBatch::ReadKey(CDataStream&& key, CDataStream& value) { if (!pdb) return false; - // Key SafeDbt datKey(key.data(), key.size()); - // Read SafeDbt datValue; int ret = pdb->get(activeTxn, datKey, datValue, 0); if (ret == 0 && datValue.get_data() != nullptr) { @@ -833,48 +803,41 @@ bool BerkeleyBatch::ReadKey(CDataStream& key, CDataStream& value) return false; } -bool BerkeleyBatch::WriteKey(CDataStream& key, CDataStream& value, bool overwrite) +bool BerkeleyBatch::WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite) { if (!pdb) return true; if (fReadOnly) assert(!"Write called on database in read-only mode"); - // Key SafeDbt datKey(key.data(), key.size()); - // Value SafeDbt datValue(value.data(), value.size()); - // Write int ret = pdb->put(activeTxn, datKey, datValue, (overwrite ? 0 : DB_NOOVERWRITE)); return (ret == 0); } -bool BerkeleyBatch::EraseKey(CDataStream& key) +bool BerkeleyBatch::EraseKey(CDataStream&& key) { if (!pdb) return false; if (fReadOnly) assert(!"Erase called on database in read-only mode"); - // Key SafeDbt datKey(key.data(), key.size()); - // Erase int ret = pdb->del(activeTxn, datKey, 0); return (ret == 0 || ret == DB_NOTFOUND); } -bool BerkeleyBatch::HasKey(CDataStream& key) +bool BerkeleyBatch::HasKey(CDataStream&& key) { if (!pdb) return false; - // Key SafeDbt datKey(key.data(), key.size()); - // Exists int ret = pdb->exists(activeTxn, datKey, 0); return ret == 0; } diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index ee9cfa46b1..9cc42ddc06 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -90,7 +90,7 @@ public: /** Get BerkeleyEnvironment and database filename given a wallet path. */ std::shared_ptr<BerkeleyEnvironment> GetWalletEnv(const fs::path& wallet_path, std::string& database_filename); -/** Return wheter a BDB wallet database is currently loaded. */ +/** Return whether a BDB wallet database is currently loaded. */ bool IsBDBWalletLoaded(const fs::path& wallet_path); /** An instance of this class represents one database. @@ -131,6 +131,9 @@ public: /** Make sure all changes are flushed to disk. */ void Flush(bool shutdown); + /* flush the wallet passively (TRY_LOCK) + ideal to be called periodically */ + bool PeriodicFlush(); void IncrementUpdateCounter(); @@ -141,6 +144,9 @@ public: unsigned int nLastFlushed; int64_t nLastWalletUpdate; + /** Verifies the environment and database file */ + bool Verify(bilingual_str& error); + /** * Pointer to shared database environment. * @@ -189,10 +195,10 @@ class BerkeleyBatch }; private: - bool ReadKey(CDataStream& key, CDataStream& value); - bool WriteKey(CDataStream& key, CDataStream& value, bool overwrite=true); - bool EraseKey(CDataStream& key); - bool HasKey(CDataStream& key); + bool ReadKey(CDataStream&& key, CDataStream& value); + bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true); + bool EraseKey(CDataStream&& key); + bool HasKey(CDataStream&& key); protected: Db* pdb; @@ -213,76 +219,55 @@ public: void Flush(); void Close(); - /* flush the wallet passively (TRY_LOCK) - ideal to be called periodically */ - static bool PeriodicFlush(BerkeleyDatabase& database); - /* verifies the database environment */ - static bool VerifyEnvironment(const fs::path& file_path, bilingual_str& errorStr); - /* verifies the database file */ - static bool VerifyDatabaseFile(const fs::path& file_path, bilingual_str& errorStr); - template <typename K, typename T> bool Read(const K& key, T& value) { - // Key CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; CDataStream ssValue(SER_DISK, CLIENT_VERSION); - bool success = false; - bool ret = ReadKey(ssKey, ssValue); - if (ret) { - // Unserialize value - try { - ssValue >> value; - success = true; - } catch (const std::exception&) { - // In this case success remains 'false' - } + if (!ReadKey(std::move(ssKey), ssValue)) return false; + try { + ssValue >> value; + return true; + } catch (const std::exception&) { + return false; } - return ret && success; } template <typename K, typename T> bool Write(const K& key, const T& value, bool fOverwrite = true) { - // Key CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - // Value CDataStream ssValue(SER_DISK, CLIENT_VERSION); ssValue.reserve(10000); ssValue << value; - // Write - return WriteKey(ssKey, ssValue, fOverwrite); + return WriteKey(std::move(ssKey), std::move(ssValue), fOverwrite); } template <typename K> bool Erase(const K& key) { - // Key CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - // Erase - return EraseKey(ssKey); + return EraseKey(std::move(ssKey)); } template <typename K> bool Exists(const K& key) { - // Key CDataStream ssKey(SER_DISK, CLIENT_VERSION); ssKey.reserve(1000); ssKey << key; - // Exists - return HasKey(ssKey); + return HasKey(std::move(ssKey)); } bool StartCursor(); @@ -291,8 +276,6 @@ public: bool TxnBegin(); bool TxnCommit(); bool TxnAbort(); - - bool static Rewrite(BerkeleyDatabase& database, const char* pszSkip = nullptr); }; std::string BerkeleyDatabaseVersion(); diff --git a/src/wallet/context.h b/src/wallet/context.h index 3c8fdd1c59..a83591154f 100644 --- a/src/wallet/context.h +++ b/src/wallet/context.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_WALLET_CONTEXT_H #define BITCOIN_WALLET_CONTEXT_H +class ArgsManager; namespace interfaces { class Chain; } // namespace interfaces @@ -21,6 +22,7 @@ class Chain; //! behavior. struct WalletContext { interfaces::Chain* chain{nullptr}; + ArgsManager* args{nullptr}; //! Declare default constructor and destructor that are not inline, so code //! instantiating the WalletContext struct doesn't need to #include class diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index f173b5e62b..781920755c 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -9,6 +9,7 @@ #include <node/context.h> #include <node/ui_interface.h> #include <outputtype.h> +#include <util/check.h> #include <util/moneystr.h> #include <util/system.h> #include <util/translation.h> @@ -16,9 +17,9 @@ #include <wallet/wallet.h> #include <walletinitinterface.h> -class WalletInit : public WalletInitInterface { +class WalletInit : public WalletInitInterface +{ public: - //! Was the wallet component compiled in. bool HasWalletSupport() const override {return true;} @@ -112,10 +113,11 @@ bool WalletInit::ParameterInteraction() const void WalletInit::Construct(NodeContext& node) const { - if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { + ArgsManager& args = *Assert(node.args); + if (args.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { LogPrintf("Wallet disabled!\n"); return; } - gArgs.SoftSetArg("-wallet", ""); - node.chain_clients.emplace_back(interfaces::MakeWalletClient(*node.chain, gArgs.GetArgs("-wallet"))); + args.SoftSetArg("-wallet", ""); + node.chain_clients.emplace_back(interfaces::MakeWalletClient(*node.chain, args, args.GetArgs("-wallet"))); } diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 8df3e78215..c2818a41e7 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -11,6 +11,7 @@ #include <util/system.h> #include <util/translation.h> #include <wallet/wallet.h> +#include <wallet/walletdb.h> bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files) { @@ -82,14 +83,16 @@ bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& walle } } -void StartWallets(CScheduler& scheduler) +void StartWallets(CScheduler& scheduler, const ArgsManager& args) { for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) { pwallet->postInitProcess(); } // Schedule periodic wallet flushes and tx rebroadcasts - scheduler.scheduleEvery(MaybeCompactWalletDB, std::chrono::milliseconds{500}); + if (args.GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) { + scheduler.scheduleEvery(MaybeCompactWalletDB, std::chrono::milliseconds{500}); + } scheduler.scheduleEvery(MaybeResendWalletTxs, std::chrono::milliseconds{1000}); } diff --git a/src/wallet/load.h b/src/wallet/load.h index e24b1f2e69..ff4f5b4b23 100644 --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -9,6 +9,7 @@ #include <string> #include <vector> +class ArgsManager; class CScheduler; namespace interfaces { @@ -22,7 +23,7 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files); //! Complete startup of wallets. -void StartWallets(CScheduler& scheduler); +void StartWallets(CScheduler& scheduler, const ArgsManager& args); //! Flush all wallets in preparation for shutdown. void FlushWallets(); 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/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 55114a17d7..9d334063c4 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -359,36 +359,54 @@ static UniValue setlabel(const JSONRPCRequest& request) return NullUniValue; } +void ParseRecipients(const UniValue& address_amounts, const UniValue& subtract_fee_outputs, std::vector<CRecipient> &recipients) { + std::set<CTxDestination> destinations; + int i = 0; + for (const std::string& address: address_amounts.getKeys()) { + CTxDestination dest = DecodeDestination(address); + if (!IsValidDestination(dest)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Bitcoin address: ") + address); + } -static CTransactionRef SendMoney(CWallet* const pwallet, const CTxDestination& address, CAmount nValue, bool fSubtractFeeFromAmount, const CCoinControl& coin_control, mapValue_t mapValue) -{ - CAmount curBalance = pwallet->GetBalance(0, coin_control.m_avoid_address_reuse).m_mine_trusted; + if (destinations.count(dest)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated address: ") + address); + } + destinations.insert(dest); - // Check amount - if (nValue <= 0) - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid amount"); + CScript script_pub_key = GetScriptForDestination(dest); + CAmount amount = AmountFromValue(address_amounts[i++]); - if (nValue > curBalance) - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Insufficient funds"); + bool subtract_fee = false; + for (unsigned int idx = 0; idx < subtract_fee_outputs.size(); idx++) { + const UniValue& addr = subtract_fee_outputs[idx]; + if (addr.get_str() == address) { + subtract_fee = true; + } + } - // Parse Bitcoin address - CScript scriptPubKey = GetScriptForDestination(address); + CRecipient recipient = {script_pub_key, amount, subtract_fee}; + recipients.push_back(recipient); + } +} + +UniValue SendMoney(CWallet* const pwallet, const CCoinControl &coin_control, std::vector<CRecipient> &recipients, mapValue_t map_value) +{ + EnsureWalletIsUnlocked(pwallet); - // Create and send the transaction + // Shuffle recipient list + std::shuffle(recipients.begin(), recipients.end(), FastRandomContext()); + + // Send CAmount nFeeRequired = 0; - bilingual_str error; - std::vector<CRecipient> vecSend; int nChangePosRet = -1; - CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount}; - vecSend.push_back(recipient); + bilingual_str error; CTransactionRef tx; - if (!pwallet->CreateTransaction(vecSend, tx, nFeeRequired, nChangePosRet, error, coin_control)) { - if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance) - error = strprintf(Untranslated("Error: This transaction requires a transaction fee of at least %s"), FormatMoney(nFeeRequired)); - throw JSONRPCError(RPC_WALLET_ERROR, error.original); + bool fCreated = pwallet->CreateTransaction(recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, !pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); + if (!fCreated) { + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original); } - pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */); - return tx; + pwallet->CommitTransaction(tx, std::move(map_value), {} /* orderForm */); + return tx->GetHash().GetHex(); } static UniValue sendtoaddress(const JSONRPCRequest& request) @@ -436,16 +454,6 @@ static UniValue sendtoaddress(const JSONRPCRequest& request) LOCK(pwallet->cs_wallet); - CTxDestination dest = DecodeDestination(request.params[0].get_str()); - if (!IsValidDestination(dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address"); - } - - // Amount - CAmount nAmount = AmountFromValue(request.params[1]); - if (nAmount <= 0) - throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount for send"); - // Wallet comments mapValue_t mapValue; if (!request.params[2].isNull() && !request.params[2].get_str().empty()) @@ -471,8 +479,18 @@ static UniValue sendtoaddress(const JSONRPCRequest& request) EnsureWalletIsUnlocked(pwallet); - CTransactionRef tx = SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, coin_control, std::move(mapValue)); - return tx->GetHash().GetHex(); + UniValue address_amounts(UniValue::VOBJ); + const std::string address = request.params[0].get_str(); + address_amounts.pushKV(address, request.params[1]); + UniValue subtractFeeFromAmount(UniValue::VARR); + if (fSubtractFeeFromAmount) { + subtractFeeFromAmount.push_back(address); + } + + std::vector<CRecipient> recipients; + ParseRecipients(address_amounts, subtractFeeFromAmount, recipients); + + return SendMoney(pwallet, coin_control, recipients, mapValue); } static UniValue listaddressgroupings(const JSONRPCRequest& request) @@ -860,52 +878,10 @@ static UniValue sendmany(const JSONRPCRequest& request) SetFeeEstimateMode(pwallet, coin_control, request.params[7], request.params[6]); - std::set<CTxDestination> destinations; - std::vector<CRecipient> vecSend; + std::vector<CRecipient> recipients; + ParseRecipients(sendTo, subtractFeeFromAmount, recipients); - std::vector<std::string> keys = sendTo.getKeys(); - for (const std::string& name_ : keys) { - CTxDestination dest = DecodeDestination(name_); - if (!IsValidDestination(dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Bitcoin address: ") + name_); - } - - if (destinations.count(dest)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated address: ") + name_); - } - destinations.insert(dest); - - CScript scriptPubKey = GetScriptForDestination(dest); - CAmount nAmount = AmountFromValue(sendTo[name_]); - if (nAmount <= 0) - throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount for send"); - - bool fSubtractFeeFromAmount = false; - for (unsigned int idx = 0; idx < subtractFeeFromAmount.size(); idx++) { - const UniValue& addr = subtractFeeFromAmount[idx]; - if (addr.get_str() == name_) - fSubtractFeeFromAmount = true; - } - - CRecipient recipient = {scriptPubKey, nAmount, fSubtractFeeFromAmount}; - vecSend.push_back(recipient); - } - - EnsureWalletIsUnlocked(pwallet); - - // Shuffle recipient list - std::shuffle(vecSend.begin(), vecSend.end(), FastRandomContext()); - - // Send - CAmount nFeeRequired = 0; - int nChangePosRet = -1; - bilingual_str error; - CTransactionRef tx; - bool fCreated = pwallet->CreateTransaction(vecSend, tx, nFeeRequired, nChangePosRet, error, coin_control); - if (!fCreated) - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original); - pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */); - return tx->GetHash().GetHex(); + return SendMoney(pwallet, coin_control, recipients, std::move(mapValue)); } static UniValue addmultisigaddress(const JSONRPCRequest& request) @@ -3141,7 +3117,7 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) CAmount fee; int change_position; CCoinControl coin_control; - // Automatically select (additional) coins. Can be overriden by options.add_inputs. + // Automatically select (additional) coins. Can be overridden by options.add_inputs. coin_control.m_add_inputs = true; FundTransaction(pwallet, tx, fee, change_position, request.params[1], coin_control); @@ -4075,7 +4051,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); CCoinControl coin_control; // Automatically select coins, unless at least one is manually selected. Can - // be overriden by options.add_inputs. + // be overridden by options.add_inputs. coin_control.m_add_inputs = rawTx.vin.size() == 0; FundTransaction(pwallet, rawTx, fee, change_position, request.params[3], coin_control); diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index d42950ee42..e6e62332c0 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -20,6 +20,11 @@ bool RecoverDatabaseFile(const fs::path& file_path) std::string filename; std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, filename); + if (!env->Open(true /* retry */)) { + tfm::format(std::cerr, "Error initializing wallet database environment %s!", env->Directory()); + return false; + } + // Recovery procedure: // move wallet file to walletfilename.timestamp.bak // Call Salvage with fAggressive=true to 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/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index 797a0d634f..35bd965673 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -3,13 +3,14 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <fs.h> +#include <util/check.h> #include <util/system.h> #include <wallet/test/init_test_fixture.h> -InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName): BasicTestingSetup(chainName) +InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName) : BasicTestingSetup(chainName) { - m_chain_client = MakeWalletClient(*m_chain, {}); + m_chain_client = MakeWalletClient(*m_chain, *Assert(m_node.args), {}); std::string sep; sep += fs::path::preferred_separator; diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index 6c32868b1e..99d7cfe921 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -10,17 +10,18 @@ #include <interfaces/chain.h> #include <interfaces/wallet.h> #include <node/context.h> +#include <util/check.h> #include <wallet/wallet.h> #include <memory> /** Testing setup and teardown for wallet. */ -struct WalletTestingSetup: public TestingSetup { +struct WalletTestingSetup : public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node); - std::unique_ptr<interfaces::ChainClient> m_chain_client = interfaces::MakeWalletClient(*m_chain, {}); + std::unique_ptr<interfaces::ChainClient> m_chain_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args), {}); CWallet m_wallet; std::unique_ptr<interfaces::Handler> m_chain_notifications_handler; }; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 5c565a3d38..9cc847b2d0 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), 1u); + + 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), 0u); + } + + TestUnloadWallet(std::move(wallet)); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 235b269805..8eec00993f 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 non-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); } @@ -3732,15 +3743,11 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b std::unique_ptr<WalletDatabase> database = CreateWalletDatabase(wallet_path); try { - if (!WalletBatch::VerifyEnvironment(wallet_path, error_string)) { - return false; - } + return database->Verify(error_string); } catch (const fs::filesystem_error& e) { error_string = Untranslated(strprintf("Error loading wallet %s. %s", location.GetName(), fsbridge::get_filesystem_error_message(e))); return false; } - - return WalletBatch::VerifyDatabaseFile(wallet_path, error_string); } std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector<bilingual_str>& warnings, uint64_t wallet_creation_flags) @@ -3800,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)) { @@ -4421,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; @@ -4434,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 ba5087e2e1..1e5c4da6d8 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; } @@ -592,9 +592,6 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, ssValue >> ser_xpub; CExtPubKey xpub; xpub.Decode(ser_xpub.data()); - if (wss.m_descriptor_caches.count(desc_id)) { - wss.m_descriptor_caches[desc_id] = DescriptorCache(); - } if (parent) { wss.m_descriptor_caches[desc_id].CacheParentExtPubKey(key_exp_index, xpub); } else { @@ -752,10 +749,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 @@ -952,9 +949,6 @@ void MaybeCompactWalletDB() if (fOneThread.exchange(true)) { return; } - if (!gArgs.GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) { - return; - } for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) { WalletDatabase& dbh = pwallet->GetDBHandle(); @@ -967,7 +961,7 @@ void MaybeCompactWalletDB() } if (dbh.nLastFlushed != nUpdateCounter && GetTime() - dbh.nLastWalletUpdate >= 2) { - if (BerkeleyBatch::PeriodicFlush(dbh)) { + if (dbh.PeriodicFlush()) { dbh.nLastFlushed = nUpdateCounter; } } @@ -976,16 +970,6 @@ void MaybeCompactWalletDB() fOneThread = false; } -bool WalletBatch::VerifyEnvironment(const fs::path& wallet_path, bilingual_str& errorStr) -{ - return BerkeleyBatch::VerifyEnvironment(wallet_path, errorStr); -} - -bool WalletBatch::VerifyDatabaseFile(const fs::path& wallet_path, bilingual_str& errorStr) -{ - return BerkeleyBatch::VerifyDatabaseFile(wallet_path, errorStr); -} - bool WalletBatch::WriteDestData(const std::string &address, const std::string &key, const std::string &value) { return WriteIC(std::make_pair(DBKeys::DESTDATA, std::make_pair(address, key)), value); diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 77ed6beb5d..8a45d81456 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -112,7 +112,7 @@ static bool SalvageWallet(const fs::path& path) // Initialize the environment before recovery bilingual_str error_string; try { - WalletBatch::VerifyEnvironment(path, error_string); + database->Verify(error_string); } catch (const fs::filesystem_error& e) { error_string = Untranslated(strprintf("Error loading wallet. %s", fsbridge::get_filesystem_error_message(e))); } @@ -140,11 +140,6 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) tfm::format(std::cerr, "Error: no wallet file at %s\n", name); return false; } - bilingual_str error; - if (!WalletBatch::VerifyEnvironment(path, error)) { - tfm::format(std::cerr, "%s\nError loading %s. Is wallet being used by other process?\n", error.original, name); - return false; - } if (command == "info") { std::shared_ptr<CWallet> wallet_instance = LoadWallet(name, path); |