diff options
48 files changed, 361 insertions, 172 deletions
diff --git a/src/addrman.h b/src/addrman.h index 8e82020df0..9e742339db 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -157,7 +157,7 @@ public: #define ADDRMAN_GETADDR_MAX_PCT 23 //! the maximum number of nodes to return in a getaddr call -#define ADDRMAN_GETADDR_MAX 2500 +#define ADDRMAN_GETADDR_MAX 1000 //! Convenience #define ADDRMAN_TRIED_BUCKET_COUNT (1 << ADDRMAN_TRIED_BUCKET_COUNT_LOG2) diff --git a/src/base58.cpp b/src/base58.cpp index 6a9e21ffc2..9b2946e7a9 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -141,7 +141,7 @@ std::string EncodeBase58Check(const std::vector<unsigned char>& vchIn) { // add 4-byte hash check to the end std::vector<unsigned char> vch(vchIn); - uint256 hash = Hash(vch.begin(), vch.end()); + uint256 hash = Hash(vch); vch.insert(vch.end(), (unsigned char*)&hash, (unsigned char*)&hash + 4); return EncodeBase58(vch); } @@ -154,7 +154,7 @@ bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet, int return false; } // re-calculate the checksum, ensure it matches the included 4-byte checksum - uint256 hash = Hash(vchRet.begin(), vchRet.end() - 4); + uint256 hash = Hash(MakeSpan(vchRet).first(vchRet.size() - 4)); if (memcmp(&hash, &vchRet[vchRet.size() - 4], 4) != 0) { vchRet.clear(); return false; diff --git a/src/bench/chacha_poly_aead.cpp b/src/bench/chacha_poly_aead.cpp index 30d7851b7f..3b1d3e697a 100644 --- a/src/bench/chacha_poly_aead.cpp +++ b/src/bench/chacha_poly_aead.cpp @@ -93,7 +93,7 @@ static void HASH(benchmark::Bench& bench, size_t buffersize) uint8_t hash[CHash256::OUTPUT_SIZE]; std::vector<uint8_t> in(buffersize,0); bench.batch(in.size()).unit("byte").run([&] { - CHash256().Write(in.data(), in.size()).Finalize(hash); + CHash256().Write(in).Finalize(hash); }); } diff --git a/src/bench/verify_script.cpp b/src/bench/verify_script.cpp index c489bae4f6..9af0b502eb 100644 --- a/src/bench/verify_script.cpp +++ b/src/bench/verify_script.cpp @@ -34,7 +34,7 @@ static void VerifyScriptBench(benchmark::Bench& bench) key.Set(vchKey.begin(), vchKey.end(), false); CPubKey pubkey = key.GetPubKey(); uint160 pubkeyHash; - CHash160().Write(pubkey.begin(), pubkey.size()).Finalize(pubkeyHash.begin()); + CHash160().Write(pubkey).Finalize(pubkeyHash); // Script. CScript scriptPubKey = CScript() << witnessversion << ToByteVector(pubkeyHash); diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp index 5f5bed5bda..9a6fb4abd0 100644 --- a/src/blockfilter.cpp +++ b/src/blockfilter.cpp @@ -291,7 +291,7 @@ uint256 BlockFilter::GetHash() const const std::vector<unsigned char>& data = GetEncodedFilter(); uint256 result; - CHash256().Write(data.data(), data.size()).Finalize(result.begin()); + CHash256().Write(data).Finalize(result); return result; } @@ -301,8 +301,8 @@ uint256 BlockFilter::ComputeHeader(const uint256& prev_header) const uint256 result; CHash256() - .Write(filter_hash.begin(), filter_hash.size()) - .Write(prev_header.begin(), prev_header.size()) - .Finalize(result.begin()); + .Write(filter_hash) + .Write(prev_header) + .Finalize(result); return result; } diff --git a/src/hash.cpp b/src/hash.cpp index 26150e5ca8..4c09f5f646 100644 --- a/src/hash.cpp +++ b/src/hash.cpp @@ -12,7 +12,7 @@ inline uint32_t ROTL32(uint32_t x, int8_t r) return (x << r) | (x >> (32 - r)); } -unsigned int MurmurHash3(unsigned int nHashSeed, const std::vector<unsigned char>& vDataToHash) +unsigned int MurmurHash3(unsigned int nHashSeed, Span<const unsigned char> vDataToHash) { // The following is MurmurHash3 (x86_32), see http://code.google.com/p/smhasher/source/browse/trunk/MurmurHash3.cpp uint32_t h1 = nHashSeed; diff --git a/src/hash.h b/src/hash.h index c295568a3e..71806483ff 100644 --- a/src/hash.h +++ b/src/hash.h @@ -25,14 +25,15 @@ private: public: static const size_t OUTPUT_SIZE = CSHA256::OUTPUT_SIZE; - void Finalize(unsigned char hash[OUTPUT_SIZE]) { + void Finalize(Span<unsigned char> output) { + assert(output.size() == OUTPUT_SIZE); unsigned char buf[CSHA256::OUTPUT_SIZE]; sha.Finalize(buf); - sha.Reset().Write(buf, CSHA256::OUTPUT_SIZE).Finalize(hash); + sha.Reset().Write(buf, CSHA256::OUTPUT_SIZE).Finalize(output.data()); } - CHash256& Write(const unsigned char *data, size_t len) { - sha.Write(data, len); + CHash256& Write(Span<const unsigned char> input) { + sha.Write(input.data(), input.size()); return *this; } @@ -49,14 +50,15 @@ private: public: static const size_t OUTPUT_SIZE = CRIPEMD160::OUTPUT_SIZE; - void Finalize(unsigned char hash[OUTPUT_SIZE]) { + void Finalize(Span<unsigned char> output) { + assert(output.size() == OUTPUT_SIZE); unsigned char buf[CSHA256::OUTPUT_SIZE]; sha.Finalize(buf); - CRIPEMD160().Write(buf, CSHA256::OUTPUT_SIZE).Finalize(hash); + CRIPEMD160().Write(buf, CSHA256::OUTPUT_SIZE).Finalize(output.data()); } - CHash160& Write(const unsigned char *data, size_t len) { - sha.Write(data, len); + CHash160& Write(Span<const unsigned char> input) { + sha.Write(input.data(), input.size()); return *this; } @@ -67,52 +69,31 @@ public: }; /** Compute the 256-bit hash of an object. */ -template<typename T1> -inline uint256 Hash(const T1 pbegin, const T1 pend) +template<typename T> +inline uint256 Hash(const T& in1) { - static const unsigned char pblank[1] = {}; uint256 result; - CHash256().Write(pbegin == pend ? pblank : (const unsigned char*)&pbegin[0], (pend - pbegin) * sizeof(pbegin[0])) - .Finalize((unsigned char*)&result); + CHash256().Write(MakeUCharSpan(in1)).Finalize(result); return result; } /** Compute the 256-bit hash of the concatenation of two objects. */ template<typename T1, typename T2> -inline uint256 Hash(const T1 p1begin, const T1 p1end, - const T2 p2begin, const T2 p2end) { - static const unsigned char pblank[1] = {}; +inline uint256 Hash(const T1& in1, const T2& in2) { uint256 result; - CHash256().Write(p1begin == p1end ? pblank : (const unsigned char*)&p1begin[0], (p1end - p1begin) * sizeof(p1begin[0])) - .Write(p2begin == p2end ? pblank : (const unsigned char*)&p2begin[0], (p2end - p2begin) * sizeof(p2begin[0])) - .Finalize((unsigned char*)&result); + CHash256().Write(MakeUCharSpan(in1)).Write(MakeUCharSpan(in2)).Finalize(result); return result; } /** Compute the 160-bit hash an object. */ template<typename T1> -inline uint160 Hash160(const T1 pbegin, const T1 pend) +inline uint160 Hash160(const T1& in1) { - static unsigned char pblank[1] = {}; uint160 result; - CHash160().Write(pbegin == pend ? pblank : (const unsigned char*)&pbegin[0], (pend - pbegin) * sizeof(pbegin[0])) - .Finalize((unsigned char*)&result); + CHash160().Write(MakeUCharSpan(in1)).Finalize(result); return result; } -/** Compute the 160-bit hash of a vector. */ -inline uint160 Hash160(const std::vector<unsigned char>& vch) -{ - return Hash160(vch.begin(), vch.end()); -} - -/** Compute the 160-bit hash of a vector. */ -template<unsigned int N> -inline uint160 Hash160(const prevector<N, unsigned char>& vch) -{ - return Hash160(vch.begin(), vch.end()); -} - /** A writer stream (for serialization) that computes a 256-bit hash. */ class CHashWriter { @@ -129,13 +110,13 @@ public: int GetVersion() const { return nVersion; } void write(const char *pch, size_t size) { - ctx.Write((const unsigned char*)pch, size); + ctx.Write({(const unsigned char*)pch, size}); } // invalidates the object uint256 GetHash() { uint256 result; - ctx.Finalize((unsigned char*)&result); + ctx.Finalize(result); return result; } @@ -200,7 +181,7 @@ uint256 SerializeHash(const T& obj, int nType=SER_GETHASH, int nVersion=PROTOCOL return ss.GetHash(); } -unsigned int MurmurHash3(unsigned int nHashSeed, const std::vector<unsigned char>& vDataToHash); +unsigned int MurmurHash3(unsigned int nHashSeed, Span<const unsigned char> vDataToHash); void BIP32Hash(const ChainCode &chainCode, unsigned int nChild, unsigned char header, const unsigned char data[32], unsigned char output[64]); diff --git a/src/key.cpp b/src/key.cpp index 7eecc6e083..4ed74a39b1 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -237,7 +237,7 @@ bool CKey::VerifyPubKey(const CPubKey& pubkey) const { std::string str = "Bitcoin key verification\n"; GetRandBytes(rnd, sizeof(rnd)); uint256 hash; - CHash256().Write((unsigned char*)str.data(), str.size()).Write(rnd, sizeof(rnd)).Finalize(hash.begin()); + CHash256().Write(MakeUCharSpan(str)).Write(rnd).Finalize(hash); std::vector<unsigned char> vchSig; Sign(hash, vchSig); return pubkey.Verify(hash, vchSig); diff --git a/src/merkleblock.cpp b/src/merkleblock.cpp index 8072b12119..b571d463c9 100644 --- a/src/merkleblock.cpp +++ b/src/merkleblock.cpp @@ -70,7 +70,7 @@ uint256 CPartialMerkleTree::CalcHash(int height, unsigned int pos, const std::ve else right = left; // combine subhashes - return Hash(left.begin(), left.end(), right.begin(), right.end()); + return Hash(left, right); } } @@ -126,7 +126,7 @@ uint256 CPartialMerkleTree::TraverseAndExtract(int height, unsigned int pos, uns right = left; } // and combine them before returning - return Hash(left.begin(), left.end(), right.begin(), right.end()); + return Hash(left, right); } } diff --git a/src/net.cpp b/src/net.cpp index 0c56cddbdc..9c72c62df9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -685,7 +685,7 @@ int V1TransportDeserializer::readData(const char *pch, unsigned int nBytes) vRecv.resize(std::min(hdr.nMessageSize, nDataPos + nCopy + 256 * 1024)); } - hasher.Write((const unsigned char*)pch, nCopy); + hasher.Write({(const unsigned char*)pch, nCopy}); memcpy(&vRecv[nDataPos], pch, nCopy); nDataPos += nCopy; @@ -696,7 +696,7 @@ const uint256& V1TransportDeserializer::GetMessageHash() const { assert(Complete()); if (data_hash.IsNull()) - hasher.Finalize(data_hash.begin()); + hasher.Finalize(data_hash); return data_hash; } @@ -736,7 +736,7 @@ CNetMessage V1TransportDeserializer::GetMessage(const CMessageHeader::MessageSta void V1TransportSerializer::prepareForTransport(CSerializedNetMsg& msg, std::vector<unsigned char>& header) { // create dbl-sha256 checksum - uint256 hash = Hash(msg.data.begin(), msg.data.end()); + uint256 hash = Hash(msg.data); // create header CMessageHeader hdr(Params().MessageStart(), msg.m_type.c_str(), msg.data.size()); @@ -2530,7 +2530,24 @@ void CConnman::AddNewAddresses(const std::vector<CAddress>& vAddr, const CAddres std::vector<CAddress> CConnman::GetAddresses() { - return addrman.GetAddr(); + std::vector<CAddress> addresses = addrman.GetAddr(); + if (m_banman) { + addresses.erase(std::remove_if(addresses.begin(), addresses.end(), + [this](const CAddress& addr){return m_banman->IsDiscouraged(addr) || m_banman->IsBanned(addr);}), + addresses.end()); + } + return addresses; +} + +std::vector<CAddress> CConnman::GetAddresses(Network requestor_network) +{ + const auto current_time = GetTime<std::chrono::microseconds>(); + if (m_addr_response_caches.find(requestor_network) == m_addr_response_caches.end() || + m_addr_response_caches[requestor_network].m_update_addr_response < current_time) { + m_addr_response_caches[requestor_network].m_addrs_response_cache = GetAddresses(); + m_addr_response_caches[requestor_network].m_update_addr_response = current_time + std::chrono::hours(21) + GetRandMillis(std::chrono::hours(6)); + } + return m_addr_response_caches[requestor_network].m_addrs_response_cache; } bool CConnman::AddNode(const std::string& strNode) @@ -27,6 +27,7 @@ #include <atomic> #include <cstdint> #include <deque> +#include <map> #include <thread> #include <memory> #include <condition_variable> @@ -52,6 +53,9 @@ static const int TIMEOUT_INTERVAL = 20 * 60; static const int FEELER_INTERVAL = 120; /** The maximum number of new addresses to accumulate before announcing. */ static const unsigned int MAX_ADDR_TO_SEND = 1000; +// TODO: remove ADDRMAN_GETADDR_MAX and let the caller specify this limit with MAX_ADDR_TO_SEND. +static_assert(MAX_ADDR_TO_SEND == ADDRMAN_GETADDR_MAX, + "Max allowed ADDR message size should be equal to the max number of records returned from AddrMan."); /** Maximum length of incoming protocol messages (no message over 4 MB is currently acceptable). */ static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 4 * 1000 * 1000; /** Maximum length of the user agent string in `version` message */ @@ -251,6 +255,13 @@ public: void MarkAddressGood(const CAddress& addr); void AddNewAddresses(const std::vector<CAddress>& vAddr, const CAddress& addrFrom, int64_t nTimePenalty = 0); std::vector<CAddress> GetAddresses(); + /** + * Cache is used to minimize topology leaks, so it should + * be used for all non-trusted calls, for example, p2p. + * A non-malicious call (from RPC or a peer with addr permission) should + * call the function without a parameter to avoid using the cache. + */ + std::vector<CAddress> GetAddresses(Network requestor_network); // This allows temporarily exceeding m_max_outbound_full_relay, with the goal of finding // a peer that is better than all our current peers. @@ -416,6 +427,29 @@ private: unsigned int nPrevNodeCount{0}; /** + * Cache responses to addr requests to minimize privacy leak. + * Attack example: scraping addrs in real-time may allow an attacker + * to infer new connections of the victim by detecting new records + * with fresh timestamps (per self-announcement). + */ + struct CachedAddrResponse { + std::vector<CAddress> m_addrs_response_cache; + std::chrono::microseconds m_update_addr_response{0}; + }; + + /** + * Addr responses stored in different caches + * per network prevent cross-network node identification. + * If a node for example is multi-homed under Tor and IPv6, + * a single cache (or no cache at all) would let an attacker + * to easily detect that it is the same node by comparing responses. + * The used memory equals to 1000 CAddress records (or around 32 bytes) per + * distinct Network (up to 5) we have/had an inbound peer from, + * resulting in at most ~160 KB. + */ + std::map<Network, CachedAddrResponse> m_addr_response_caches; + + /** * Services this instance offers. * * This data is replicated in each CNode instance we create during peer diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp index a75838307c..53648deb40 100644 --- a/src/net_permissions.cpp +++ b/src/net_permissions.cpp @@ -15,6 +15,7 @@ const std::vector<std::string> NET_PERMISSIONS_DOC{ "relay (relay even in -blocksonly mode)", "mempool (allow requesting BIP35 mempool contents)", "download (allow getheaders during IBD, no disconnect after maxuploadtarget limit)", + "addr (responses to GETADDR avoid hitting the cache and contain random records with the most up-to-date info)" }; namespace { @@ -50,6 +51,7 @@ bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, 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 == "addr") NetPermissions::AddFlag(flags, PF_ADDR); else if (permission.length() == 0); // Allow empty entries else { error = strprintf(_("Invalid P2P permission: '%s'"), permission); @@ -75,6 +77,7 @@ std::vector<std::string> NetPermissions::ToStrings(NetPermissionFlags flags) 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"); + if (NetPermissions::HasFlag(flags, PF_ADDR)) strings.push_back("addr"); return strings; } diff --git a/src/net_permissions.h b/src/net_permissions.h index a9633ee2ae..5b68f635a7 100644 --- a/src/net_permissions.h +++ b/src/net_permissions.h @@ -29,10 +29,12 @@ enum NetPermissionFlags { PF_NOBAN = (1U << 4) | PF_DOWNLOAD, // Can query the mempool PF_MEMPOOL = (1U << 5), + // Can request addrs without hitting a privacy-preserving cache + PF_ADDR = (1U << 7), // 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_DOWNLOAD, + PF_ALL = PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY | PF_NOBAN | PF_MEMPOOL | PF_DOWNLOAD | PF_ADDR, }; class NetPermissions diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 88e73cd804..c503a97be5 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2541,7 +2541,7 @@ void ProcessMessage( if (!pfrom.IsAddrRelayPeer()) { return; } - if (vAddr.size() > 1000) + if (vAddr.size() > MAX_ADDR_TO_SEND) { LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("addr message size = %u", vAddr.size())); @@ -3474,13 +3474,15 @@ void ProcessMessage( pfrom.fSentAddr = true; pfrom.vAddrToSend.clear(); - std::vector<CAddress> vAddr = connman.GetAddresses(); + std::vector<CAddress> vAddr; + if (pfrom.HasPermission(PF_ADDR)) { + vAddr = connman.GetAddresses(); + } else { + vAddr = connman.GetAddresses(pfrom.addr.GetNetwork()); + } FastRandomContext insecure_rand; for (const CAddress &addr : vAddr) { - bool banned_or_discouraged = banman && (banman->IsDiscouraged(addr) || banman->IsBanned(addr)); - if (!banned_or_discouraged) { - pfrom.PushAddress(addr, insecure_rand); - } + pfrom.PushAddress(addr, insecure_rand); } return; } @@ -4079,8 +4081,8 @@ bool PeerLogicValidation::SendMessages(CNode* pto) { pto->m_addr_known->insert(addr.GetKey()); vAddr.push_back(addr); - // receiver rejects addr messages larger than 1000 - if (vAddr.size() >= 1000) + // receiver rejects addr messages larger than MAX_ADDR_TO_SEND + if (vAddr.size() >= MAX_ADDR_TO_SEND) { connman->PushMessage(pto, msgMaker.Make(NetMsgType::ADDR, vAddr)); vAddr.clear(); diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 0874b8dcea..d29aed6c8b 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -553,7 +553,7 @@ std::vector<unsigned char> CNetAddr::GetGroup(const std::vector<bool> &asmap) co uint64_t CNetAddr::GetHash() const { - uint256 hash = Hash(&ip[0], &ip[16]); + uint256 hash = Hash(ip); uint64_t nRet; memcpy(&nRet, &hash, sizeof(nRet)); return nRet; diff --git a/src/pubkey.h b/src/pubkey.h index 4c28af4a4d..fcbc7e8416 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -157,13 +157,13 @@ public: //! Get the KeyID of this public key (hash of its serialization) CKeyID GetID() const { - return CKeyID(Hash160(vch, vch + size())); + return CKeyID(Hash160(MakeSpan(vch).first(size()))); } //! Get the 256-bit hash of this public key. uint256 GetHash() const { - return Hash(vch, vch + size()); + return Hash(MakeSpan(vch).first(size())); } /* diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 70caf6009a..cf856af6e9 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -601,7 +601,7 @@ static UniValue decodescript(const JSONRPCRequest& request) UniValue sr(UniValue::VOBJ); CScript segwitScr; if (which_type == TxoutType::PUBKEY) { - segwitScr = GetScriptForDestination(WitnessV0KeyHash(Hash160(solutions_data[0].begin(), solutions_data[0].end()))); + segwitScr = GetScriptForDestination(WitnessV0KeyHash(Hash160(solutions_data[0]))); } else if (which_type == TxoutType::PUBKEYHASH) { segwitScr = GetScriptForDestination(WitnessV0KeyHash(uint160{solutions_data[0]})); } else { diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 9415bba585..39feb4ccc9 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -986,9 +986,9 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& else if (opcode == OP_SHA256) CSHA256().Write(vch.data(), vch.size()).Finalize(vchHash.data()); else if (opcode == OP_HASH160) - CHash160().Write(vch.data(), vch.size()).Finalize(vchHash.data()); + CHash160().Write(vch).Finalize(vchHash); else if (opcode == OP_HASH256) - CHash256().Write(vch.data(), vch.size()).Finalize(vchHash.data()); + CHash256().Write(vch).Finalize(vchHash); popstack(stack); stack.push_back(vchHash); } diff --git a/src/script/standard.cpp b/src/script/standard.cpp index 1c4990791c..3a4882f280 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -16,10 +16,10 @@ typedef std::vector<unsigned char> valtype; bool fAcceptDatacarrier = DEFAULT_ACCEPT_DATACARRIER; unsigned nMaxDatacarrierBytes = MAX_OP_RETURN_RELAY; -CScriptID::CScriptID(const CScript& in) : BaseHash(Hash160(in.begin(), in.end())) {} +CScriptID::CScriptID(const CScript& in) : BaseHash(Hash160(in)) {} CScriptID::CScriptID(const ScriptHash& in) : BaseHash(static_cast<uint160>(in)) {} -ScriptHash::ScriptHash(const CScript& in) : BaseHash(Hash160(in.begin(), in.end())) {} +ScriptHash::ScriptHash(const CScript& in) : BaseHash(Hash160(in)) {} ScriptHash::ScriptHash(const CScriptID& in) : BaseHash(static_cast<uint160>(in)) {} PKHash::PKHash(const CPubKey& pubkey) : BaseHash(pubkey.GetID()) {} @@ -318,7 +318,7 @@ CScript GetScriptForWitness(const CScript& redeemscript) std::vector<std::vector<unsigned char> > vSolutions; TxoutType typ = Solver(redeemscript, vSolutions); if (typ == TxoutType::PUBKEY) { - return GetScriptForDestination(WitnessV0KeyHash(Hash160(vSolutions[0].begin(), vSolutions[0].end()))); + return GetScriptForDestination(WitnessV0KeyHash(Hash160(vSolutions[0]))); } else if (typ == TxoutType::PUBKEYHASH) { return GetScriptForDestination(WitnessV0KeyHash(uint160{vSolutions[0]})); } diff --git a/src/script/standard.h b/src/script/standard.h index fd29353886..992e37675f 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -79,6 +79,9 @@ public: { return m_hash.size(); } + + unsigned char* data() { return m_hash.data(); } + const unsigned char* data() const { return m_hash.data(); } }; /** A reference to a CScript: the Hash160 of its serialization (see script.h) */ diff --git a/src/span.h b/src/span.h index 79f13c9203..4afb383a59 100644 --- a/src/span.h +++ b/src/span.h @@ -207,4 +207,16 @@ T& SpanPopBack(Span<T>& span) return back; } +// Helper functions to safely cast to unsigned char pointers. +inline unsigned char* UCharCast(char* c) { return (unsigned char*)c; } +inline unsigned char* UCharCast(unsigned char* c) { return c; } +inline const unsigned char* UCharCast(const char* c) { return (unsigned char*)c; } +inline const unsigned char* UCharCast(const unsigned char* c) { return c; } + +// Helper function to safely convert a Span to a Span<[const] unsigned char>. +template <typename T> constexpr auto UCharSpanCast(Span<T> s) -> Span<typename std::remove_pointer<decltype(UCharCast(s.data()))>::type> { return {UCharCast(s.data()), s.size()}; } + +/** Like MakeSpan, but for (const) unsigned char member types only. Only works for (un)signed char containers. */ +template <typename V> constexpr auto MakeUCharSpan(V&& v) -> decltype(UCharSpanCast(MakeSpan(std::forward<V>(v)))) { return UCharSpanCast(MakeSpan(std::forward<V>(v))); } + #endif diff --git a/src/sync.cpp b/src/sync.cpp index 10f0483189..4be13a3c48 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -149,12 +149,17 @@ static void push_lock(void* c, const CLockLocation& locklocation) const LockPair p1 = std::make_pair(i.first, c); if (lockdata.lockorders.count(p1)) continue; - lockdata.lockorders.emplace(p1, lock_stack); const LockPair p2 = std::make_pair(c, i.first); + if (lockdata.lockorders.count(p2)) { + auto lock_stack_copy = lock_stack; + lock_stack.pop_back(); + potential_deadlock_detected(p1, lockdata.lockorders[p2], lock_stack_copy); + // potential_deadlock_detected() does not return. + } + + lockdata.lockorders.emplace(p1, lock_stack); lockdata.invlockorders.insert(p2); - if (lockdata.lockorders.count(p2)) - potential_deadlock_detected(p1, lockdata.lockorders[p2], lockdata.lockorders[p1]); } } @@ -259,6 +264,17 @@ void DeleteLock(void* cs) } } +bool LockStackEmpty() +{ + LockData& lockdata = GetLockData(); + std::lock_guard<std::mutex> lock(lockdata.dd_mutex); + const auto it = lockdata.m_lock_stacks.find(std::this_thread::get_id()); + if (it == lockdata.m_lock_stacks.end()) { + return true; + } + return it->second.empty(); +} + bool g_debug_lockorder_abort = true; #endif /* DEBUG_LOCKORDER */ diff --git a/src/sync.h b/src/sync.h index 77327d8bfe..05ff2ee8a9 100644 --- a/src/sync.h +++ b/src/sync.h @@ -56,6 +56,7 @@ template <typename MutexType> void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs); void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs); void DeleteLock(void* cs); +bool LockStackEmpty(); /** * Call abort() if a potential lock order deadlock bug is detected, instead of @@ -64,13 +65,14 @@ void DeleteLock(void* cs); */ extern bool g_debug_lockorder_abort; #else -void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} -void static inline LeaveCritical() {} -void static inline CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {} +inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} +inline void LeaveCritical() {} +inline void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {} template <typename MutexType> -void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} -void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} -void static inline DeleteLock(void* cs) {} +inline void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} +inline void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} +inline void DeleteLock(void* cs) {} +inline bool LockStackEmpty() { return true; } #endif #define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) #define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs) diff --git a/src/test/crypto_tests.cpp b/src/test/crypto_tests.cpp index f64251fe32..bf5c587774 100644 --- a/src/test/crypto_tests.cpp +++ b/src/test/crypto_tests.cpp @@ -743,7 +743,7 @@ BOOST_AUTO_TEST_CASE(sha256d64) in[j] = InsecureRandBits(8); } for (int j = 0; j < i; ++j) { - CHash256().Write(in + 64 * j, 64).Finalize(out1 + 32 * j); + CHash256().Write({in + 64 * j, 64}).Finalize({out1 + 32 * j, 32}); } SHA256D64(out2, in, i); BOOST_CHECK(memcmp(out1, out2, 32 * i) == 0); diff --git a/src/test/fuzz/crypto.cpp b/src/test/fuzz/crypto.cpp index 595cdf9abb..3edcf96495 100644 --- a/src/test/fuzz/crypto.cpp +++ b/src/test/fuzz/crypto.cpp @@ -44,8 +44,8 @@ void test_one_input(const std::vector<uint8_t>& buffer) } } - (void)hash160.Write(data.data(), data.size()); - (void)hash256.Write(data.data(), data.size()); + (void)hash160.Write(data); + (void)hash256.Write(data); (void)hmac_sha256.Write(data.data(), data.size()); (void)hmac_sha512.Write(data.data(), data.size()); (void)ripemd160.Write(data.data(), data.size()); @@ -54,9 +54,8 @@ void test_one_input(const std::vector<uint8_t>& buffer) (void)sha512.Write(data.data(), data.size()); (void)sip_hasher.Write(data.data(), data.size()); - (void)Hash(data.begin(), data.end()); + (void)Hash(data); (void)Hash160(data); - (void)Hash160(data.begin(), data.end()); (void)sha512.Size(); break; } @@ -73,12 +72,12 @@ void test_one_input(const std::vector<uint8_t>& buffer) switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 8)) { case 0: { data.resize(CHash160::OUTPUT_SIZE); - hash160.Finalize(data.data()); + hash160.Finalize(data); break; } case 1: { data.resize(CHash256::OUTPUT_SIZE); - hash256.Finalize(data.data()); + hash256.Finalize(data); break; } case 2: { diff --git a/src/test/fuzz/key.cpp b/src/test/fuzz/key.cpp index c746374c61..955b954700 100644 --- a/src/test/fuzz/key.cpp +++ b/src/test/fuzz/key.cpp @@ -85,7 +85,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) assert(negated_key == key); } - const uint256 random_uint256 = Hash(buffer.begin(), buffer.end()); + const uint256 random_uint256 = Hash(buffer); { CKey child_key; diff --git a/src/test/fuzz/net_permissions.cpp b/src/test/fuzz/net_permissions.cpp index ae531f4462..8a674ac1e9 100644 --- a/src/test/fuzz/net_permissions.cpp +++ b/src/test/fuzz/net_permissions.cpp @@ -24,6 +24,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) NetPermissionFlags::PF_FORCERELAY, NetPermissionFlags::PF_NOBAN, NetPermissionFlags::PF_MEMPOOL, + NetPermissionFlags::PF_ADDR, NetPermissionFlags::PF_ISIMPLICIT, NetPermissionFlags::PF_ALL, }) : diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index fd35537c77..4e4c44266a 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -77,7 +77,7 @@ BOOST_AUTO_TEST_CASE(key_test1) for (int n=0; n<16; n++) { std::string strMsg = strprintf("Very secret message %i: 11", n); - uint256 hashMsg = Hash(strMsg.begin(), strMsg.end()); + uint256 hashMsg = Hash(strMsg); // normal signatures @@ -134,7 +134,7 @@ BOOST_AUTO_TEST_CASE(key_test1) std::vector<unsigned char> detsig, detsigc; std::string strMsg = "Very deterministic message"; - uint256 hashMsg = Hash(strMsg.begin(), strMsg.end()); + uint256 hashMsg = Hash(strMsg); BOOST_CHECK(key1.Sign(hashMsg, detsig)); BOOST_CHECK(key1C.Sign(hashMsg, detsigc)); BOOST_CHECK(detsig == detsigc); @@ -158,7 +158,7 @@ BOOST_AUTO_TEST_CASE(key_signature_tests) // When entropy is specified, we should see at least one high R signature within 20 signatures CKey key = DecodeSecret(strSecret1); std::string msg = "A message to be signed"; - uint256 msg_hash = Hash(msg.begin(), msg.end()); + uint256 msg_hash = Hash(msg); std::vector<unsigned char> sig; bool found = false; @@ -179,7 +179,7 @@ BOOST_AUTO_TEST_CASE(key_signature_tests) for (int i = 0; i < 256; ++i) { sig.clear(); std::string msg = "A message to be signed" + ToString(i); - msg_hash = Hash(msg.begin(), msg.end()); + msg_hash = Hash(msg); BOOST_CHECK(key.Sign(msg_hash, sig)); found = sig[3] == 0x20; BOOST_CHECK(sig.size() <= 70); @@ -196,7 +196,7 @@ BOOST_AUTO_TEST_CASE(key_key_negation) std::string str = "Bitcoin key verification\n"; GetRandBytes(rnd, sizeof(rnd)); uint256 hash; - CHash256().Write((unsigned char*)str.data(), str.size()).Write(rnd, sizeof(rnd)).Finalize(hash.begin()); + CHash256().Write(MakeUCharSpan(str)).Write(rnd).Finalize(hash); // import the static test key CKey key = DecodeSecret(strSecret1C); diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp index 03dce552fc..9bc7cc5dab 100644 --- a/src/test/merkle_tests.cpp +++ b/src/test/merkle_tests.cpp @@ -13,9 +13,9 @@ static uint256 ComputeMerkleRootFromBranch(const uint256& leaf, const std::vecto uint256 hash = leaf; for (std::vector<uint256>::const_iterator it = vMerkleBranch.begin(); it != vMerkleBranch.end(); ++it) { if (nIndex & 1) { - hash = Hash(it->begin(), it->end(), hash.begin(), hash.end()); + hash = Hash(*it, hash); } else { - hash = Hash(hash.begin(), hash.end(), it->begin(), it->end()); + hash = Hash(hash, *it); } nIndex >>= 1; } @@ -60,7 +60,7 @@ static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot } } mutated |= (inner[level] == h); - CHash256().Write(inner[level].begin(), 32).Write(h.begin(), 32).Finalize(h.begin()); + CHash256().Write(inner[level]).Write(h).Finalize(h); } // Store the resulting hash at inner position level. inner[level] = h; @@ -86,7 +86,7 @@ static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot if (pbranch && matchh) { pbranch->push_back(h); } - CHash256().Write(h.begin(), 32).Write(h.begin(), 32).Finalize(h.begin()); + CHash256().Write(h).Write(h).Finalize(h); // Increment count to the value it would have if two entries at this // level had existed. count += (((uint32_t)1) << level); @@ -101,7 +101,7 @@ static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot matchh = true; } } - CHash256().Write(inner[level].begin(), 32).Write(h.begin(), 32).Finalize(h.begin()); + CHash256().Write(inner[level]).Write(h).Finalize(h); level++; } } @@ -144,8 +144,7 @@ static uint256 BlockBuildMerkleTree(const CBlock& block, bool* fMutated, std::ve // Two identical hashes at the end of the list at a particular level. mutated = true; } - vMerkleTree.push_back(Hash(vMerkleTree[j+i].begin(), vMerkleTree[j+i].end(), - vMerkleTree[j+i2].begin(), vMerkleTree[j+i2].end())); + vMerkleTree.push_back(Hash(vMerkleTree[j+i], vMerkleTree[j+i2])); } j += nSize; } diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 591b4ce49a..49073ea657 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -406,13 +406,14 @@ 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(), 6U); + BOOST_CHECK_EQUAL(strings.size(), 7U); 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_CHECK(std::find(strings.begin(), strings.end(), "addr") != strings.end()); } BOOST_AUTO_TEST_CASE(netbase_dont_resolve_strings_with_embedded_nul_characters) diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index 77d748241b..87678af4d1 100644 --- a/src/test/script_standard_tests.cpp +++ b/src/test/script_standard_tests.cpp @@ -216,7 +216,7 @@ BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination) s << OP_0 << ToByteVector(pubkey.GetID()); BOOST_CHECK(ExtractDestination(s, address)); WitnessV0KeyHash keyhash; - CHash160().Write(pubkey.begin(), pubkey.size()).Finalize(keyhash.begin()); + CHash160().Write(pubkey).Finalize(keyhash); BOOST_CHECK(boost::get<WitnessV0KeyHash>(&address) && *boost::get<WitnessV0KeyHash>(&address) == keyhash); // TxoutType::WITNESS_V0_SCRIPTHASH diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index cb3ae290d1..0830743d61 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -282,7 +282,7 @@ public: CScript scriptPubKey = script; if (wm == WitnessMode::PKH) { uint160 hash; - CHash160().Write(&script[1], script.size() - 1).Finalize(hash.begin()); + CHash160().Write(MakeSpan(script).subspan(1)).Finalize(hash); script = CScript() << OP_DUP << OP_HASH160 << ToByteVector(hash) << OP_EQUALVERIFY << OP_CHECKSIG; scriptPubKey = CScript() << witnessversion << ToByteVector(hash); } else if (wm == WitnessMode::SH) { diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index c2328f931c..f625b67c2a 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(floats) for (int i = 0; i < 1000; i++) { ss << float(i); } - BOOST_CHECK(Hash(ss.begin(), ss.end()) == uint256S("8e8b4cf3e4df8b332057e3e23af42ebc663b61e0495d5e7e32d85099d7f3fe0c")); + BOOST_CHECK(Hash(ss) == uint256S("8e8b4cf3e4df8b332057e3e23af42ebc663b61e0495d5e7e32d85099d7f3fe0c")); // decode for (int i = 0; i < 1000; i++) { @@ -162,7 +162,7 @@ BOOST_AUTO_TEST_CASE(doubles) for (int i = 0; i < 1000; i++) { ss << double(i); } - BOOST_CHECK(Hash(ss.begin(), ss.end()) == uint256S("43d0c82591953c4eafe114590d392676a01585d25b25d433557f0d7878b23f96")); + BOOST_CHECK(Hash(ss) == uint256S("43d0c82591953c4eafe114590d392676a01585d25b25d433557f0d7878b23f96")); // decode for (int i = 0; i < 1000; i++) { diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp index 1a2d775f49..91e039416c 100644 --- a/src/test/settings_tests.cpp +++ b/src/test/settings_tests.cpp @@ -228,7 +228,7 @@ BOOST_FIXTURE_TEST_CASE(Merge, MergeTestingSetup) if (OnlyHasDefaultSectionSetting(settings, network, name)) desc += " ignored"; desc += "\n"; - out_sha.Write((const unsigned char*)desc.data(), desc.size()); + out_sha.Write(MakeUCharSpan(desc)); if (out_file) { BOOST_REQUIRE(fwrite(desc.data(), 1, desc.size(), out_file) == desc.size()); } diff --git a/src/test/sync_tests.cpp b/src/test/sync_tests.cpp index 3ea8714f3a..19029ebd3c 100644 --- a/src/test/sync_tests.cpp +++ b/src/test/sync_tests.cpp @@ -14,6 +14,7 @@ void TestPotentialDeadLockDetected(MutexType& mutex1, MutexType& mutex2) { LOCK2(mutex1, mutex2); } + BOOST_CHECK(LockStackEmpty()); bool error_thrown = false; try { LOCK2(mutex2, mutex1); @@ -21,6 +22,7 @@ void TestPotentialDeadLockDetected(MutexType& mutex1, MutexType& mutex2) BOOST_CHECK_EQUAL(e.what(), "potential deadlock detected: mutex1 -> mutex2 -> mutex1"); error_thrown = true; } + BOOST_CHECK(LockStackEmpty()); #ifdef DEBUG_LOCKORDER BOOST_CHECK(error_thrown); #else @@ -40,9 +42,13 @@ BOOST_AUTO_TEST_CASE(potential_deadlock_detected) RecursiveMutex rmutex1, rmutex2; TestPotentialDeadLockDetected(rmutex1, rmutex2); + // The second test ensures that lock tracking data have not been broken by exception. + TestPotentialDeadLockDetected(rmutex1, rmutex2); Mutex mutex1, mutex2; TestPotentialDeadLockDetected(mutex1, mutex2); + // The second test ensures that lock tracking data have not been broken by exception. + TestPotentialDeadLockDetected(mutex1, mutex2); #ifdef DEBUG_LOCKORDER g_debug_lockorder_abort = prev; diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index a30e366028..15a2c1e300 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1009,7 +1009,7 @@ BOOST_FIXTURE_TEST_CASE(util_ArgsMerge, ArgsMergeTestingSetup) desc += "\n"; - out_sha.Write((const unsigned char*)desc.data(), desc.size()); + out_sha.Write(MakeUCharSpan(desc)); if (out_file) { BOOST_REQUIRE(fwrite(desc.data(), 1, desc.size(), out_file) == desc.size()); } @@ -1112,7 +1112,7 @@ BOOST_FIXTURE_TEST_CASE(util_ChainMerge, ChainMergeTestingSetup) } desc += "\n"; - out_sha.Write((const unsigned char*)desc.data(), desc.size()); + out_sha.Write(MakeUCharSpan(desc)); if (out_file) { BOOST_REQUIRE(fwrite(desc.data(), 1, desc.size(), out_file) == desc.size()); } @@ -2186,8 +2186,8 @@ BOOST_AUTO_TEST_CASE(message_hash) std::string(1, (char)unsigned_tx.length()) + unsigned_tx; - const uint256 signature_hash = Hash(unsigned_tx.begin(), unsigned_tx.end()); - const uint256 message_hash1 = Hash(prefixed_message.begin(), prefixed_message.end()); + const uint256 signature_hash = Hash(unsigned_tx); + const uint256 message_hash1 = Hash(prefixed_message); const uint256 message_hash2 = MessageHash(unsigned_tx); BOOST_CHECK_EQUAL(message_hash1, message_hash2); diff --git a/src/uint256.cpp b/src/uint256.cpp index a943e71062..a5dfba41e2 100644 --- a/src/uint256.cpp +++ b/src/uint256.cpp @@ -12,20 +12,20 @@ template <unsigned int BITS> base_blob<BITS>::base_blob(const std::vector<unsigned char>& vch) { - assert(vch.size() == sizeof(data)); - memcpy(data, vch.data(), sizeof(data)); + assert(vch.size() == sizeof(m_data)); + memcpy(m_data, vch.data(), sizeof(m_data)); } template <unsigned int BITS> std::string base_blob<BITS>::GetHex() const { - return HexStr(std::reverse_iterator<const uint8_t*>(data + sizeof(data)), std::reverse_iterator<const uint8_t*>(data)); + return HexStr(std::reverse_iterator<const uint8_t*>(m_data + sizeof(m_data)), std::reverse_iterator<const uint8_t*>(m_data)); } template <unsigned int BITS> void base_blob<BITS>::SetHex(const char* psz) { - memset(data, 0, sizeof(data)); + memset(m_data, 0, sizeof(m_data)); // skip leading spaces while (IsSpace(*psz)) @@ -39,7 +39,7 @@ void base_blob<BITS>::SetHex(const char* psz) size_t digits = 0; while (::HexDigit(psz[digits]) != -1) digits++; - unsigned char* p1 = (unsigned char*)data; + unsigned char* p1 = (unsigned char*)m_data; unsigned char* pend = p1 + WIDTH; while (digits > 0 && p1 < pend) { *p1 = ::HexDigit(psz[--digits]); diff --git a/src/uint256.h b/src/uint256.h index b36598f572..8ab747ef49 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -18,11 +18,11 @@ class base_blob { protected: static constexpr int WIDTH = BITS / 8; - uint8_t data[WIDTH]; + uint8_t m_data[WIDTH]; public: base_blob() { - memset(data, 0, sizeof(data)); + memset(m_data, 0, sizeof(m_data)); } explicit base_blob(const std::vector<unsigned char>& vch); @@ -30,17 +30,17 @@ public: bool IsNull() const { for (int i = 0; i < WIDTH; i++) - if (data[i] != 0) + if (m_data[i] != 0) return false; return true; } void SetNull() { - memset(data, 0, sizeof(data)); + memset(m_data, 0, sizeof(m_data)); } - inline int Compare(const base_blob& other) const { return memcmp(data, other.data, sizeof(data)); } + inline int Compare(const base_blob& other) const { return memcmp(m_data, other.m_data, sizeof(m_data)); } friend inline bool operator==(const base_blob& a, const base_blob& b) { return a.Compare(b) == 0; } friend inline bool operator!=(const base_blob& a, const base_blob& b) { return a.Compare(b) != 0; } @@ -51,34 +51,37 @@ public: void SetHex(const std::string& str); std::string ToString() const; + const unsigned char* data() const { return m_data; } + unsigned char* data() { return m_data; } + unsigned char* begin() { - return &data[0]; + return &m_data[0]; } unsigned char* end() { - return &data[WIDTH]; + return &m_data[WIDTH]; } const unsigned char* begin() const { - return &data[0]; + return &m_data[0]; } const unsigned char* end() const { - return &data[WIDTH]; + return &m_data[WIDTH]; } unsigned int size() const { - return sizeof(data); + return sizeof(m_data); } uint64_t GetUint64(int pos) const { - const uint8_t* ptr = data + pos * 8; + const uint8_t* ptr = m_data + pos * 8; return ((uint64_t)ptr[0]) | \ ((uint64_t)ptr[1]) << 8 | \ ((uint64_t)ptr[2]) << 16 | \ @@ -92,13 +95,13 @@ public: template<typename Stream> void Serialize(Stream& s) const { - s.write((char*)data, sizeof(data)); + s.write((char*)m_data, sizeof(m_data)); } template<typename Stream> void Unserialize(Stream& s) { - s.read((char*)data, sizeof(data)); + s.read((char*)m_data, sizeof(m_data)); } }; diff --git a/src/validation.cpp b/src/validation.cpp index 3cebf1090e..81af972777 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3434,7 +3434,7 @@ std::vector<unsigned char> GenerateCoinbaseCommitment(CBlock& block, const CBloc if (consensusParams.SegwitHeight != std::numeric_limits<int>::max()) { if (commitpos == -1) { uint256 witnessroot = BlockWitnessMerkleRoot(block, nullptr); - CHash256().Write(witnessroot.begin(), 32).Write(ret.data(), 32).Finalize(witnessroot.begin()); + CHash256().Write(witnessroot).Write(ret).Finalize(witnessroot); CTxOut out; out.nValue = 0; out.scriptPubKey.resize(MINIMUM_WITNESS_COMMITMENT); @@ -3579,7 +3579,7 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat if (block.vtx[0]->vin[0].scriptWitness.stack.size() != 1 || block.vtx[0]->vin[0].scriptWitness.stack[0].size() != 32) { return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-witness-nonce-size", strprintf("%s : invalid witness reserved value size", __func__)); } - CHash256().Write(hashWitness.begin(), 32).Write(&block.vtx[0]->vin[0].scriptWitness.stack[0][0], 32).Finalize(hashWitness.begin()); + CHash256().Write(hashWitness).Write(block.vtx[0]->vin[0].scriptWitness.stack[0]).Finalize(hashWitness); if (memcmp(hashWitness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) { return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__)); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 9d334063c4..39d1f49e9e 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2338,7 +2338,7 @@ static UniValue getwalletinfo(const JSONRPCRequest& request) {RPCResult::Type::NUM_TIME, "keypoololdest", "the " + UNIX_EPOCH_TIME + " of the oldest pre-generated key in the key pool. Legacy wallets only."}, {RPCResult::Type::NUM, "keypoolsize", "how many new keys are pre-generated (only counts external keys)"}, {RPCResult::Type::NUM, "keypoolsize_hd_internal", "how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)"}, - {RPCResult::Type::NUM_TIME, "unlocked_until", "the " + UNIX_EPOCH_TIME + " until which the wallet is unlocked for transfers, or 0 if the wallet is locked"}, + {RPCResult::Type::NUM_TIME, "unlocked_until", /* optional */ true, "the " + UNIX_EPOCH_TIME + " until which the wallet is unlocked for transfers, or 0 if the wallet is locked (only present for passphrase-encrypted wallets)"}, {RPCResult::Type::STR_AMOUNT, "paytxfee", "the transaction fee configuration, set in " + CURRENCY_UNIT + "/kB"}, {RPCResult::Type::STR_HEX, "hdseedid", /* optional */ true, "the Hash160 of the HD seed (only present when HD is enabled)"}, {RPCResult::Type::BOOL, "private_keys_enabled", "false if privatekeys are disabled for this wallet (enforced watch-only wallet)"}, diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index d2770a46f7..7ef06663b5 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -630,13 +630,13 @@ static size_t CalculateNestedKeyhashInputSize(bool use_max_sig) CPubKey pubkey = key.GetPubKey(); // Generate pubkey hash - uint160 key_hash(Hash160(pubkey.begin(), pubkey.end())); + uint160 key_hash(Hash160(pubkey)); // Create inner-script to enter into keystore. Key hash can't be 0... CScript inner_script = CScript() << OP_0 << std::vector<unsigned char>(key_hash.begin(), key_hash.end()); // Create outer P2SH script for the output - uint160 script_id(Hash160(inner_script.begin(), inner_script.end())); + uint160 script_id(Hash160(inner_script)); CScript script_pubkey = CScript() << OP_HASH160 << std::vector<unsigned char>(script_id.begin(), script_id.end()) << OP_EQUAL; // Add inner-script to key store and key to watchonly diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index a6d327994b..fa6814d0d3 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -103,7 +103,7 @@ bool WalletBatch::WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, vchKey.insert(vchKey.end(), vchPubKey.begin(), vchPubKey.end()); vchKey.insert(vchKey.end(), vchPrivKey.begin(), vchPrivKey.end()); - return WriteIC(std::make_pair(DBKeys::KEY, vchPubKey), std::make_pair(vchPrivKey, Hash(vchKey.begin(), vchKey.end())), false); + return WriteIC(std::make_pair(DBKeys::KEY, vchPubKey), std::make_pair(vchPrivKey, Hash(vchKey)), false); } bool WalletBatch::WriteCryptedKey(const CPubKey& vchPubKey, @@ -115,7 +115,7 @@ bool WalletBatch::WriteCryptedKey(const CPubKey& vchPubKey, } // Compute a checksum of the encrypted key - uint256 checksum = Hash(vchCryptedSecret.begin(), vchCryptedSecret.end()); + uint256 checksum = Hash(vchCryptedSecret); const auto key = std::make_pair(DBKeys::CRYPTED_KEY, vchPubKey); if (!WriteIC(key, std::make_pair(vchCryptedSecret, checksum), false)) { @@ -209,7 +209,7 @@ bool WalletBatch::WriteDescriptorKey(const uint256& desc_id, const CPubKey& pubk key.insert(key.end(), pubkey.begin(), pubkey.end()); key.insert(key.end(), privkey.begin(), privkey.end()); - return WriteIC(std::make_pair(DBKeys::WALLETDESCRIPTORKEY, std::make_pair(desc_id, pubkey)), std::make_pair(privkey, Hash(key.begin(), key.end())), false); + return WriteIC(std::make_pair(DBKeys::WALLETDESCRIPTORKEY, std::make_pair(desc_id, pubkey)), std::make_pair(privkey, Hash(key)), false); } bool WalletBatch::WriteCryptedDescriptorKey(const uint256& desc_id, const CPubKey& pubkey, const std::vector<unsigned char>& secret) @@ -365,7 +365,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, vchKey.insert(vchKey.end(), vchPubKey.begin(), vchPubKey.end()); vchKey.insert(vchKey.end(), pkey.begin(), pkey.end()); - if (Hash(vchKey.begin(), vchKey.end()) != hash) + if (Hash(vchKey) != hash) { strErr = "Error reading wallet database: CPubKey/CPrivKey corrupt"; return false; @@ -414,7 +414,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, if (!ssValue.eof()) { uint256 checksum; ssValue >> checksum; - if ((checksum_valid = Hash(vchPrivKey.begin(), vchPrivKey.end()) != checksum)) { + if ((checksum_valid = Hash(vchPrivKey) != checksum)) { strErr = "Error reading wallet database: Crypted key corrupt"; return false; } @@ -621,7 +621,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, to_hash.insert(to_hash.end(), pubkey.begin(), pubkey.end()); to_hash.insert(to_hash.end(), pkey.begin(), pkey.end()); - if (Hash(to_hash.begin(), to_hash.end()) != hash) + if (Hash(to_hash) != hash) { strErr = "Error reading wallet database: CPubKey/CPrivKey corrupt"; return false; diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py index 741da3be31..c8e2616b79 100755 --- a/test/functional/p2p_filter.py +++ b/test/functional/p2p_filter.py @@ -218,7 +218,11 @@ class FilterTest(BitcoinTestFramework): # Add peer but do not send version yet filter_peer_without_nrelay = self.nodes[0].add_p2p_connection(P2PBloomFilter(), send_version=False, wait_for_verack=False) # Send version with fRelay=False - filter_peer_without_nrelay.wait_until(lambda: filter_peer_without_nrelay.is_connected, timeout=10) + filter_peer_without_nrelay.wait_until( + lambda: filter_peer_without_nrelay.is_connected, + timeout=10, + check_connected=False, + ) version_without_fRelay = msg_version() version_without_fRelay.nRelay = 0 filter_peer_without_nrelay.send_message(version_without_fRelay) diff --git a/test/functional/p2p_getaddr_caching.py b/test/functional/p2p_getaddr_caching.py new file mode 100755 index 0000000000..c9278eab92 --- /dev/null +++ b/test/functional/p2p_getaddr_caching.py @@ -0,0 +1,109 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test addr response caching""" + +import time +from test_framework.messages import ( + CAddress, + NODE_NETWORK, + NODE_WITNESS, + msg_addr, + msg_getaddr, +) +from test_framework.mininode import ( + P2PInterface, + mininode_lock +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, +) + +MAX_ADDR_TO_SEND = 1000 + +def gen_addrs(n): + addrs = [] + for i in range(n): + addr = CAddress() + addr.time = int(time.time()) + addr.nServices = NODE_NETWORK | NODE_WITNESS + # Use first octets to occupy different AddrMan buckets + first_octet = i >> 8 + second_octet = i % 256 + addr.ip = "{}.{}.1.1".format(first_octet, second_octet) + addr.port = 8333 + addrs.append(addr) + return addrs + +class AddrReceiver(P2PInterface): + + def __init__(self): + super().__init__() + self.received_addrs = None + + def get_received_addrs(self): + with mininode_lock: + return self.received_addrs + + def on_addr(self, message): + self.received_addrs = [] + for addr in message.addrs: + self.received_addrs.append(addr.ip) + + def addr_received(self): + return self.received_addrs is not None + + +class AddrTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = False + self.num_nodes = 1 + + def run_test(self): + self.log.info('Create connection that sends and requests addr messages') + addr_source = self.nodes[0].add_p2p_connection(P2PInterface()) + + msg_send_addrs = msg_addr() + self.log.info('Fill peer AddrMan with a lot of records') + # Since these addrs are sent from the same source, not all of them will be stored, + # because we allocate a limited number of AddrMan buckets per addr source. + total_addrs = 10000 + addrs = gen_addrs(total_addrs) + for i in range(int(total_addrs/MAX_ADDR_TO_SEND)): + msg_send_addrs.addrs = addrs[i * MAX_ADDR_TO_SEND:(i + 1) * MAX_ADDR_TO_SEND] + addr_source.send_and_ping(msg_send_addrs) + + responses = [] + self.log.info('Send many addr requests within short time to receive same response') + N = 5 + cur_mock_time = int(time.time()) + for i in range(N): + addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) + addr_receiver.send_and_ping(msg_getaddr()) + # Trigger response + cur_mock_time += 5 * 60 + self.nodes[0].setmocktime(cur_mock_time) + addr_receiver.wait_until(addr_receiver.addr_received) + responses.append(addr_receiver.get_received_addrs()) + for response in responses[1:]: + assert_equal(response, responses[0]) + assert(len(response) < MAX_ADDR_TO_SEND) + + cur_mock_time += 3 * 24 * 60 * 60 + self.nodes[0].setmocktime(cur_mock_time) + + self.log.info('After time passed, see a new response to addr request') + last_addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) + last_addr_receiver.send_and_ping(msg_getaddr()) + # Trigger response + cur_mock_time += 5 * 60 + self.nodes[0].setmocktime(cur_mock_time) + last_addr_receiver.wait_until(last_addr_receiver.addr_received) + # new response is different + assert(set(responses[0]) != set(last_addr_receiver.get_received_addrs())) + + +if __name__ == '__main__': + AddrTest().main() diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py index 32a795e345..254352c816 100755 --- a/test/functional/p2p_permissions.py +++ b/test/functional/p2p_permissions.py @@ -96,7 +96,7 @@ class P2PPermissionsTests(BitcoinTestFramework): self.checkpermission( # all permission added ["-whitelist=all@127.0.0.1"], - ["forcerelay", "noban", "mempool", "bloomfilter", "relay", "download"], + ["forcerelay", "noban", "mempool", "bloomfilter", "relay", "download", "addr"], False) self.stop_node(1) diff --git a/test/functional/p2p_ping.py b/test/functional/p2p_ping.py index e00af88cc4..5f5fd3e104 100755 --- a/test/functional/p2p_ping.py +++ b/test/functional/p2p_ping.py @@ -7,13 +7,8 @@ import time -from test_framework.messages import ( - msg_pong, -) -from test_framework.mininode import ( - P2PInterface, - wait_until, -) +from test_framework.messages import msg_pong +from test_framework.mininode import P2PInterface from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal @@ -78,7 +73,7 @@ class PingPongTest(BitcoinTestFramework): with self.nodes[0].assert_debug_log(['pong peer=0: Nonce mismatch']): # mock time PING_INTERVAL ahead to trigger node into sending a ping self.mock_forward(PING_INTERVAL + 1) - wait_until(lambda: 'ping' in no_pong_node.last_message) + no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message) self.mock_forward(9) # Send the wrong pong no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce - 1)) @@ -93,27 +88,27 @@ class PingPongTest(BitcoinTestFramework): assert 'ping' not in no_pong_node.last_message # mock time PING_INTERVAL ahead to trigger node into sending a ping self.mock_forward(PING_INTERVAL + 1) - wait_until(lambda: 'ping' in no_pong_node.last_message) + no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message) ping_delay = 29 self.mock_forward(ping_delay) - wait_until(lambda: 'ping' in no_pong_node.last_message) + no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message) no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce)) self.check_peer_info(pingtime=ping_delay, minping=ping_delay, pingwait=None) self.log.info('Check that minping is decreased after a fast roundtrip') # mock time PING_INTERVAL ahead to trigger node into sending a ping self.mock_forward(PING_INTERVAL + 1) - wait_until(lambda: 'ping' in no_pong_node.last_message) + no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message) ping_delay = 9 self.mock_forward(ping_delay) - wait_until(lambda: 'ping' in no_pong_node.last_message) + no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message) no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce)) self.check_peer_info(pingtime=ping_delay, minping=ping_delay, pingwait=None) self.log.info('Check that peer is disconnected after ping timeout') assert 'ping' not in no_pong_node.last_message self.nodes[0].ping() - wait_until(lambda: 'ping' in no_pong_node.last_message) + no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message) with self.nodes[0].assert_debug_log(['ping timeout: 1201.000000s']): self.mock_forward(20 * 60 + 1) time.sleep(4) # peertimeout + 1 diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index 07811667a8..53d6e474d9 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -388,18 +388,22 @@ class P2PInterface(P2PConnection): # Connection helper methods - def wait_until(self, test_function, timeout=60): + def wait_until(self, test_function_in, *, timeout=60, check_connected=True): + def test_function(): + if check_connected: + assert self.is_connected + return test_function_in() + wait_until(test_function, timeout=timeout, lock=mininode_lock, timeout_factor=self.timeout_factor) def wait_for_disconnect(self, timeout=60): test_function = lambda: not self.is_connected - self.wait_until(test_function, timeout=timeout) + self.wait_until(test_function, timeout=timeout, check_connected=False) # Message receiving helper methods def wait_for_tx(self, txid, timeout=60): def test_function(): - assert self.is_connected if not self.last_message.get('tx'): return False return self.last_message['tx'].tx.rehash() == txid @@ -408,14 +412,12 @@ class P2PInterface(P2PConnection): def wait_for_block(self, blockhash, timeout=60): def test_function(): - assert self.is_connected return self.last_message.get("block") and self.last_message["block"].block.rehash() == blockhash self.wait_until(test_function, timeout=timeout) def wait_for_header(self, blockhash, timeout=60): def test_function(): - assert self.is_connected last_headers = self.last_message.get('headers') if not last_headers: return False @@ -425,7 +427,6 @@ class P2PInterface(P2PConnection): def wait_for_merkleblock(self, blockhash, timeout=60): def test_function(): - assert self.is_connected last_filtered_block = self.last_message.get('merkleblock') if not last_filtered_block: return False @@ -437,9 +438,7 @@ class P2PInterface(P2PConnection): """Waits for a getdata message. The object hashes in the inventory vector must match the provided hash_list.""" - def test_function(): - assert self.is_connected last_data = self.last_message.get("getdata") if not last_data: return False @@ -454,9 +453,7 @@ class P2PInterface(P2PConnection): value must be explicitly cleared before calling this method, or this will return immediately with success. TODO: change this method to take a hash value and only return true if the correct block header has been requested.""" - def test_function(): - assert self.is_connected return self.last_message.get("getheaders") self.wait_until(test_function, timeout=timeout) @@ -467,7 +464,6 @@ class P2PInterface(P2PConnection): raise NotImplementedError("wait_for_inv() will only verify the first inv object") def test_function(): - assert self.is_connected return self.last_message.get("inv") and \ self.last_message["inv"].inv[0].type == expected_inv[0].type and \ self.last_message["inv"].inv[0].hash == expected_inv[0].hash @@ -478,7 +474,7 @@ class P2PInterface(P2PConnection): def test_function(): return "verack" in self.last_message - self.wait_until(test_function, timeout=timeout) + self.wait_until(test_function, timeout=timeout, check_connected=False) # Message sending helper functions @@ -491,7 +487,6 @@ class P2PInterface(P2PConnection): self.send_message(msg_ping(nonce=self.ping_counter)) def test_function(): - assert self.is_connected return self.last_message.get("pong") and self.last_message["pong"].nonce == self.ping_counter self.wait_until(test_function, timeout=timeout) @@ -609,7 +604,11 @@ class P2PDataStore(P2PInterface): self.send_message(msg_block(block=b)) else: self.send_message(msg_headers([CBlockHeader(block) for block in blocks])) - self.wait_until(lambda: blocks[-1].sha256 in self.getdata_requests, timeout=timeout) + self.wait_until( + lambda: blocks[-1].sha256 in self.getdata_requests, + timeout=timeout, + check_connected=success, + ) if expect_disconnect: self.wait_for_disconnect(timeout=timeout) @@ -677,6 +676,6 @@ class P2PTxInvStore(P2PInterface): The mempool should mark unbroadcast=False for these transactions. """ # Wait until invs have been received (and getdatas sent) for each txid. - self.wait_until(lambda: set(self.tx_invs_received.keys()) == set([int(tx, 16) for tx in txns]), timeout) + self.wait_until(lambda: set(self.tx_invs_received.keys()) == set([int(tx, 16) for tx in txns]), timeout=timeout) # Flush messages and wait for the getdatas to be processed self.sync_with_ping() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 95c2b7c5ec..b090e93394 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -159,6 +159,7 @@ BASE_SCRIPTS = [ 'rpc_deprecated.py', 'wallet_disable.py', 'p2p_addr_relay.py', + 'p2p_getaddr_caching.py', 'p2p_getdata.py', 'rpc_net.py', 'wallet_keypool.py', |