From d9a8ce2749e7c5b1e81f73ddb18f3ceab588b5fe Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 22 Feb 2024 18:01:13 +0000 Subject: [PATCH 01/17] trivial: move GetSerializeSize away from Stream (Un)serialize functions done to reduce the number of conflicts associated with the unexpected forward decl, moved it just above WriteAutoBitSet and added comment to clarify reason. --- src/serialize.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/serialize.h b/src/serialize.h index c1c278fea596..e81b78d45edb 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -265,7 +265,6 @@ template inline void Unserialize(Stream& s, Span template inline void Serialize(Stream& s, bool a) { char f=a; ser_writedata8(s, f); } template inline void Unserialize(Stream& s, bool& a) { char f=ser_readdata8(s); a=f; } -template size_t GetSerializeSize(const T& t, int nVersion = 0); @@ -551,6 +550,9 @@ struct CFixedVarIntsBitSet void Serialize(Stream& s) const { WriteFixedVarIntsBitSet(s, vec, vec.size()); } }; +/* Forward declaration for WriteAutoBitSet */ +template size_t GetSerializeSize(const T& t, int nVersion = 0); + template void WriteAutoBitSet(Stream& s, const autobitset_t& item) { From 1d6aafea47f9ea3d4546ef5f8ab72b4c1cb6d359 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 26 Apr 2021 22:08:35 +0200 Subject: [PATCH 02/17] merge bitcoin#21817: Replace &foo[0] with foo.data() --- src/compressor.cpp | 2 +- src/dbwrapper.cpp | 7 +++---- src/key.cpp | 4 ++-- src/key.h | 2 +- src/pubkey.cpp | 2 +- src/pubkey.h | 4 ++-- src/random.cpp | 2 +- src/script/descriptor.cpp | 4 ++-- src/script/sigcache.cpp | 2 +- src/test/compress_tests.cpp | 10 +++++----- src/test/crypto_tests.cpp | 8 ++++---- src/test/script_tests.cpp | 14 +++++++------- src/wallet/crypter.cpp | 14 +++++++------- src/wallet/rpcdump.cpp | 8 ++++---- src/wallet/salvage.cpp | 4 ++-- src/wallet/wallet.cpp | 4 ++-- src/zmq/zmqpublishnotifier.cpp | 2 +- 17 files changed, 46 insertions(+), 47 deletions(-) diff --git a/src/compressor.cpp b/src/compressor.cpp index ef3135e7a5d0..a161c4286680 100644 --- a/src/compressor.cpp +++ b/src/compressor.cpp @@ -124,7 +124,7 @@ bool DecompressScript(CScript& script, unsigned int nSize, const CompressedScrip unsigned char vch[33] = {}; vch[0] = nSize - 2; memcpy(&vch[1], in.data(), 32); - CPubKey pubkey(&vch[0], &vch[33]); + CPubKey pubkey{vch}; if (!pubkey.Decompress()) return false; assert(pubkey.size() == 65); diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index 6a36d717efd3..644a44a3d689 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -223,10 +223,9 @@ const unsigned int CDBWrapper::OBFUSCATE_KEY_NUM_BYTES = 8; */ std::vector CDBWrapper::CreateObfuscateKey() const { - unsigned char buff[OBFUSCATE_KEY_NUM_BYTES]; - GetRandBytes(buff, OBFUSCATE_KEY_NUM_BYTES); - return std::vector(&buff[0], &buff[OBFUSCATE_KEY_NUM_BYTES]); - + std::vector ret(OBFUSCATE_KEY_NUM_BYTES); + GetRandBytes(ret.data(), OBFUSCATE_KEY_NUM_BYTES); + return ret; } bool CDBWrapper::IsEmpty() diff --git a/src/key.cpp b/src/key.cpp index bdf1d8851266..372dc8cada07 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -344,7 +344,7 @@ ECDHSecret CKey::ComputeBIP324ECDHSecret(const EllSwiftPubKey& their_ellswift, c bool CExtKey::Derive(CExtKey &out, unsigned int _nChild) const { out.nDepth = nDepth + 1; CKeyID id = key.GetPubKey().GetID(); - memcpy(&out.vchFingerprint[0], &id, 4); + memcpy(out.vchFingerprint, &id, 4); out.nChild = _nChild; return key.Derive(out.key, out.chaincode, _nChild, chaincode); } @@ -364,7 +364,7 @@ void CExtKey::SetSeed(Span seed) CExtPubKey CExtKey::Neuter() const { CExtPubKey ret; ret.nDepth = nDepth; - memcpy(&ret.vchFingerprint[0], &vchFingerprint[0], 4); + memcpy(ret.vchFingerprint, vchFingerprint, 4); ret.nChild = nChild; ret.pubkey = key.GetPubKey(); ret.chaincode = chaincode; diff --git a/src/key.h b/src/key.h index c994f6576ca3..f9723e4f22d1 100644 --- a/src/key.h +++ b/src/key.h @@ -178,7 +178,7 @@ struct CExtKey { friend bool operator==(const CExtKey& a, const CExtKey& b) { return a.nDepth == b.nDepth && - memcmp(&a.vchFingerprint[0], &b.vchFingerprint[0], sizeof(vchFingerprint)) == 0 && + memcmp(a.vchFingerprint, b.vchFingerprint, sizeof(vchFingerprint)) == 0 && a.nChild == b.nChild && a.chaincode == b.chaincode && a.key == b.key; diff --git a/src/pubkey.cpp b/src/pubkey.cpp index 0bf1751ffb67..c8c7e825dcfb 100644 --- a/src/pubkey.cpp +++ b/src/pubkey.cpp @@ -287,7 +287,7 @@ void CExtPubKey::Decode(const unsigned char code[BIP32_EXTKEY_SIZE]) { bool CExtPubKey::Derive(CExtPubKey &out, unsigned int _nChild) const { out.nDepth = nDepth + 1; CKeyID id = pubkey.GetID(); - memcpy(&out.vchFingerprint[0], &id, 4); + memcpy(out.vchFingerprint, &id, 4); out.nChild = _nChild; return pubkey.Derive(out.pubkey, out.chaincode, _nChild, chaincode); } diff --git a/src/pubkey.h b/src/pubkey.h index 010621d614ee..79dd66ed71c7 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -101,7 +101,7 @@ class CPubKey } //! Construct a public key from a byte vector. - explicit CPubKey(const std::vector& _vch) + explicit CPubKey(Span _vch) { Set(_vch.begin(), _vch.end()); } @@ -247,7 +247,7 @@ struct CExtPubKey { friend bool operator==(const CExtPubKey &a, const CExtPubKey &b) { return a.nDepth == b.nDepth && - memcmp(&a.vchFingerprint[0], &b.vchFingerprint[0], sizeof(vchFingerprint)) == 0 && + memcmp(a.vchFingerprint, b.vchFingerprint, sizeof(vchFingerprint)) == 0 && a.nChild == b.nChild && a.chaincode == b.chaincode && a.pubkey == b.pubkey; diff --git a/src/random.cpp b/src/random.cpp index ca61ebf02a44..da564309209a 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -635,7 +635,7 @@ std::vector FastRandomContext::randbytes(size_t len) if (requires_seed) RandomSeed(); std::vector ret(len); if (len > 0) { - rng.Keystream(&ret[0], len); + rng.Keystream(ret.data(), len); } return ret; } diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index a0b973ddeefa..3fc97f2998b1 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -930,7 +930,7 @@ std::unique_ptr InferScript(const CScript& script, ParseScriptCo TxoutType txntype = Solver(script, data); if (txntype == TxoutType::PUBKEY) { - CPubKey pubkey(data[0].begin(), data[0].end()); + CPubKey pubkey(data[0]); if (pubkey.IsValid()) { return std::make_unique(InferPubkey(pubkey, ctx, provider)); } @@ -946,7 +946,7 @@ std::unique_ptr InferScript(const CScript& script, ParseScriptCo if (txntype == TxoutType::MULTISIG) { std::vector> providers; for (size_t i = 1; i + 1 < data.size(); ++i) { - CPubKey pubkey(data[i].begin(), data[i].end()); + CPubKey pubkey(data[i]); providers.push_back(InferPubkey(pubkey, ctx, provider)); } return std::make_unique((int)data[0][0], std::move(providers)); diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp index 0b4835f3e2c3..b7831d61d031 100644 --- a/src/script/sigcache.cpp +++ b/src/script/sigcache.cpp @@ -46,7 +46,7 @@ class CSignatureCache ComputeEntry(uint256& entry, const uint256 &hash, const std::vector& vchSig, const CPubKey& pubkey) { CSHA256 hasher = m_salted_hasher; - hasher.Write(hash.begin(), 32).Write(&pubkey[0], pubkey.size()).Write(&vchSig[0], vchSig.size()).Finalize(entry.begin()); + hasher.Write(hash.begin(), 32).Write(pubkey.data(), pubkey.size()).Write(vchSig.data(), vchSig.size()).Finalize(entry.begin()); } bool diff --git a/src/test/compress_tests.cpp b/src/test/compress_tests.cpp index 7b661a0d1da9..b7ba46bd3035 100644 --- a/src/test/compress_tests.cpp +++ b/src/test/compress_tests.cpp @@ -79,7 +79,7 @@ BOOST_AUTO_TEST_CASE(compress_script_to_ckey_id) // Check compressed script BOOST_CHECK_EQUAL(out.size(), 21U); BOOST_CHECK_EQUAL(out[0], 0x00); - BOOST_CHECK_EQUAL(memcmp(&out[1], &script[3], 20), 0); // compare the 20 relevant chars of the CKeyId in the script + BOOST_CHECK_EQUAL(memcmp(out.data() + 1, script.data() + 3, 20), 0); // compare the 20 relevant chars of the CKeyId in the script } BOOST_AUTO_TEST_CASE(compress_script_to_cscript_id) @@ -96,7 +96,7 @@ BOOST_AUTO_TEST_CASE(compress_script_to_cscript_id) // Check compressed script BOOST_CHECK_EQUAL(out.size(), 21U); BOOST_CHECK_EQUAL(out[0], 0x01); - BOOST_CHECK_EQUAL(memcmp(&out[1], &script[2], 20), 0); // compare the 20 relevant chars of the CScriptId in the script + BOOST_CHECK_EQUAL(memcmp(out.data() + 1, script.data() + 2, 20), 0); // compare the 20 relevant chars of the CScriptId in the script } BOOST_AUTO_TEST_CASE(compress_script_to_compressed_pubkey_id) @@ -113,8 +113,8 @@ BOOST_AUTO_TEST_CASE(compress_script_to_compressed_pubkey_id) // Check compressed script BOOST_CHECK_EQUAL(out.size(), 33U); - BOOST_CHECK_EQUAL(memcmp(&out[0], &script[1], 1), 0); - BOOST_CHECK_EQUAL(memcmp(&out[1], &script[2], 32), 0); // compare the 32 chars of the compressed CPubKey + BOOST_CHECK_EQUAL(memcmp(out.data(), script.data() + 1, 1), 0); + BOOST_CHECK_EQUAL(memcmp(out.data() + 1, script.data() + 2, 32), 0); // compare the 32 chars of the compressed CPubKey } BOOST_AUTO_TEST_CASE(compress_script_to_uncompressed_pubkey_id) @@ -130,7 +130,7 @@ BOOST_AUTO_TEST_CASE(compress_script_to_uncompressed_pubkey_id) // Check compressed script BOOST_CHECK_EQUAL(out.size(), 33U); - BOOST_CHECK_EQUAL(memcmp(&out[1], &script[2], 32), 0); // first 32 chars of CPubKey are copied into out[1:] + BOOST_CHECK_EQUAL(memcmp(out.data() + 1, script.data() + 2, 32), 0); // first 32 chars of CPubKey are copied into out[1:] BOOST_CHECK_EQUAL(out[0], 0x04 | (script[65] & 0x01)); // least significant bit (lsb) of last char of pubkey is mapped into out[0] } diff --git a/src/test/crypto_tests.cpp b/src/test/crypto_tests.cpp index 178dd5a1d649..e262b40d748d 100644 --- a/src/test/crypto_tests.cpp +++ b/src/test/crypto_tests.cpp @@ -34,7 +34,7 @@ static void TestVector(const Hasher &h, const In &in, const Out &out) { hash.resize(out.size()); { // Test that writing the whole input string at once works. - Hasher(h).Write((unsigned char*)&in[0], in.size()).Finalize(&hash[0]); + Hasher(h).Write((const uint8_t*)in.data(), in.size()).Finalize(hash.data()); BOOST_CHECK(hash == out); } for (int i=0; i<32; i++) { @@ -43,15 +43,15 @@ static void TestVector(const Hasher &h, const In &in, const Out &out) { size_t pos = 0; while (pos < in.size()) { size_t len = InsecureRandRange((in.size() - pos + 1) / 2 + 1); - hasher.Write((unsigned char*)&in[pos], len); + hasher.Write((const uint8_t*)in.data() + pos, len); pos += len; if (pos > 0 && pos + 2 * out.size() > in.size() && pos < in.size()) { // Test that writing the rest at once to a copy of a hasher works. - Hasher(hasher).Write((unsigned char*)&in[pos], in.size() - pos).Finalize(&hash[0]); + Hasher(hasher).Write((const uint8_t*)in.data() + pos, in.size() - pos).Finalize(hash.data()); BOOST_CHECK(hash == out); } } - hasher.Finalize(&hash[0]); + hasher.Finalize(hash.data()); BOOST_CHECK(hash == out); } } diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index bd062b15ceab..a24d313e5e65 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -147,7 +147,7 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, uint32_t flag uint32_t libconsensus_flags{flags & dashconsensus_SCRIPT_FLAGS_VERIFY_ALL}; if (libconsensus_flags == flags) { int expectedSuccessCode = expect ? 1 : 0; - BOOST_CHECK_MESSAGE(dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), (const unsigned char*)&stream[0], stream.size(), 0, libconsensus_flags, nullptr) == expectedSuccessCode, message); + BOOST_CHECK_MESSAGE(dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), stream.data(), stream.size(), 0, libconsensus_flags, nullptr) == expectedSuccessCode, message); } #endif } @@ -212,7 +212,7 @@ struct KeyData pubkey0 = key0.GetPubKey(); pubkey0H = key0.GetPubKey(); pubkey0C = key0C.GetPubKey(); - *const_cast(&pubkey0H[0]) = 0x06 | (pubkey0H[64] & 1); + *const_cast(pubkey0H.data()) = 0x06 | (pubkey0H[64] & 1); key1.Set(vchKey1, vchKey1 + 32, false); key1C.Set(vchKey1, vchKey1 + 32, true); @@ -1421,7 +1421,7 @@ BOOST_AUTO_TEST_CASE(dashconsensus_verify_script_returns_true) stream << spendTx; dashconsensus_error err; - int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), (const unsigned char*)&stream[0], stream.size(), nIn, libconsensus_flags, &err); + int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), stream.data(), stream.size(), nIn, libconsensus_flags, &err); BOOST_CHECK_EQUAL(result, 1); BOOST_CHECK_EQUAL(err, dashconsensus_ERR_OK); } @@ -1443,7 +1443,7 @@ BOOST_AUTO_TEST_CASE(dashconsensus_verify_script_tx_index_err) stream << spendTx; dashconsensus_error err; - int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), (const unsigned char*)&stream[0], stream.size(), nIn, libconsensus_flags, &err); + int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), stream.data(), stream.size(), nIn, libconsensus_flags, &err); BOOST_CHECK_EQUAL(result, 0); BOOST_CHECK_EQUAL(err, dashconsensus_ERR_TX_INDEX); } @@ -1465,7 +1465,7 @@ BOOST_AUTO_TEST_CASE(dashconsensus_verify_script_tx_size) stream << spendTx; dashconsensus_error err; - int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), (const unsigned char*)&stream[0], stream.size() * 2, nIn, libconsensus_flags, &err); + int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), stream.data(), stream.size() * 2, nIn, libconsensus_flags, &err); BOOST_CHECK_EQUAL(result, 0); BOOST_CHECK_EQUAL(err, dashconsensus_ERR_TX_SIZE_MISMATCH); } @@ -1487,7 +1487,7 @@ BOOST_AUTO_TEST_CASE(dashconsensus_verify_script_tx_serialization) stream << 0xffffffff; dashconsensus_error err; - int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), (const unsigned char*)&stream[0], stream.size(), nIn, libconsensus_flags, &err); + int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), stream.data(), stream.size(), nIn, libconsensus_flags, &err); BOOST_CHECK_EQUAL(result, 0); BOOST_CHECK_EQUAL(err, dashconsensus_ERR_TX_DESERIALIZE); } @@ -1509,7 +1509,7 @@ BOOST_AUTO_TEST_CASE(dashconsensus_verify_script_invalid_flags) stream << spendTx; dashconsensus_error err; - int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), (const unsigned char*)&stream[0], stream.size(), nIn, libconsensus_flags, &err); + int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), stream.data(), stream.size(), nIn, libconsensus_flags, &err); BOOST_CHECK_EQUAL(result, 0); BOOST_CHECK_EQUAL(err, dashconsensus_ERR_INVALID_FLAGS); } diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index 627da120eb9d..93801b24e533 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -78,7 +78,7 @@ bool CCrypter::Encrypt(const CKeyingMaterial& vchPlaintext, std::vector& vchCiphertext, CKeyingM vchPlaintext.resize(nLen); AES256CBCDecrypt dec(vchKey.data(), vchIV.data(), true); - nLen = dec.Decrypt(vchCiphertext.data(), vchCiphertext.size(), &vchPlaintext[0]); + nLen = dec.Decrypt(vchCiphertext.data(), vchCiphertext.size(), vchPlaintext.data()); if(nLen == 0) return false; vchPlaintext.resize(nLen); @@ -127,8 +127,8 @@ bool EncryptAES256(const SecureString& sKey, const SecureString& sPlaintext, con // n + AES_BLOCKSIZE bytes sCiphertext.resize(sPlaintext.size() + AES_BLOCKSIZE); - AES256CBCEncrypt enc((const unsigned char*) &sKey[0], (const unsigned char*) &sIV[0], true); - size_t nLen = enc.Encrypt((const unsigned char*) &sPlaintext[0], sPlaintext.size(), (unsigned char*) &sCiphertext[0]); + AES256CBCEncrypt enc((const unsigned char*)sKey.data(), (const unsigned char*)sIV.data(), true); + size_t nLen = enc.Encrypt((const unsigned char*)sPlaintext.data(), sPlaintext.size(), (unsigned char*)&sCiphertext[0]); if(nLen < sPlaintext.size()) return false; sCiphertext.resize(nLen); @@ -143,7 +143,7 @@ bool DecryptSecret(const CKeyingMaterial& vMasterKey, const std::vector data(ParseHex(request.params[0].get_str())); - CPubKey pubKey(data.begin(), data.end()); + CPubKey pubKey(data); if (!pubKey.IsFullyValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key"); @@ -1085,7 +1085,7 @@ static std::string RecurseImportData(const CScript& script, ImportData& import_d switch (script_type) { case TxoutType::PUBKEY: { - CPubKey pubkey(solverdata[0].begin(), solverdata[0].end()); + CPubKey pubkey(solverdata[0]); import_data.used_keys.emplace(pubkey.GetID(), false); return ""; } @@ -1106,7 +1106,7 @@ static std::string RecurseImportData(const CScript& script, ImportData& import_d } case TxoutType::MULTISIG: { for (size_t i = 1; i + 1< solverdata.size(); ++i) { - CPubKey pubkey(solverdata[i].begin(), solverdata[i].end()); + CPubKey pubkey(solverdata[i]); import_data.used_keys.emplace(pubkey.GetID(), false); } return ""; @@ -1177,7 +1177,7 @@ static UniValue ProcessImportLegacy(ImportData& import_data, std::mapput(ptxn, &datKey, &datValue, DB_NOOVERWRITE); if (ret2 > 0) fSuccess = false; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 833d6ec924b7..a1ff8f11c17c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -607,12 +607,12 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) CKeyingMaterial _vMasterKey; _vMasterKey.resize(WALLET_CRYPTO_KEY_SIZE); - GetStrongRandBytes(&_vMasterKey[0], WALLET_CRYPTO_KEY_SIZE); + GetStrongRandBytes(_vMasterKey.data(), WALLET_CRYPTO_KEY_SIZE); CMasterKey kMasterKey; kMasterKey.vchSalt.resize(WALLET_CRYPTO_SALT_SIZE); - GetStrongRandBytes(&kMasterKey.vchSalt[0], WALLET_CRYPTO_SALT_SIZE); + GetStrongRandBytes(kMasterKey.vchSalt.data(), WALLET_CRYPTO_SALT_SIZE); CCrypter crypter; int64_t nStartTime = GetTimeMillis(); diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index c08ae206fec6..5a114bca816a 100644 --- a/src/zmq/zmqpublishnotifier.cpp +++ b/src/zmq/zmqpublishnotifier.cpp @@ -184,7 +184,7 @@ bool CZMQAbstractPublishNotifier::SendZmqMessage(const char *command, const void /* send three parts, command & data & a LE 4byte sequence number */ unsigned char msgseq[sizeof(uint32_t)]; - WriteLE32(&msgseq[0], nSequence); + WriteLE32(msgseq, nSequence); int rc = zmq_send_multipart(psocket, command, strlen(command), data, size, msgseq, (size_t)sizeof(uint32_t), nullptr); if (rc == -1) return false; From d0b4e560a6467c895ecf1910af012ff167cadcd2 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 22 Feb 2024 20:10:01 +0000 Subject: [PATCH 03/17] merge bitcoin#21966: Remove double serialization; use software encoder for fee estimation continuation of f946c68f8 from dash#4197 --- src/Makefile.test.include | 1 + src/compat/assumptions.h | 5 -- src/policy/fees.cpp | 45 ++++++++---- src/serialize.h | 32 --------- src/test/fuzz/float.cpp | 35 ++++------ src/test/fuzz/integer.cpp | 4 -- src/test/fuzz/util.h | 4 -- src/test/serfloat_tests.cpp | 129 +++++++++++++++++++++++++++++++++++ src/test/serialize_tests.cpp | 86 ----------------------- 9 files changed, 177 insertions(+), 164 deletions(-) create mode 100644 src/test/serfloat_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 16f3268d7641..838bad0f7264 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -151,6 +151,7 @@ BITCOIN_TESTS =\ test/script_tests.cpp \ test/script_standard_tests.cpp \ test/scriptnum_tests.cpp \ + test/serfloat_tests.cpp \ test/serialize_tests.cpp \ test/settings_tests.cpp \ test/sighash_tests.cpp \ diff --git a/src/compat/assumptions.h b/src/compat/assumptions.h index f01662c31c50..0241f63d69f0 100644 --- a/src/compat/assumptions.h +++ b/src/compat/assumptions.h @@ -36,11 +36,6 @@ static_assert(std::numeric_limits::is_iec559, "IEEE 754 double assumed") // Example(s): Everywhere :-) static_assert(std::numeric_limits::digits == 8, "8-bit byte assumed"); -// Assumption: We assume floating-point widths. -// Example(s): Type punning in serialization code (ser_{float,double}_to_uint{32,64}). -static_assert(sizeof(float) == 4, "32-bit float assumed"); -static_assert(sizeof(double) == 8, "64-bit double assumed"); - // Assumption: We assume integer widths. // Example(s): GetSizeOfCompactSize and WriteCompactSize in the serialization // code. diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 7ed57cfec6b4..d961f3892729 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include static const char* FEE_ESTIMATES_FILENAME = "fee_estimates.dat"; @@ -25,6 +26,26 @@ std::string StringForFeeEstimateHorizon(FeeEstimateHorizon horizon) } // no default case, so the compiler can warn about missing cases assert(false); } + +namespace { + +struct EncodedDoubleFormatter +{ + template void Ser(Stream &s, double v) + { + s << EncodeDouble(v); + } + + template void Unser(Stream& s, double& v) + { + uint64_t encoded; + s >> encoded; + v = DecodeDouble(encoded); + } +}; + +} // namespace + /** * We will instantiate an instance of this class to track transactions that were * included in a block. We will lump transactions into a bucket according to their @@ -355,12 +376,12 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal, void TxConfirmStats::Write(CAutoFile& fileout) const { - fileout << decay; + fileout << Using(decay); fileout << scale; - fileout << m_feerate_avg; - fileout << txCtAvg; - fileout << confAvg; - fileout << failAvg; + fileout << Using>(m_feerate_avg); + fileout << Using>(txCtAvg); + fileout << Using>>(confAvg); + fileout << Using>>(failAvg); } void TxConfirmStats::Read(CAutoFile& filein, int nFileVersion, size_t numBuckets) @@ -371,7 +392,7 @@ void TxConfirmStats::Read(CAutoFile& filein, int nFileVersion, size_t numBuckets size_t maxConfirms, maxPeriods; // The current version will store the decay with each individual TxConfirmStats and also keep a scale factor - filein >> decay; + filein >> Using(decay); if (decay <= 0 || decay >= 1) { throw std::runtime_error("Corrupt estimates file. Decay must be between 0 and 1 (non-inclusive)"); } @@ -380,15 +401,15 @@ void TxConfirmStats::Read(CAutoFile& filein, int nFileVersion, size_t numBuckets throw std::runtime_error("Corrupt estimates file. Scale must be non-zero"); } - filein >> m_feerate_avg; + filein >> Using>(m_feerate_avg); if (m_feerate_avg.size() != numBuckets) { throw std::runtime_error("Corrupt estimates file. Mismatch in feerate average bucket count"); } - filein >> txCtAvg; + filein >> Using>(txCtAvg); if (txCtAvg.size() != numBuckets) { throw std::runtime_error("Corrupt estimates file. Mismatch in tx count bucket count"); } - filein >> confAvg; + filein >> Using>>(confAvg); maxPeriods = confAvg.size(); maxConfirms = scale * maxPeriods; @@ -401,7 +422,7 @@ void TxConfirmStats::Read(CAutoFile& filein, int nFileVersion, size_t numBuckets } } - filein >> failAvg; + filein >> Using>>(failAvg); if (maxPeriods != failAvg.size()) { throw std::runtime_error("Corrupt estimates file. Mismatch in confirms tracked for failures"); } @@ -889,7 +910,7 @@ bool CBlockPolicyEstimator::Write(CAutoFile& fileout) const else { fileout << historicalFirst << historicalBest; } - fileout << buckets; + fileout << Using>(buckets); feeStats->Write(fileout); shortStats->Write(fileout); longStats->Write(fileout); @@ -925,7 +946,7 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) throw std::runtime_error("Corrupt estimates file. Historical block range for estimates is invalid"); } std::vector fileBuckets; - filein >> fileBuckets; + filein >> Using>(fileBuckets); size_t numBuckets = fileBuckets.size(); if (numBuckets <= 1 || numBuckets > 1000) { throw std::runtime_error("Corrupt estimates file. Must have between 2 and 1000 feerate buckets"); diff --git a/src/serialize.h b/src/serialize.h index e81b78d45edb..623a1b838c29 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -126,34 +126,6 @@ template inline uint64_t ser_readdata64(Stream &s) s.read((char*)&obj, 8); return le64toh(obj); } -inline uint64_t ser_double_to_uint64(double x) -{ - uint64_t tmp; - std::memcpy(&tmp, &x, sizeof(x)); - static_assert(sizeof(tmp) == sizeof(x), "double and uint64_t assumed to have the same size"); - return tmp; -} -inline uint32_t ser_float_to_uint32(float x) -{ - uint32_t tmp; - std::memcpy(&tmp, &x, sizeof(x)); - static_assert(sizeof(tmp) == sizeof(x), "float and uint32_t assumed to have the same size"); - return tmp; -} -inline double ser_uint64_to_double(uint64_t y) -{ - double tmp; - std::memcpy(&tmp, &y, sizeof(y)); - static_assert(sizeof(tmp) == sizeof(y), "double and uint64_t assumed to have the same size"); - return tmp; -} -inline float ser_uint32_to_float(uint32_t y) -{ - float tmp; - std::memcpy(&tmp, &y, sizeof(y)); - static_assert(sizeof(tmp) == sizeof(y), "float and uint32_t assumed to have the same size"); - return tmp; -} ///////////////////////////////////////////////////////////////// @@ -238,8 +210,6 @@ template inline void Serialize(Stream& s, int32_t a ) { ser_wri template inline void Serialize(Stream& s, uint32_t a) { ser_writedata32(s, a); } template inline void Serialize(Stream& s, int64_t a ) { ser_writedata64(s, a); } template inline void Serialize(Stream& s, uint64_t a) { ser_writedata64(s, a); } -template inline void Serialize(Stream& s, float a ) { ser_writedata32(s, ser_float_to_uint32(a)); } -template inline void Serialize(Stream& s, double a ) { ser_writedata64(s, ser_double_to_uint64(a)); } template inline void Serialize(Stream& s, const char (&a)[N]) { s.write(a, N); } template inline void Serialize(Stream& s, const unsigned char (&a)[N]) { s.write(CharCast(a), N); } template inline void Serialize(Stream& s, const Span& span) { s.write(CharCast(span.data()), span.size()); } @@ -256,8 +226,6 @@ template inline void Unserialize(Stream& s, int32_t& a ) { a = template inline void Unserialize(Stream& s, uint32_t& a) { a = ser_readdata32(s); } template inline void Unserialize(Stream& s, int64_t& a ) { a = ser_readdata64(s); } template inline void Unserialize(Stream& s, uint64_t& a) { a = ser_readdata64(s); } -template inline void Unserialize(Stream& s, float& a ) { a = ser_uint32_to_float(ser_readdata32(s)); } -template inline void Unserialize(Stream& s, double& a ) { a = ser_uint64_to_double(ser_readdata64(s)); } template inline void Unserialize(Stream& s, char (&a)[N]) { s.read(a, N); } template inline void Unserialize(Stream& s, unsigned char (&a)[N]) { s.read(CharCast(a), N); } template inline void Unserialize(Stream& s, Span& span) { s.read(CharCast(span.data()), span.size()); } diff --git a/src/test/fuzz/float.cpp b/src/test/fuzz/float.cpp index d18a87d17742..adef66a3ee01 100644 --- a/src/test/fuzz/float.cpp +++ b/src/test/fuzz/float.cpp @@ -3,14 +3,14 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include -#include -#include #include #include +#include #include #include -#include +#include +#include FUZZ_TARGET(float) { @@ -19,24 +19,17 @@ FUZZ_TARGET(float) { const double d = fuzzed_data_provider.ConsumeFloatingPoint(); (void)memusage::DynamicUsage(d); - assert(ser_uint64_to_double(ser_double_to_uint64(d)) == d); - CDataStream stream(SER_NETWORK, INIT_PROTO_VERSION); - stream << d; - double d_deserialized; - stream >> d_deserialized; - assert(d == d_deserialized); - } - - { - const float f = fuzzed_data_provider.ConsumeFloatingPoint(); - (void)memusage::DynamicUsage(f); - assert(ser_uint32_to_float(ser_float_to_uint32(f)) == f); - - CDataStream stream(SER_NETWORK, INIT_PROTO_VERSION); - stream << f; - float f_deserialized; - stream >> f_deserialized; - assert(f == f_deserialized); + uint64_t encoded = EncodeDouble(d); + if constexpr (std::numeric_limits::is_iec559) { + if (!std::isnan(d)) { + uint64_t encoded_in_memory; + std::copy((const unsigned char*)&d, (const unsigned char*)(&d + 1), (unsigned char*)&encoded_in_memory); + assert(encoded_in_memory == encoded); + } + } + double d_deserialized = DecodeDouble(encoded); + assert(std::isnan(d) == std::isnan(d_deserialized)); + assert(std::isnan(d) || d == d_deserialized); } } diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index 78813ec9b58f..6a4d7896b082 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -117,10 +117,6 @@ FUZZ_TARGET_INIT(integer, initialize_integer) assert(dynamic_usage == incremental_dynamic_usage * i64s.size()); } (void)MillisToTimeval(i64); - const double d = ser_uint64_to_double(u64); - assert(ser_double_to_uint64(d) == u64); - const float f = ser_uint32_to_float(u32); - assert(ser_float_to_uint32(f) == u32); (void)SighashToStr(uch); (void)SipHashUint256(u64, u64, u256); (void)SipHashUint256Extra(u64, u64, u256, u32); diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index ea45eb64490e..d16014148e7d 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -507,8 +507,6 @@ void WriteToStream(FuzzedDataProvider& fuzzed_data_provider, Stream& stream) noe WRITE_TO_STREAM_CASE(uint32_t, fuzzed_data_provider.ConsumeIntegral()), WRITE_TO_STREAM_CASE(int64_t, fuzzed_data_provider.ConsumeIntegral()), WRITE_TO_STREAM_CASE(uint64_t, fuzzed_data_provider.ConsumeIntegral()), - WRITE_TO_STREAM_CASE(float, fuzzed_data_provider.ConsumeFloatingPoint()), - WRITE_TO_STREAM_CASE(double, fuzzed_data_provider.ConsumeFloatingPoint()), WRITE_TO_STREAM_CASE(std::string, fuzzed_data_provider.ConsumeRandomLengthString(32)), WRITE_TO_STREAM_CASE(std::vector, ConsumeRandomLengthIntegralVector(fuzzed_data_provider))); } catch (const std::ios_base::failure&) { @@ -539,8 +537,6 @@ void ReadFromStream(FuzzedDataProvider& fuzzed_data_provider, Stream& stream) no READ_FROM_STREAM_CASE(uint32_t), READ_FROM_STREAM_CASE(int64_t), READ_FROM_STREAM_CASE(uint64_t), - READ_FROM_STREAM_CASE(float), - READ_FROM_STREAM_CASE(double), READ_FROM_STREAM_CASE(std::string), READ_FROM_STREAM_CASE(std::vector)); } catch (const std::ios_base::failure&) { diff --git a/src/test/serfloat_tests.cpp b/src/test/serfloat_tests.cpp new file mode 100644 index 000000000000..54e07b0f61b3 --- /dev/null +++ b/src/test/serfloat_tests.cpp @@ -0,0 +1,129 @@ +// Copyright (c) 2014-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 +#include +#include +#include +#include + +#include + +#include +#include + +BOOST_FIXTURE_TEST_SUITE(serfloat_tests, BasicTestingSetup) + +namespace { + +uint64_t TestDouble(double f) { + uint64_t i = EncodeDouble(f); + double f2 = DecodeDouble(i); + if (std::isnan(f)) { + // NaN is not guaranteed to round-trip exactly. + BOOST_CHECK(std::isnan(f2)); + } else { + // Everything else is. + BOOST_CHECK(!std::isnan(f2)); + uint64_t i2 = EncodeDouble(f2); + BOOST_CHECK_EQUAL(f, f2); + BOOST_CHECK_EQUAL(i, i2); + } + return i; +} + +} // namespace + +BOOST_AUTO_TEST_CASE(double_serfloat_tests) { + BOOST_CHECK_EQUAL(TestDouble(0.0), 0); + BOOST_CHECK_EQUAL(TestDouble(-0.0), 0x8000000000000000); + BOOST_CHECK_EQUAL(TestDouble(std::numeric_limits::infinity()), 0x7ff0000000000000); + BOOST_CHECK_EQUAL(TestDouble(-std::numeric_limits::infinity()), 0xfff0000000000000); + BOOST_CHECK_EQUAL(TestDouble(0.5), 0x3fe0000000000000ULL); + BOOST_CHECK_EQUAL(TestDouble(1.0), 0x3ff0000000000000ULL); + BOOST_CHECK_EQUAL(TestDouble(2.0), 0x4000000000000000ULL); + BOOST_CHECK_EQUAL(TestDouble(4.0), 0x4010000000000000ULL); + BOOST_CHECK_EQUAL(TestDouble(785.066650390625), 0x4088888880000000ULL); + + // Roundtrip test on IEC559-compatible systems + if (std::numeric_limits::is_iec559) { + BOOST_CHECK_EQUAL(sizeof(double), 8); + BOOST_CHECK_EQUAL(sizeof(uint64_t), 8); + // Test extreme values + TestDouble(std::numeric_limits::min()); + TestDouble(-std::numeric_limits::min()); + TestDouble(std::numeric_limits::max()); + TestDouble(-std::numeric_limits::max()); + TestDouble(std::numeric_limits::lowest()); + TestDouble(-std::numeric_limits::lowest()); + TestDouble(std::numeric_limits::quiet_NaN()); + TestDouble(-std::numeric_limits::quiet_NaN()); + TestDouble(std::numeric_limits::signaling_NaN()); + TestDouble(-std::numeric_limits::signaling_NaN()); + TestDouble(std::numeric_limits::denorm_min()); + TestDouble(-std::numeric_limits::denorm_min()); + // Test exact encoding: on currently supported platforms, EncodeDouble + // should produce exactly the same as the in-memory representation for non-NaN. + for (int j = 0; j < 1000; ++j) { + // Iterate over 9 specific bits exhaustively; the others are chosen randomly. + // These specific bits are the sign bit, and the 2 top and bottom bits of + // exponent and mantissa in the IEEE754 binary64 format. + for (int x = 0; x < 512; ++x) { + uint64_t v = InsecureRandBits(64); + v &= ~(uint64_t{1} << 0); + if (x & 1) v |= (uint64_t{1} << 0); + v &= ~(uint64_t{1} << 1); + if (x & 2) v |= (uint64_t{1} << 1); + v &= ~(uint64_t{1} << 50); + if (x & 4) v |= (uint64_t{1} << 50); + v &= ~(uint64_t{1} << 51); + if (x & 8) v |= (uint64_t{1} << 51); + v &= ~(uint64_t{1} << 52); + if (x & 16) v |= (uint64_t{1} << 52); + v &= ~(uint64_t{1} << 53); + if (x & 32) v |= (uint64_t{1} << 53); + v &= ~(uint64_t{1} << 61); + if (x & 64) v |= (uint64_t{1} << 61); + v &= ~(uint64_t{1} << 62); + if (x & 128) v |= (uint64_t{1} << 62); + v &= ~(uint64_t{1} << 63); + if (x & 256) v |= (uint64_t{1} << 63); + double f; + memcpy(&f, &v, 8); + uint64_t v2 = TestDouble(f); + if (!std::isnan(f)) BOOST_CHECK_EQUAL(v, v2); + } + } + } +} + +/* +Python code to generate the below hashes: + + def reversed_hex(x): + return binascii.hexlify(''.join(reversed(x))) + def dsha256(x): + return hashlib.sha256(hashlib.sha256(x).digest()).digest() + + reversed_hex(dsha256(''.join(struct.pack('> val; + double j = DecodeDouble(val); + BOOST_CHECK_MESSAGE(i == j, "decoded:" << j << " expected:" << i); + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index f1332d4a659a..a1097d584a14 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -70,8 +70,6 @@ BOOST_AUTO_TEST_CASE(sizes) BOOST_CHECK_EQUAL(sizeof(uint32_t), GetSerializeSize(uint32_t(0), 0)); BOOST_CHECK_EQUAL(sizeof(int64_t), GetSerializeSize(int64_t(0), 0)); BOOST_CHECK_EQUAL(sizeof(uint64_t), GetSerializeSize(uint64_t(0), 0)); - BOOST_CHECK_EQUAL(sizeof(float), GetSerializeSize(float(0), 0)); - BOOST_CHECK_EQUAL(sizeof(double), GetSerializeSize(double(0), 0)); // Bool is serialized as char BOOST_CHECK_EQUAL(sizeof(char), GetSerializeSize(bool(0), 0)); @@ -85,93 +83,9 @@ BOOST_AUTO_TEST_CASE(sizes) BOOST_CHECK_EQUAL(GetSerializeSize(uint32_t(0), 0), 4U); BOOST_CHECK_EQUAL(GetSerializeSize(int64_t(0), 0), 8U); BOOST_CHECK_EQUAL(GetSerializeSize(uint64_t(0), 0), 8U); - BOOST_CHECK_EQUAL(GetSerializeSize(float(0), 0), 4U); - BOOST_CHECK_EQUAL(GetSerializeSize(double(0), 0), 8U); BOOST_CHECK_EQUAL(GetSerializeSize(bool(0), 0), 1U); } -BOOST_AUTO_TEST_CASE(floats_conversion) -{ - // Choose values that map unambiguously to binary floating point to avoid - // rounding issues at the compiler side. - BOOST_CHECK_EQUAL(ser_uint32_to_float(0x00000000), 0.0F); - BOOST_CHECK_EQUAL(ser_uint32_to_float(0x3f000000), 0.5F); - BOOST_CHECK_EQUAL(ser_uint32_to_float(0x3f800000), 1.0F); - BOOST_CHECK_EQUAL(ser_uint32_to_float(0x40000000), 2.0F); - BOOST_CHECK_EQUAL(ser_uint32_to_float(0x40800000), 4.0F); - BOOST_CHECK_EQUAL(ser_uint32_to_float(0x44444444), 785.066650390625F); - - BOOST_CHECK_EQUAL(ser_float_to_uint32(0.0F), 0x00000000U); - BOOST_CHECK_EQUAL(ser_float_to_uint32(0.5F), 0x3f000000U); - BOOST_CHECK_EQUAL(ser_float_to_uint32(1.0F), 0x3f800000U); - BOOST_CHECK_EQUAL(ser_float_to_uint32(2.0F), 0x40000000U); - BOOST_CHECK_EQUAL(ser_float_to_uint32(4.0F), 0x40800000U); - BOOST_CHECK_EQUAL(ser_float_to_uint32(785.066650390625F), 0x44444444U); -} - -BOOST_AUTO_TEST_CASE(doubles_conversion) -{ - // Choose values that map unambiguously to binary floating point to avoid - // rounding issues at the compiler side. - BOOST_CHECK_EQUAL(ser_uint64_to_double(0x0000000000000000ULL), 0.0); - BOOST_CHECK_EQUAL(ser_uint64_to_double(0x3fe0000000000000ULL), 0.5); - BOOST_CHECK_EQUAL(ser_uint64_to_double(0x3ff0000000000000ULL), 1.0); - BOOST_CHECK_EQUAL(ser_uint64_to_double(0x4000000000000000ULL), 2.0); - BOOST_CHECK_EQUAL(ser_uint64_to_double(0x4010000000000000ULL), 4.0); - BOOST_CHECK_EQUAL(ser_uint64_to_double(0x4088888880000000ULL), 785.066650390625); - - BOOST_CHECK_EQUAL(ser_double_to_uint64(0.0), 0x0000000000000000ULL); - BOOST_CHECK_EQUAL(ser_double_to_uint64(0.5), 0x3fe0000000000000ULL); - BOOST_CHECK_EQUAL(ser_double_to_uint64(1.0), 0x3ff0000000000000ULL); - BOOST_CHECK_EQUAL(ser_double_to_uint64(2.0), 0x4000000000000000ULL); - BOOST_CHECK_EQUAL(ser_double_to_uint64(4.0), 0x4010000000000000ULL); - BOOST_CHECK_EQUAL(ser_double_to_uint64(785.066650390625), 0x4088888880000000ULL); -} -/* -Python code to generate the below hashes: - - def reversed_hex(x): - return binascii.hexlify(''.join(reversed(x))) - def dsha256(x): - return hashlib.sha256(hashlib.sha256(x).digest()).digest() - - reversed_hex(dsha256(''.join(struct.pack('> j; - BOOST_CHECK_MESSAGE(i == j, "decoded:" << j << " expected:" << i); - } -} - -BOOST_AUTO_TEST_CASE(doubles) -{ - CDataStream ss(SER_DISK, 0); - // encode - for (int i = 0; i < 1000; i++) { - ss << double(i); - } - BOOST_CHECK(Hash(ss) == uint256S("43d0c82591953c4eafe114590d392676a01585d25b25d433557f0d7878b23f96")); - - // decode - for (int i = 0; i < 1000; i++) { - double j; - ss >> j; - BOOST_CHECK_MESSAGE(i == j, "decoded:" << j << " expected:" << i); - } -} - BOOST_AUTO_TEST_CASE(varints) { // encode From 0a08dbf3f46874334ec3779b14dae774816a2c94 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 1 May 2021 13:49:37 +0200 Subject: [PATCH 04/17] merge bitcoin#21824: Replace deprecated char with uint8_t in serialization --- src/index/base.cpp | 2 +- src/index/blockfilterindex.cpp | 15 +++++++-------- src/index/coinstatsindex.cpp | 10 +++++----- src/index/txindex.cpp | 17 ++++++++--------- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index d58c3e22f000..f5ccd36344dc 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -13,7 +13,7 @@ #include // For g_chainman #include -constexpr char DB_BEST_BLOCK = 'B'; +constexpr uint8_t DB_BEST_BLOCK{'B'}; constexpr auto SYNC_LOG_INTERVAL{30s}; constexpr auto SYNC_LOCATOR_WRITE_INTERVAL{30s}; diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index 4152dfee5a52..1c89eb77d5a5 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -24,9 +24,9 @@ * as big-endian so that sequential reads of filters by height are fast. * Keys for the hash index have the type [DB_BLOCK_HASH, uint256]. */ -constexpr char DB_BLOCK_HASH = 's'; -constexpr char DB_BLOCK_HEIGHT = 't'; -constexpr char DB_FILTER_POS = 'P'; +constexpr uint8_t DB_BLOCK_HASH{'s'}; +constexpr uint8_t DB_BLOCK_HEIGHT{'t'}; +constexpr uint8_t DB_FILTER_POS{'P'}; constexpr unsigned int MAX_FLTR_FILE_SIZE = 0x1000000; // 16 MiB /** The pre-allocation chunk size for fltr?????.dat files */ @@ -68,7 +68,7 @@ struct DBHeightKey { template void Unserialize(Stream& s) { - char prefix = ser_readdata8(s); + const uint8_t prefix{ser_readdata8(s)}; if (prefix != DB_BLOCK_HEIGHT) { throw std::ios_base::failure("Invalid format for block filter index DB height key"); } @@ -81,9 +81,8 @@ struct DBHashKey { explicit DBHashKey(const uint256& hash_in) : hash(hash_in) {} - SERIALIZE_METHODS(DBHashKey, obj) - { - char prefix = DB_BLOCK_HASH; + SERIALIZE_METHODS(DBHashKey, obj) { + uint8_t prefix{DB_BLOCK_HASH}; READWRITE(prefix); if (prefix != DB_BLOCK_HASH) { throw std::ios_base::failure("Invalid format for block filter index DB hash key"); @@ -155,7 +154,7 @@ bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, BlockFilter& f } uint256 block_hash; - std::vector encoded_filter; + std::vector encoded_filter; try { filein >> block_hash >> encoded_filter; filter = BlockFilter(GetFilterType(), block_hash, std::move(encoded_filter)); diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index 682550e1d6f0..238b2312a091 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -13,9 +13,9 @@ #include #include -static constexpr char DB_BLOCK_HASH = 's'; -static constexpr char DB_BLOCK_HEIGHT = 't'; -static constexpr char DB_MUHASH = 'M'; +static constexpr uint8_t DB_BLOCK_HASH{'s'}; +static constexpr uint8_t DB_BLOCK_HEIGHT{'t'}; +static constexpr uint8_t DB_MUHASH{'M'}; namespace { @@ -67,7 +67,7 @@ struct DBHeightKey { template void Unserialize(Stream& s) { - char prefix{static_cast(ser_readdata8(s))}; + const uint8_t prefix{ser_readdata8(s)}; if (prefix != DB_BLOCK_HEIGHT) { throw std::ios_base::failure("Invalid format for coinstatsindex DB height key"); } @@ -82,7 +82,7 @@ struct DBHashKey { SERIALIZE_METHODS(DBHashKey, obj) { - char prefix{DB_BLOCK_HASH}; + uint8_t prefix{DB_BLOCK_HASH}; READWRITE(prefix); if (prefix != DB_BLOCK_HASH) { throw std::ios_base::failure("Invalid format for coinstatsindex DB hash key"); diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp index a50b991bb4ce..2b6af54afdf9 100644 --- a/src/index/txindex.cpp +++ b/src/index/txindex.cpp @@ -10,14 +10,13 @@ #include #include -constexpr char DB_BEST_BLOCK = 'B'; -constexpr char DB_TXINDEX = 't'; -constexpr char DB_TXINDEX_BLOCK = 'T'; +constexpr uint8_t DB_BEST_BLOCK{'B'}; +constexpr uint8_t DB_TXINDEX{'t'}; +constexpr uint8_t DB_TXINDEX_BLOCK{'T'}; std::unique_ptr g_txindex; - /** Access to the txindex database (indexes/txindex/) */ class TxIndex::DB : public BaseIndex::DB { @@ -60,8 +59,8 @@ bool TxIndex::DB::WriteTxs(const std::vector>& v_ */ static void WriteTxIndexMigrationBatches(CDBWrapper& newdb, CDBWrapper& olddb, CDBBatch& batch_newdb, CDBBatch& batch_olddb, - const std::pair& begin_key, - const std::pair& end_key) + const std::pair& begin_key, + const std::pair& end_key) { // Sync new DB changes to disk before deleting from old DB. newdb.WriteBatch(batch_newdb, /*fSync=*/ true); @@ -113,9 +112,9 @@ bool TxIndex::DB::MigrateData(CBlockTreeDB& block_tree_db, const CBlockLocator& CDBBatch batch_newdb(*this); CDBBatch batch_olddb(block_tree_db); - std::pair key; - std::pair begin_key{DB_TXINDEX, uint256()}; - std::pair prev_key = begin_key; + std::pair key; + std::pair begin_key{DB_TXINDEX, uint256()}; + std::pair prev_key = begin_key; bool interrupted = false; std::unique_ptr cursor(block_tree_db.NewIterator()); From 2c32a09f4e25c0f626ab5ca10b11af2c3737c008 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 31 May 2021 14:57:32 +0200 Subject: [PATCH 05/17] merge bitcoin#21969: Switch serialize to uint8_t --- src/addrman.cpp | 4 ++-- src/net.cpp | 2 +- src/serialize.h | 8 ++----- src/test/dbwrapper_tests.cpp | 28 ++++++++++++------------ src/test/serialize_tests.cpp | 8 +++---- src/txdb.cpp | 42 ++++++++++++++++++------------------ src/wallet/wallet.h | 4 ++-- src/wallet/walletdb.cpp | 8 +++---- 8 files changed, 50 insertions(+), 54 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 7bafc28c5690..ad78ae7f5ce8 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -39,7 +39,7 @@ int CAddrInfo::GetNewBucket(const uint256& nKey, const CNetAddr& src, const std: int CAddrInfo::GetBucketPosition(const uint256 &nKey, bool fNew, int nBucket) const { - uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << (fNew ? 'N' : 'K') << nBucket << GetKey()).GetCheapHash(); + uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << (fNew ? uint8_t{'N'} : uint8_t{'K'}) << nBucket << GetKey()).GetCheapHash(); return hash1 % ADDRMAN_BUCKET_SIZE; } @@ -762,7 +762,7 @@ std::vector CAddrMan::DecodeAsmap(fs::path path) int length = ftell(filestr); LogPrintf("Opened asmap file %s (%d bytes) from disk\n", path, length); fseek(filestr, 0, SEEK_SET); - char cur_byte; + uint8_t cur_byte; for (int i = 0; i < length; ++i) { file >> cur_byte; for (int bit = 0; bit < 8; ++bit) { diff --git a/src/net.cpp b/src/net.cpp index 2ca12bb0658f..6430f6ce1d99 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -4291,7 +4291,7 @@ void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Spa ser_writedata64(f, now.count()); f.write(msg_type.data(), msg_type.length()); for (auto i = msg_type.length(); i < CMessageHeader::COMMAND_SIZE; ++i) { - f << '\0'; + f << uint8_t{'\0'}; } uint32_t size = data.size(); ser_writedata32(f, size); diff --git a/src/serialize.h b/src/serialize.h index 623a1b838c29..e2c9230dc979 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -230,12 +230,8 @@ template inline void Unserialize(Stream& s, char (&a)[N] template inline void Unserialize(Stream& s, unsigned char (&a)[N]) { s.read(CharCast(a), N); } template inline void Unserialize(Stream& s, Span& span) { s.read(CharCast(span.data()), span.size()); } -template inline void Serialize(Stream& s, bool a) { char f=a; ser_writedata8(s, f); } -template inline void Unserialize(Stream& s, bool& a) { char f=ser_readdata8(s); a=f; } - - - - +template inline void Serialize(Stream& s, bool a) { uint8_t f = a; ser_writedata8(s, f); } +template inline void Unserialize(Stream& s, bool& a) { uint8_t f = ser_readdata8(s); a = f; } /** diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index a6080ad3ddb9..df3689349f09 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -28,7 +28,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper) for (const bool obfuscate : {false, true}) { fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_obfuscate_true" : "dbwrapper_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); - char key = 'k'; + uint8_t key{'k'}; uint256 in = InsecureRand256(); uint256 res; @@ -88,21 +88,21 @@ BOOST_AUTO_TEST_CASE(dbwrapper_basic_data) BOOST_CHECK_EQUAL(res.ToString(), in_utxo.ToString()); //Simulate last block file number - "l" - char key_last_blockfile_number = 'l'; + uint8_t key_last_blockfile_number{'l'}; uint32_t lastblockfilenumber = InsecureRand32(); BOOST_CHECK(dbw.Write(key_last_blockfile_number, lastblockfilenumber)); BOOST_CHECK(dbw.Read(key_last_blockfile_number, res_uint_32)); BOOST_CHECK_EQUAL(lastblockfilenumber, res_uint_32); //Simulate Is Reindexing - "R" - char key_IsReindexing = 'R'; + uint8_t key_IsReindexing{'R'}; bool isInReindexing = InsecureRandBool(); BOOST_CHECK(dbw.Write(key_IsReindexing, isInReindexing)); BOOST_CHECK(dbw.Read(key_IsReindexing, res_bool)); BOOST_CHECK_EQUAL(isInReindexing, res_bool); //Simulate last block hash up to which UXTO covers - 'B' - char key_lastblockhash_uxto = 'B'; + uint8_t key_lastblockhash_uxto{'B'}; uint256 lastblock_hash = InsecureRand256(); BOOST_CHECK(dbw.Write(key_lastblockhash_uxto, lastblock_hash)); BOOST_CHECK(dbw.Read(key_lastblockhash_uxto, res)); @@ -129,11 +129,11 @@ BOOST_AUTO_TEST_CASE(dbwrapper_batch) fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_batch_obfuscate_true" : "dbwrapper_batch_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); - char key = 'i'; + uint8_t key{'i'}; uint256 in = InsecureRand256(); - char key2 = 'j'; + uint8_t key2{'j'}; uint256 in2 = InsecureRand256(); - char key3 = 'k'; + uint8_t key3{'k'}; uint256 in3 = InsecureRand256(); uint256 res; @@ -166,10 +166,10 @@ BOOST_AUTO_TEST_CASE(dbwrapper_iterator) CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); // The two keys are intentionally chosen for ordering - char key = 'j'; + uint8_t key{'j'}; uint256 in = InsecureRand256(); BOOST_CHECK(dbw.Write(key, in)); - char key2 = 'k'; + uint8_t key2{'k'}; uint256 in2 = InsecureRand256(); BOOST_CHECK(dbw.Write(key2, in2)); @@ -178,7 +178,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_iterator) // Be sure to seek past the obfuscation key (if it exists) it->Seek(key); - char key_res; + uint8_t key_res; uint256 val_res; BOOST_REQUIRE(it->GetKey(key_res)); @@ -207,7 +207,7 @@ BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate) // Set up a non-obfuscated wrapper to write some initial data. std::unique_ptr dbw = std::make_unique(ph, (1 << 10), false, false, false); - char key = 'k'; + uint8_t key{'k'}; uint256 in = InsecureRand256(); uint256 res; @@ -248,7 +248,7 @@ BOOST_AUTO_TEST_CASE(existing_data_reindex) // Set up a non-obfuscated wrapper to write some initial data. std::unique_ptr dbw = std::make_unique(ph, (1 << 10), false, false, false); - char key = 'k'; + uint8_t key{'k'}; uint256 in = InsecureRand256(); uint256 res; @@ -334,7 +334,7 @@ struct StringContentsSerializer { void Serialize(Stream& s) const { for (size_t i = 0; i < str.size(); i++) { - s << str[i]; + s << uint8_t(str[i]); } } @@ -342,7 +342,7 @@ struct StringContentsSerializer { void Unserialize(Stream& s) { str.clear(); - char c = 0; + uint8_t c{0}; while (true) { try { s >> c; diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index a1097d584a14..0f532f91d8c6 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -24,7 +24,7 @@ class CSerializeMethodsTestSingle CTransactionRef txval; public: CSerializeMethodsTestSingle() = default; - CSerializeMethodsTestSingle(int intvalin, bool boolvalin, std::string stringvalin, const char* charstrvalin, const CTransactionRef& txvalin) : intval(intvalin), boolval(boolvalin), stringval(std::move(stringvalin)), txval(txvalin) + CSerializeMethodsTestSingle(int intvalin, bool boolvalin, std::string stringvalin, const uint8_t* charstrvalin, const CTransactionRef& txvalin) : intval(intvalin), boolval(boolvalin), stringval(std::move(stringvalin)), txval(txvalin) { memcpy(charstrval, charstrvalin, sizeof(charstrval)); } @@ -70,8 +70,8 @@ BOOST_AUTO_TEST_CASE(sizes) BOOST_CHECK_EQUAL(sizeof(uint32_t), GetSerializeSize(uint32_t(0), 0)); BOOST_CHECK_EQUAL(sizeof(int64_t), GetSerializeSize(int64_t(0), 0)); BOOST_CHECK_EQUAL(sizeof(uint64_t), GetSerializeSize(uint64_t(0), 0)); - // Bool is serialized as char - BOOST_CHECK_EQUAL(sizeof(char), GetSerializeSize(bool(0), 0)); + // Bool is serialized as uint8_t + BOOST_CHECK_EQUAL(sizeof(uint8_t), GetSerializeSize(bool(0), 0)); // Sanity-check GetSerializeSize and c++ type matching BOOST_CHECK_EQUAL(GetSerializeSize(char(0), 0), 1U); @@ -315,7 +315,7 @@ BOOST_AUTO_TEST_CASE(class_methods) int intval(100); bool boolval(true); std::string stringval("testing"); - const char charstrval[16] = "testing charstr"; + const uint8_t charstrval[16]{"testing charstr"}; CMutableTransaction txval; CTransactionRef tx_ref{MakeTransactionRef(txval)}; CSerializeMethodsTestSingle methodtest1(intval, boolval, stringval, charstrval, tx_ref); diff --git a/src/txdb.cpp b/src/txdb.cpp index fae9ff1128d9..d4ca0656c811 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -16,26 +16,26 @@ #include -static const char DB_COIN = 'C'; -static const char DB_COINS = 'c'; -static const char DB_BLOCK_FILES = 'f'; -static const char DB_ADDRESSINDEX = 'a'; -static const char DB_ADDRESSUNSPENTINDEX = 'u'; -static const char DB_TIMESTAMPINDEX = 's'; -static const char DB_SPENTINDEX = 'p'; -static const char DB_BLOCK_INDEX = 'b'; - -static const char DB_BEST_BLOCK = 'B'; -static const char DB_HEAD_BLOCKS = 'H'; -static const char DB_FLAG = 'F'; -static const char DB_REINDEX_FLAG = 'R'; -static const char DB_LAST_BLOCK = 'l'; +static constexpr uint8_t DB_COIN{'C'}; +static constexpr uint8_t DB_COINS{'c'}; +static constexpr uint8_t DB_BLOCK_FILES{'f'}; +static constexpr uint8_t DB_ADDRESSINDEX{'a'}; +static constexpr uint8_t DB_ADDRESSUNSPENTINDEX{'u'}; +static constexpr uint8_t DB_TIMESTAMPINDEX{'s'}; +static constexpr uint8_t DB_SPENTINDEX{'p'}; +static constexpr uint8_t DB_BLOCK_INDEX{'b'}; + +static constexpr uint8_t DB_BEST_BLOCK{'B'}; +static constexpr uint8_t DB_HEAD_BLOCKS{'H'}; +static constexpr uint8_t DB_FLAG{'F'}; +static constexpr uint8_t DB_REINDEX_FLAG{'R'}; +static constexpr uint8_t DB_LAST_BLOCK{'l'}; namespace { struct CoinEntry { COutPoint* outpoint; - char key; + uint8_t key; explicit CoinEntry(const COutPoint* ptr) : outpoint(const_cast(ptr)), key(DB_COIN) {} SERIALIZE_METHODS(CoinEntry, obj) { READWRITE(obj.key, obj.outpoint->hash, VARINT(obj.outpoint->n)); } @@ -147,7 +147,7 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { size_t CCoinsViewDB::EstimateSize() const { - return m_db->EstimateSize(DB_COIN, (char)(DB_COIN+1)); + return m_db->EstimateSize(DB_COIN, uint8_t(DB_COIN + 1)); } CBlockTreeDB::CBlockTreeDB(size_t nCacheSize, bool fMemory, bool fWipe) : CDBWrapper(GetDataDir() / "blocks" / "index", nCacheSize, fMemory, fWipe) { @@ -159,7 +159,7 @@ bool CBlockTreeDB::ReadBlockFileInfo(int nFile, CBlockFileInfo &info) { bool CBlockTreeDB::WriteReindexing(bool fReindexing) { if (fReindexing) - return Write(DB_REINDEX_FLAG, '1'); + return Write(DB_REINDEX_FLAG, uint8_t{'1'}); else return Erase(DB_REINDEX_FLAG); } @@ -396,14 +396,14 @@ bool CBlockTreeDB::ReadTimestampIndex(const unsigned int &high, const unsigned i } bool CBlockTreeDB::WriteFlag(const std::string &name, bool fValue) { - return Write(std::make_pair(DB_FLAG, name), fValue ? '1' : '0'); + return Write(std::make_pair(DB_FLAG, name), fValue ? uint8_t{'1'} : uint8_t{'0'}); } bool CBlockTreeDB::ReadFlag(const std::string &name, bool &fValue) { - char ch; + uint8_t ch; if (!Read(std::make_pair(DB_FLAG, name), ch)) return false; - fValue = ch == '1'; + fValue = ch == uint8_t{'1'}; return true; } @@ -416,7 +416,7 @@ bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams, // Load m_block_index while (pcursor->Valid()) { if (ShutdownRequested()) return false; - std::pair key; + std::pair key; if (pcursor->GetKey(key) && key.first == DB_BLOCK_INDEX) { CDiskBlockIndex diskindex; if (pcursor->GetValue(diskindex)) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index d5f34ebb3cb6..78e2772623bc 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -420,8 +420,8 @@ class CWalletTx mapValueCopy["timesmart"] = strprintf("%u", nTimeSmart); } - std::vector dummy_vector1; //!< Used to be vMerkleBranch - std::vector dummy_vector2; //!< Used to be vtxPrev + std::vector dummy_vector1; //!< Used to be vMerkleBranch + std::vector dummy_vector2; //!< Used to be vtxPrev bool dummy_bool = false; //!< Used to be fSpent uint256 serializedHash = isAbandoned() ? ABANDON_HASH : m_confirm.hashBlock; int serializedIndex = isAbandoned() || isConflicted() ? -1 : m_confirm.nIndex; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 6a5e8c2822e0..90ae0c6564d9 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -157,7 +157,7 @@ bool WalletBatch::WriteWatchOnly(const CScript &dest, const CKeyMetadata& keyMet if (!WriteIC(std::make_pair(DBKeys::WATCHMETA, dest), keyMeta)) { return false; } - return WriteIC(std::make_pair(DBKeys::WATCHS, dest), '1'); + return WriteIC(std::make_pair(DBKeys::WATCHS, dest), uint8_t{'1'}); } bool WalletBatch::EraseWatchOnly(const CScript &dest) @@ -277,8 +277,8 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, { if (!ssValue.empty()) { - char fTmp; - char fUnused; + uint8_t fTmp; + uint8_t fUnused; std::string unused_string; ssValue >> fTmp >> fUnused >> unused_string; strErr = strprintf("LoadWallet() upgrading tx ver=%d %d %s", @@ -305,7 +305,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, wss.nWatchKeys++; CScript script; ssKey >> script; - char fYes; + uint8_t fYes; ssValue >> fYes; if (fYes == '1') { pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadWatchOnly(script); From d3b282208bc083b37cea81e972ce5390f88ffac1 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 1 Dec 2021 14:40:25 -0500 Subject: [PATCH 06/17] merge bitcoin#23653: Generalize/simplify VectorReader into SpanReader --- src/blockfilter.cpp | 8 ++++---- src/streams.h | 35 +++++++++++++++++------------------ src/test/fuzz/golomb_rice.cpp | 8 ++++---- src/test/streams_tests.cpp | 6 +++--- 4 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp index bcf4cfbc7707..931269736547 100644 --- a/src/blockfilter.cpp +++ b/src/blockfilter.cpp @@ -50,7 +50,7 @@ GCSFilter::GCSFilter(const Params& params) GCSFilter::GCSFilter(const Params& params, std::vector encoded_filter) : m_params(params), m_encoded(std::move(encoded_filter)) { - VectorReader stream(GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0); + SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0}; uint64_t N = ReadCompactSize(stream); m_N = static_cast(N); @@ -61,7 +61,7 @@ GCSFilter::GCSFilter(const Params& params, std::vector encoded_fi // Verify that the encoded filter contains exactly N elements. If it has too much or too little // data, a std::ios_base::failure exception will be raised. - BitStreamReader bitreader(stream); + BitStreamReader bitreader{stream}; for (uint64_t i = 0; i < m_N; ++i) { GolombRiceDecode(bitreader, m_params.m_P); } @@ -102,13 +102,13 @@ GCSFilter::GCSFilter(const Params& params, const ElementSet& elements) bool GCSFilter::MatchInternal(const uint64_t* element_hashes, size_t size) const { - VectorReader stream(GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0); + SpanReader stream{GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0}; // Seek forward by size of N uint64_t N = ReadCompactSize(stream); assert(N == m_N); - BitStreamReader bitreader(stream); + BitStreamReader bitreader{stream}; uint64_t value = 0; size_t hashes_index = 0; diff --git a/src/streams.h b/src/streams.h index 9eaad38396d0..ddf0a80cdc4f 100644 --- a/src/streams.h +++ b/src/streams.h @@ -132,15 +132,14 @@ class CVectorWriter size_t nPos; }; -/** Minimal stream for reading from an existing vector by reference +/** Minimal stream for reading from an existing byte array by Span. */ -class VectorReader +class SpanReader { private: const int m_type; const int m_version; - const std::vector& m_data; - size_t m_pos = 0; + Span m_data; public: @@ -150,12 +149,13 @@ class VectorReader * @param[in] data Referenced byte vector to overwrite/append * @param[in] pos Starting position. Vector index where reads should start. */ - VectorReader(int type, int version, const std::vector& data, size_t pos) - : m_type(type), m_version(version), m_data(data), m_pos(pos) + SpanReader(int type, int version, Span data, size_t pos) + : m_type(type), m_version(version), m_data(data) { - if (m_pos > m_data.size()) { - throw std::ios_base::failure("VectorReader(...): end of data (m_pos > m_data.size())"); + if (pos > m_data.size()) { + throw std::ios_base::failure("SpanReader(...): end of data (pos > m_data.size())"); } + data = data.subspan(pos); } /** @@ -163,15 +163,15 @@ class VectorReader * @param[in] args A list of items to deserialize starting at pos. */ template - VectorReader(int type, int version, const std::vector& data, size_t pos, + SpanReader(int type, int version, Span data, size_t pos, Args&&... args) - : VectorReader(type, version, data, pos) + : SpanReader(type, version, data, pos) { ::UnserializeMany(*this, std::forward(args)...); } template - VectorReader& operator>>(T&& obj) + SpanReader& operator>>(T&& obj) { // Unserialize from this stream ::Unserialize(*this, obj); @@ -181,8 +181,8 @@ class VectorReader int GetVersion() const { return m_version; } int GetType() const { return m_type; } - size_t size() const { return m_data.size() - m_pos; } - bool empty() const { return m_data.size() == m_pos; } + size_t size() const { return m_data.size(); } + bool empty() const { return m_data.empty(); } void read(char* dst, size_t n) { @@ -191,12 +191,11 @@ class VectorReader } // Read from the beginning of the buffer - size_t pos_next = m_pos + n; - if (pos_next > m_data.size()) { - throw std::ios_base::failure("VectorReader::read(): end of data"); + if (n > m_data.size()) { + throw std::ios_base::failure("SpanReader::read(): end of data"); } - memcpy(dst, m_data.data() + m_pos, n); - m_pos = pos_next; + memcpy(dst, m_data.data(), n); + m_data = m_data.subspan(n); } }; diff --git a/src/test/fuzz/golomb_rice.cpp b/src/test/fuzz/golomb_rice.cpp index af1c2a26dcb5..9a48b663eb37 100644 --- a/src/test/fuzz/golomb_rice.cpp +++ b/src/test/fuzz/golomb_rice.cpp @@ -68,8 +68,8 @@ FUZZ_TARGET(golomb_rice) std::vector decoded_deltas; { - VectorReader stream{SER_NETWORK, 0, golomb_rice_data, 0}; - BitStreamReader bitreader(stream); + SpanReader stream{SER_NETWORK, 0, golomb_rice_data, 0}; + BitStreamReader bitreader{stream}; const uint32_t n = static_cast(ReadCompactSize(stream)); for (uint32_t i = 0; i < n; ++i) { decoded_deltas.push_back(GolombRiceDecode(bitreader, BASIC_FILTER_P)); @@ -80,14 +80,14 @@ FUZZ_TARGET(golomb_rice) { const std::vector random_bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider, 1024); - VectorReader stream{SER_NETWORK, 0, random_bytes, 0}; + SpanReader stream{SER_NETWORK, 0, random_bytes, 0}; uint32_t n; try { n = static_cast(ReadCompactSize(stream)); } catch (const std::ios_base::failure&) { return; } - BitStreamReader bitreader(stream); + BitStreamReader bitreader{stream}; for (uint32_t i = 0; i < std::min(n, 1024); ++i) { try { (void)GolombRiceDecode(bitreader, BASIC_FILTER_P); diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index af6e302878f0..5a4c5fc988e4 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -71,7 +71,7 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader) { std::vector vch = {1, 255, 3, 4, 5, 6}; - VectorReader reader(SER_NETWORK, INIT_PROTO_VERSION, vch, 0); + SpanReader reader{SER_NETWORK, INIT_PROTO_VERSION, vch, 0}; BOOST_CHECK_EQUAL(reader.size(), 6U); BOOST_CHECK(!reader.empty()); @@ -101,7 +101,7 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader) BOOST_CHECK_THROW(reader >> d, std::ios_base::failure); // Read a 4 bytes as a signed int from the beginning of the buffer. - VectorReader new_reader(SER_NETWORK, INIT_PROTO_VERSION, vch, 0); + SpanReader new_reader{SER_NETWORK, INIT_PROTO_VERSION, vch, 0}; new_reader >> d; BOOST_CHECK_EQUAL(d, 67370753); // 1,255,3,4 in little-endian base-256 BOOST_CHECK_EQUAL(new_reader.size(), 2U); @@ -115,7 +115,7 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader) BOOST_AUTO_TEST_CASE(streams_vector_reader_rvalue) { std::vector data{0x82, 0xa7, 0x31}; - VectorReader reader(SER_NETWORK, INIT_PROTO_VERSION, data, /* pos= */ 0); + SpanReader reader{SER_NETWORK, INIT_PROTO_VERSION, data, /* pos= */ 0}; uint32_t varint = 0; // Deserialize into r-value reader >> VARINT(varint); From e933d78a88fdb07b4e9ef7fec69ec16f02c1240c Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 24 Feb 2024 07:36:25 +0000 Subject: [PATCH 07/17] merge bitcoin#23438: Use spans of std::byte in serialize continuation of de54b878 from dash#5574 --- src/bench/checkblock.cpp | 8 +-- src/bench/rpc_blockchain.cpp | 4 +- src/bloom.cpp | 2 +- src/bls/bls.h | 10 +-- src/bls/bls_ies.h | 4 +- src/dbwrapper.h | 6 +- src/evo/specialtx.h | 2 +- src/flat-database.h | 2 +- src/hash.h | 15 +++-- src/llmq/dkgsessionhandler.cpp | 2 +- src/net.cpp | 4 +- src/psbt.cpp | 2 +- src/pubkey.h | 8 +-- src/rpc/evo.cpp | 2 +- src/script/bitcoinconsensus.cpp | 17 +++-- src/script/interpreter.cpp | 4 +- src/serialize.h | 89 ++++++++++++-------------- src/streams.h | 86 +++++++++++++------------ src/support/allocators/zeroafterfree.h | 2 +- src/test/bloom_tests.cpp | 12 ++-- src/test/fuzz/autofile.cpp | 8 +-- src/test/fuzz/buffered_file.cpp | 6 +- src/test/fuzz/integer.cpp | 5 -- src/test/fuzz/util.h | 6 +- src/test/script_tests.cpp | 12 ++-- src/test/serialize_tests.cpp | 52 +++++++-------- src/test/streams_tests.cpp | 10 +-- src/txdb.cpp | 8 +-- src/uint256.h | 4 +- src/validation.cpp | 2 +- src/wallet/bdb.cpp | 6 +- src/wallet/sqlite.cpp | 18 +++--- 32 files changed, 210 insertions(+), 208 deletions(-) diff --git a/src/bench/checkblock.cpp b/src/bench/checkblock.cpp index cf3b5338b119..3dbb29ebe56c 100644 --- a/src/bench/checkblock.cpp +++ b/src/bench/checkblock.cpp @@ -17,8 +17,8 @@ static void DeserializeBlockTest(benchmark::Bench& bench) { CDataStream stream(benchmark::data::block813851, SER_NETWORK, PROTOCOL_VERSION); - char a = '\0'; - stream.write(&a, 1); // Prevent compaction + std::byte a{0}; + stream.write({&a, 1}); // Prevent compaction bench.unit("block").run([&] { CBlock block; @@ -31,8 +31,8 @@ static void DeserializeBlockTest(benchmark::Bench& bench) static void DeserializeAndCheckBlockTest(benchmark::Bench& bench) { CDataStream stream(benchmark::data::block813851, SER_NETWORK, PROTOCOL_VERSION); - char a = '\0'; - stream.write(&a, 1); // Prevent compaction + std::byte a{0}; + stream.write({&a, 1}); // Prevent compaction ArgsManager bench_args; const auto chainParams = CreateChainParams(bench_args, CBaseChainParams::MAIN); diff --git a/src/bench/rpc_blockchain.cpp b/src/bench/rpc_blockchain.cpp index bf4dbb1c8dcd..ffd9411438e1 100644 --- a/src/bench/rpc_blockchain.cpp +++ b/src/bench/rpc_blockchain.cpp @@ -27,8 +27,8 @@ struct TestBlockAndIndex { TestBlockAndIndex() { CDataStream stream(benchmark::data::block813851, SER_NETWORK, PROTOCOL_VERSION); - char a = '\0'; - stream.write(&a, 1); // Prevent compaction + std::byte a{0}; + stream.write({&a, 1}); // Prevent compaction stream >> block; diff --git a/src/bloom.cpp b/src/bloom.cpp index 77eb2f3600cc..73adaab8cac1 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -66,7 +66,7 @@ void CBloomFilter::insert(const COutPoint& outpoint) { CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << outpoint; - insert(stream); + insert(MakeUCharSpan(stream)); } bool CBloomFilter::contains(Span vKey) const diff --git a/src/bls/bls.h b/src/bls/bls.h index 5c21d99042f4..eb55cea70164 100644 --- a/src/bls/bls.h +++ b/src/bls/bls.h @@ -167,7 +167,7 @@ class CBLSWrapper template inline void Serialize(Stream& s, const bool specificLegacyScheme) const { - s.write(reinterpret_cast(ToByteVector(specificLegacyScheme).data()), SerSize); + s.write(AsBytes(Span{ToByteVector(specificLegacyScheme).data(), SerSize})); } template @@ -180,7 +180,7 @@ class CBLSWrapper inline void Unserialize(Stream& s, const bool specificLegacyScheme) { std::array vecBytes{}; - s.read(reinterpret_cast(vecBytes.data()), SerSize); + s.read(AsWritableBytes(Span{vecBytes.data(), SerSize})); SetByteVector(vecBytes, specificLegacyScheme); if (!CheckMalleable(vecBytes, specificLegacyScheme)) { @@ -456,7 +456,7 @@ class CBLSLazyWrapper bufLegacyScheme = specificLegacyScheme; hash.SetNull(); } - s.write(reinterpret_cast(vecBytes.data()), vecBytes.size()); + s.write(MakeByteSpan(vecBytes)); } template @@ -469,7 +469,7 @@ class CBLSLazyWrapper inline void Unserialize(Stream& s, const bool specificLegacyScheme) const { std::unique_lock l(mutex); - s.read(reinterpret_cast(vecBytes.data()), BLSObject::SerSize); + s.read(AsWritableBytes(Span{vecBytes.data(), BLSObject::SerSize})); bufValid = true; bufLegacyScheme = specificLegacyScheme; objInitialized = false; @@ -543,7 +543,7 @@ class CBLSLazyWrapper } if (hash.IsNull()) { CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION); - ss.write(reinterpret_cast(vecBytes.data()), vecBytes.size()); + ss.write(MakeByteSpan(vecBytes)); hash = ss.GetHash(); } return hash; diff --git a/src/bls/bls_ies.h b/src/bls/bls_ies.h index 43d4c0be10e0..0a43437550f6 100644 --- a/src/bls/bls_ies.h +++ b/src/bls/bls_ies.h @@ -109,7 +109,7 @@ class CBLSIESMultiRecipientObjects : public CBLSIESMultiRecipientBlobs ds.clear(); ds << _objects[i]; - blobs[i].assign(ds.begin(), ds.end()); + blobs[i].assign(UCharCast(ds.data()), UCharCast(ds.data() + ds.size())); } } catch (const std::exception&) { return false; @@ -122,7 +122,7 @@ class CBLSIESMultiRecipientObjects : public CBLSIESMultiRecipientBlobs { CDataStream ds(SER_NETWORK, nVersion); ds << obj; - Blob blob(ds.begin(), ds.end()); + Blob blob(UCharCast(ds.data()), UCharCast(ds.data() + ds.size())); return CBLSIESMultiRecipientBlobs::Encrypt(idx, recipient, blob); } diff --git a/src/dbwrapper.h b/src/dbwrapper.h index cea0ad54bce4..28c559f7aa53 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -169,7 +169,7 @@ class CDBIterator CDataStream GetKey() { leveldb::Slice slKey = piter->key(); - return CDataStream(MakeUCharSpan(slKey), SER_DISK, CLIENT_VERSION); + return CDataStream{MakeByteSpan(slKey), SER_DISK, CLIENT_VERSION}; } unsigned int GetKeySize() { @@ -179,7 +179,7 @@ class CDBIterator template bool GetValue(V& value) { leveldb::Slice slValue = piter->value(); try { - CDataStream ssValue(MakeUCharSpan(slValue), SER_DISK, CLIENT_VERSION); + CDataStream ssValue{MakeByteSpan(slValue), SER_DISK, CLIENT_VERSION}; ssValue.Xor(dbwrapper_private::GetObfuscateKey(parent)); ssValue >> value; } catch (const std::exception&) { @@ -269,7 +269,7 @@ class CDBWrapper LogPrintf("LevelDB read failure: %s\n", status.ToString()); dbwrapper_private::HandleError(status); } - CDataStream ssValueTmp(MakeUCharSpan(strValue), SER_DISK, CLIENT_VERSION); + CDataStream ssValueTmp{MakeByteSpan(strValue), SER_DISK, CLIENT_VERSION}; ssValueTmp.Xor(obfuscate_key); ssValue = std::move(ssValueTmp); return true; diff --git a/src/evo/specialtx.h b/src/evo/specialtx.h index 49c3d6477059..1de481104c92 100644 --- a/src/evo/specialtx.h +++ b/src/evo/specialtx.h @@ -40,7 +40,7 @@ void SetTxPayload(CMutableTransaction& tx, const T& payload) { CDataStream ds(SER_NETWORK, PROTOCOL_VERSION); ds << payload; - tx.vExtraPayload.assign(ds.begin(), ds.end()); + tx.vExtraPayload.assign(UCharCast(ds.data()), UCharCast(ds.data() + ds.size())); } uint256 CalcTxInputsHash(const CTransaction& tx); diff --git a/src/flat-database.h b/src/flat-database.h index 4d2745f92b2b..7bb774f3f144 100644 --- a/src/flat-database.h +++ b/src/flat-database.h @@ -97,7 +97,7 @@ class CFlatDB // read data and checksum from file try { - filein.read((char *)vchData.data(), dataSize); + filein.read(MakeWritableByteSpan(vchData)); filein >> hashIn; } catch (std::exception &e) { diff --git a/src/hash.h b/src/hash.h index e82a345b8420..bde7fb695323 100644 --- a/src/hash.h +++ b/src/hash.h @@ -124,8 +124,9 @@ class CHashWriter int GetType() const { return nType; } int GetVersion() const { return nVersion; } - void write(const char *pch, size_t size) { - ctx.Write((const unsigned char*)pch, size); + void write(Span src) + { + ctx.Write(UCharCast(src.data()), src.size()); } /** Compute the double-SHA256 hash of all data written to this object. @@ -175,18 +176,18 @@ class CHashVerifier : public CHashWriter public: explicit CHashVerifier(Source* source_) : CHashWriter(source_->GetType(), source_->GetVersion()), source(source_) {} - void read(char* pch, size_t nSize) + void read(Span dst) { - source->read(pch, nSize); - this->write(pch, nSize); + source->read(dst); + this->write(dst); } void ignore(size_t nSize) { - char data[1024]; + std::byte data[1024]; while (nSize > 0) { size_t now = std::min(nSize, 1024); - read(data, now); + read({data, now}); nSize -= now; } } diff --git a/src/llmq/dkgsessionhandler.cpp b/src/llmq/dkgsessionhandler.cpp index f9a900bacd81..2248de9a4f23 100644 --- a/src/llmq/dkgsessionhandler.cpp +++ b/src/llmq/dkgsessionhandler.cpp @@ -62,7 +62,7 @@ void CDKGPendingMessages::PushPendingMessage(NodeId from, PeerManager* peerman, auto pm = std::make_shared(std::move(vRecv)); CHashWriter hw(SER_GETHASH, 0); - hw.write(reinterpret_cast(pm->data()), pm->size()); + hw.write(AsWritableBytes(Span{*pm})); uint256 hash = hw.GetHash(); if (from != -1) { diff --git a/src/net.cpp b/src/net.cpp index 6430f6ce1d99..aef3e7fa9aa8 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -4289,11 +4289,11 @@ void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Spa CAutoFile f(fsbridge::fopen(path, "ab"), SER_DISK, CLIENT_VERSION); ser_writedata64(f, now.count()); - f.write(msg_type.data(), msg_type.length()); + f.write(MakeByteSpan(msg_type)); for (auto i = msg_type.length(); i < CMessageHeader::COMMAND_SIZE; ++i) { f << uint8_t{'\0'}; } uint32_t size = data.size(); ser_writedata32(f, size); - f.write((const char*)data.data(), data.size()); + f.write(AsBytes(data)); } diff --git a/src/psbt.cpp b/src/psbt.cpp index be52ecc50214..7d03131661da 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -318,7 +318,7 @@ bool DecodeBase64PSBT(PartiallySignedTransaction& psbt, const std::string& base6 bool DecodeRawPSBT(PartiallySignedTransaction& psbt, const std::string& tx_data, std::string& error) { - CDataStream ss_data(MakeUCharSpan(tx_data), SER_NETWORK, PROTOCOL_VERSION); + CDataStream ss_data(MakeByteSpan(tx_data), SER_NETWORK, PROTOCOL_VERSION); try { ss_data >> psbt; if (!ss_data.empty()) { diff --git a/src/pubkey.h b/src/pubkey.h index 79dd66ed71c7..2b3af515a304 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -135,14 +135,14 @@ class CPubKey { unsigned int len = size(); ::WriteCompactSize(s, len); - s.write((char*)vch, len); + s.write(AsBytes(Span{vch, len})); } template void Unserialize(Stream& s) { const unsigned int len(::ReadCompactSize(s)); if (len <= SIZE) { - s.read((char*)vch, len); + s.read(AsWritableBytes(Span{vch, len})); if (len != size()) { Invalidate(); } @@ -269,7 +269,7 @@ struct CExtPubKey { ::WriteCompactSize(s, len); unsigned char code[BIP32_EXTKEY_SIZE]; Encode(code); - s.write((const char *)&code[0], len); + s.write(AsBytes(Span{&code[0], len})); } template void Unserialize(Stream& s) @@ -278,7 +278,7 @@ struct CExtPubKey { unsigned char code[BIP32_EXTKEY_SIZE]; if (len != BIP32_EXTKEY_SIZE) throw std::runtime_error("Invalid extended key size\n"); - s.read((char *)&code[0], len); + s.read(AsWritableBytes(Span{&code[0], len})); Decode(code); } }; diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 36820f62f04e..99639cb1ea6f 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -229,7 +229,7 @@ static void FundSpecialTx(CWallet* pwallet, CMutableTransaction& tx, const Speci CDataStream ds(SER_NETWORK, PROTOCOL_VERSION); ds << payload; - tx.vExtraPayload.assign(ds.begin(), ds.end()); + tx.vExtraPayload.assign(UCharCast(ds.data()), UCharCast(ds.data() + ds.size())); static const CTxOut dummyTxOut(0, CScript() << OP_RETURN); std::vector vecSend; diff --git a/src/script/bitcoinconsensus.cpp b/src/script/bitcoinconsensus.cpp index 2b16a64fb7f4..f1a11cf5ef4d 100644 --- a/src/script/bitcoinconsensus.cpp +++ b/src/script/bitcoinconsensus.cpp @@ -22,20 +22,23 @@ class TxInputStream m_remaining(txToLen) {} - void read(char* pch, size_t nSize) + void read(Span dst) { - if (nSize > m_remaining) + if (dst.size() > m_remaining) { throw std::ios_base::failure(std::string(__func__) + ": end of data"); + } - if (pch == nullptr) + if (dst.data() == nullptr) { throw std::ios_base::failure(std::string(__func__) + ": bad destination buffer"); + } - if (m_data == nullptr) + if (m_data == nullptr) { throw std::ios_base::failure(std::string(__func__) + ": bad source buffer"); + } - memcpy(pch, m_data, nSize); - m_remaining -= nSize; - m_data += nSize; + memcpy(dst.data(), m_data, dst.size()); + m_remaining -= dst.size(); + m_data += dst.size(); } template diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 9ad410052422..63f34a09cd47 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1418,12 +1418,12 @@ class CTransactionSignatureSerializer it = itBegin; while (scriptCode.GetOp(it, opcode)) { if (opcode == OP_CODESEPARATOR) { - s.write((char*)&itBegin[0], it-itBegin-1); + s.write(AsBytes(Span{&itBegin[0], size_t(it - itBegin - 1)})); itBegin = it; } } if (itBegin != scriptCode.end()) - s.write((char*)&itBegin[0], it-itBegin); + s.write(AsBytes(Span{&itBegin[0], size_t(it - itBegin)})); } /** Serialize an input of txTo */ diff --git a/src/serialize.h b/src/serialize.h index e2c9230dc979..3be28c7e56eb 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -51,79 +51,72 @@ static const unsigned int MAX_VECTOR_ALLOCATE = 5000000; struct deserialize_type {}; constexpr deserialize_type deserialize {}; -//! Safely convert odd char pointer types to standard ones. -inline char* CharCast(char* c) { return c; } -inline char* CharCast(unsigned char* c) { return (char*)c; } -inline const char* CharCast(const char* c) { return c; } -inline const char* CharCast(const unsigned char* c) { return (const char*)c; } - /* * Lowest-level serialization and conversion. - * @note Sizes of these types are verified in the tests */ template inline void ser_writedata8(Stream &s, uint8_t obj) { - s.write((char*)&obj, 1); + s.write(AsBytes(Span{&obj, 1})); } template inline void ser_writedata16(Stream &s, uint16_t obj) { obj = htole16(obj); - s.write((char*)&obj, 2); + s.write(AsBytes(Span{&obj, 1})); } template inline void ser_writedata16be(Stream &s, uint16_t obj) { obj = htobe16(obj); - s.write((char*)&obj, 2); + s.write(AsBytes(Span{&obj, 1})); } template inline void ser_writedata32(Stream &s, uint32_t obj) { obj = htole32(obj); - s.write((char*)&obj, 4); + s.write(AsBytes(Span{&obj, 1})); } template inline void ser_writedata32be(Stream &s, uint32_t obj) { obj = htobe32(obj); - s.write((char*)&obj, 4); + s.write(AsBytes(Span{&obj, 1})); } template inline void ser_writedata64(Stream &s, uint64_t obj) { obj = htole64(obj); - s.write((char*)&obj, 8); + s.write(AsBytes(Span{&obj, 1})); } template inline uint8_t ser_readdata8(Stream &s) { uint8_t obj; - s.read((char*)&obj, 1); + s.read(AsWritableBytes(Span{&obj, 1})); return obj; } template inline uint16_t ser_readdata16(Stream &s) { uint16_t obj; - s.read((char*)&obj, 2); + s.read(AsWritableBytes(Span{&obj, 1})); return le16toh(obj); } template inline uint16_t ser_readdata16be(Stream &s) { uint16_t obj; - s.read((char*)&obj, 2); + s.read(AsWritableBytes(Span{&obj, 1})); return be16toh(obj); } template inline uint32_t ser_readdata32(Stream &s) { uint32_t obj; - s.read((char*)&obj, 4); + s.read(AsWritableBytes(Span{&obj, 1})); return le32toh(obj); } template inline uint32_t ser_readdata32be(Stream &s) { uint32_t obj; - s.read((char*)&obj, 4); + s.read(AsWritableBytes(Span{&obj, 1})); return be32toh(obj); } template inline uint64_t ser_readdata64(Stream &s) { uint64_t obj; - s.read((char*)&obj, 8); + s.read(AsWritableBytes(Span{&obj, 1})); return le64toh(obj); } @@ -131,7 +124,7 @@ template inline uint64_t ser_readdata64(Stream &s) ///////////////////////////////////////////////////////////////// // // Templates for serializing to anything that looks like a stream, -// i.e. anything that supports .read(char*, size_t) and .write(char*, size_t) +// i.e. anything that supports .read(Span) and .write(Span) // class CSizeComputer; @@ -200,7 +193,7 @@ template const X& ReadWriteAsHelper(const X& x) { return x; } FORMATTER_METHODS(cls, obj) #ifndef CHAR_EQUALS_INT8 -template inline void Serialize(Stream& s, char a ) { ser_writedata8(s, a); } // TODO Get rid of bare char +template void Serialize(Stream&, char) = delete; // char serialization forbidden. Use uint8_t or int8_t #endif template inline void Serialize(Stream& s, int8_t a ) { ser_writedata8(s, a); } template inline void Serialize(Stream& s, uint8_t a ) { ser_writedata8(s, a); } @@ -210,13 +203,13 @@ template inline void Serialize(Stream& s, int32_t a ) { ser_wri template inline void Serialize(Stream& s, uint32_t a) { ser_writedata32(s, a); } template inline void Serialize(Stream& s, int64_t a ) { ser_writedata64(s, a); } template inline void Serialize(Stream& s, uint64_t a) { ser_writedata64(s, a); } -template inline void Serialize(Stream& s, const char (&a)[N]) { s.write(a, N); } -template inline void Serialize(Stream& s, const unsigned char (&a)[N]) { s.write(CharCast(a), N); } -template inline void Serialize(Stream& s, const Span& span) { s.write(CharCast(span.data()), span.size()); } -template inline void Serialize(Stream& s, const Span& span) { s.write(CharCast(span.data()), span.size()); } +template inline void Serialize(Stream& s, const char (&a)[N]) { s.write(MakeByteSpan(a)); } +template inline void Serialize(Stream& s, const unsigned char (&a)[N]) { s.write(MakeByteSpan(a)); } +template inline void Serialize(Stream& s, const Span& span) { s.write(AsBytes(span)); } +template inline void Serialize(Stream& s, const Span& span) { s.write(AsBytes(span)); } #ifndef CHAR_EQUALS_INT8 -template inline void Unserialize(Stream& s, char& a ) { a = ser_readdata8(s); } // TODO Get rid of bare char +template void Unserialize(Stream&, char) = delete; // char serialization forbidden. Use uint8_t or int8_t #endif template inline void Unserialize(Stream& s, int8_t& a ) { a = ser_readdata8(s); } template inline void Unserialize(Stream& s, uint8_t& a ) { a = ser_readdata8(s); } @@ -226,9 +219,9 @@ template inline void Unserialize(Stream& s, int32_t& a ) { a = template inline void Unserialize(Stream& s, uint32_t& a) { a = ser_readdata32(s); } template inline void Unserialize(Stream& s, int64_t& a ) { a = ser_readdata64(s); } template inline void Unserialize(Stream& s, uint64_t& a) { a = ser_readdata64(s); } -template inline void Unserialize(Stream& s, char (&a)[N]) { s.read(a, N); } -template inline void Unserialize(Stream& s, unsigned char (&a)[N]) { s.read(CharCast(a), N); } -template inline void Unserialize(Stream& s, Span& span) { s.read(CharCast(span.data()), span.size()); } +template inline void Unserialize(Stream& s, char (&a)[N]) { s.read(MakeWritableByteSpan(a)); } +template inline void Unserialize(Stream& s, unsigned char (&a)[N]) { s.read(MakeWritableByteSpan(a)); } +template inline void Unserialize(Stream& s, Span& span) { s.read(AsWritableBytes(span)); } template inline void Serialize(Stream& s, bool a) { uint8_t f = a; ser_writedata8(s, f); } template inline void Unserialize(Stream& s, bool& a) { uint8_t f = ser_readdata8(s); a = f; } @@ -426,11 +419,11 @@ inline unsigned int GetSizeOfFixedBitSet(size_t size) template void WriteFixedBitSet(Stream& s, const std::vector& vec, size_t size) { - std::vector vBytes((size + 7) / 8); + std::vector vBytes((size + 7) / 8); size_t ms = std::min(size, vec.size()); for (size_t p = 0; p < ms; p++) vBytes[p / 8] |= vec[p] << (p % 8); - s.write((char*)vBytes.data(), vBytes.size()); + s.write(AsBytes(Span{vBytes})); } template @@ -438,8 +431,8 @@ void ReadFixedBitSet(Stream& s, std::vector& vec, size_t size) { vec.resize(size); - std::vector vBytes((size + 7) / 8); - s.read((char*)vBytes.data(), vBytes.size()); + std::vector vBytes((size + 7) / 8); + s.read(AsWritableBytes(Span{vBytes})); for (size_t p = 0; p < size; p++) vec[p] = (vBytes[p / 8] & (1 << (p % 8))) != 0; if (vBytes.size() * 8 != size) { @@ -660,10 +653,10 @@ struct CustomUintFormatter if (v < 0 || v > MAX) throw std::ios_base::failure("CustomUintFormatter value out of range"); if (BigEndian) { uint64_t raw = htobe64(v); - s.write(((const char*)&raw) + 8 - Bytes, Bytes); + s.write({BytePtr(&raw) + 8 - Bytes, Bytes}); } else { uint64_t raw = htole64(v); - s.write((const char*)&raw, Bytes); + s.write({BytePtr(&raw), Bytes}); } } @@ -673,10 +666,10 @@ struct CustomUintFormatter static_assert(std::numeric_limits::max() >= MAX && std::numeric_limits::min() <= 0, "Assigned type too small"); uint64_t raw = 0; if (BigEndian) { - s.read(((char*)&raw) + 8 - Bytes, Bytes); + s.read({BytePtr(&raw) + 8 - Bytes, Bytes}); v = static_cast(be64toh(raw)); } else { - s.read((char*)&raw, Bytes); + s.read({BytePtr(&raw), Bytes}); v = static_cast(le64toh(raw)); } } @@ -719,7 +712,7 @@ struct LimitedStringFormatter throw std::ios_base::failure("String length limit exceeded"); } v.resize(size); - if (size != 0) s.read((char*)v.data(), size); + if (size != 0) s.read(MakeWritableByteSpan(v)); } template @@ -929,7 +922,7 @@ void Serialize(Stream& os, const std::basic_string& str) { WriteCompactSize(os, str.size()); if (!str.empty()) - os.write((char*)str.data(), str.size() * sizeof(C)); + os.write(MakeByteSpan(str)); } template @@ -938,7 +931,7 @@ void Unserialize(Stream& is, std::basic_string& str) unsigned int nSize = ReadCompactSize(is); str.resize(nSize); if (nSize != 0) - is.read((char*)str.data(), nSize * sizeof(C)); + is.read(MakeWritableByteSpan(str)); } /** @@ -949,7 +942,7 @@ void Serialize(Stream& os, const std::basic_string_view& str) { WriteCompactSize(os, str.size()); if (!str.empty()) - os.write((char*)str.data(), str.size() * sizeof(C)); + os.write(AsBytes(Span{str.data(), str.size() * sizeof(C)})); } template @@ -958,7 +951,7 @@ void Unserialize(Stream& is, std::basic_string_view& str) unsigned int nSize = ReadCompactSize(is); str.resize(nSize); if (nSize != 0) - is.read((char*)str.data(), nSize * sizeof(C)); + is.read(AsWritableBytes(Span{str.data(), nSize * sizeof(C)})); } @@ -970,7 +963,7 @@ void Serialize_impl(Stream& os, const prevector& v, const unsigned char&) { WriteCompactSize(os, v.size()); if (!v.empty()) - os.write((char*)v.data(), v.size() * sizeof(T)); + os.write(MakeByteSpan(v)); } template @@ -997,7 +990,7 @@ void Unserialize_impl(Stream& is, prevector& v, const unsigned char&) { unsigned int blk = std::min(nSize - i, (unsigned int)(1 + 4999999 / sizeof(T))); v.resize_uninitialized(i + blk); - is.read((char*)&v[i], blk * sizeof(T)); + is.read(AsWritableBytes(Span{&v[i], blk})); i += blk; } } @@ -1024,7 +1017,7 @@ void Serialize_impl(Stream& os, const std::vector& v, const unsigned char& { WriteCompactSize(os, v.size()); if (!v.empty()) - os.write((char*)v.data(), v.size() * sizeof(T)); + os.write(MakeByteSpan(v)); } template @@ -1063,7 +1056,7 @@ void Unserialize_impl(Stream& is, std::vector& v, const unsigned char&) { unsigned int blk = std::min(nSize - i, (unsigned int)(1 + 4999999 / sizeof(T))); v.resize(i + blk); - is.read((char*)&v[i], blk * sizeof(T)); + is.read(AsWritableBytes(Span{&v[i], blk})); i += blk; } } @@ -1365,9 +1358,9 @@ class CSizeComputer public: explicit CSizeComputer(int nVersionIn) : nSize(0), nVersion(nVersionIn) {} - void write(const char *psz, size_t _nSize) + void write(Span src) { - this->nSize += _nSize; + this->nSize += src.size(); } /** Pretend _nSize bytes are written, without specifying them. */ diff --git a/src/streams.h b/src/streams.h index ddf0a80cdc4f..af7e49d0753a 100644 --- a/src/streams.h +++ b/src/streams.h @@ -49,14 +49,14 @@ class OverrideStream return (*this); } - void write(const char* pch, size_t nSize) + void write(Span src) { - stream->write(pch, nSize); + stream->write(src); } - void read(char* pch, size_t nSize) + void read(Span dst) { - stream->read(pch, nSize); + stream->read(dst); } int GetVersion() const { return nVersion; } @@ -94,17 +94,17 @@ class CVectorWriter { ::SerializeMany(*this, std::forward(args)...); } - void write(const char* pch, size_t nSize) + void write(Span src) { assert(nPos <= vchData.size()); - size_t nOverwrite = std::min(nSize, vchData.size() - nPos); + size_t nOverwrite = std::min(src.size(), vchData.size() - nPos); if (nOverwrite) { - memcpy(vchData.data() + nPos, reinterpret_cast(pch), nOverwrite); + memcpy(vchData.data() + nPos, src.data(), nOverwrite); } - if (nOverwrite < nSize) { - vchData.insert(vchData.end(), reinterpret_cast(pch) + nOverwrite, reinterpret_cast(pch) + nSize); + if (nOverwrite < src.size()) { + vchData.insert(vchData.end(), UCharCast(src.data()) + nOverwrite, UCharCast(src.end())); } - nPos += nSize; + nPos += src.size(); } template CVectorWriter& operator<<(const T& obj) @@ -184,18 +184,18 @@ class SpanReader size_t size() const { return m_data.size(); } bool empty() const { return m_data.empty(); } - void read(char* dst, size_t n) + void read(Span dst) { - if (n == 0) { + if (dst.size() == 0) { return; } // Read from the beginning of the buffer - if (n > m_data.size()) { + if (dst.size() > m_data.size()) { throw std::ios_base::failure("SpanReader::read(): end of data"); } - memcpy(dst, m_data.data(), n); - m_data = m_data.subspan(n); + memcpy(dst.data(), m_data.data(), dst.size()); + m_data = m_data.subspan(dst.size()); } }; @@ -229,6 +229,7 @@ class CDataStream : nType{nTypeIn}, nVersion{nVersionIn} {} + explicit CDataStream(Span sp, int type, int version) : CDataStream{AsBytes(sp), type, version} {} explicit CDataStream(Span sp, int nTypeIn, int nVersionIn) : vch(sp.data(), sp.data() + sp.size()), nType{nTypeIn}, @@ -244,7 +245,7 @@ class CDataStream std::string str() const { - return (std::string(begin(), end())); + return std::string{UCharCast(data()), UCharCast(data() + size())}; } @@ -365,16 +366,16 @@ class CDataStream void SetVersion(int n) { nVersion = n; } int GetVersion() const { return nVersion; } - void read(char* pch, size_t nSize) + void read(Span dst) { - if (nSize == 0) return; + if (dst.size() == 0) return; // Read from the beginning of the buffer - unsigned int nReadPosNext = nReadPos + nSize; + unsigned int nReadPosNext = nReadPos + dst.size(); if (nReadPosNext > vch.size()) { throw std::ios_base::failure("CDataStream::read(): end of data"); } - memcpy(pch, &vch[nReadPos], nSize); + memcpy(dst.data(), &vch[nReadPos], dst.size()); if (nReadPosNext == vch.size()) { nReadPos = 0; @@ -402,10 +403,10 @@ class CDataStream nReadPos = nReadPosNext; } - void write(const char* pch, size_t nSize) + void write(Span src) { // Write to the end of the buffer - vch.insert(vch.end(), pch, pch + nSize); + vch.insert(vch.end(), src.begin(), src.end()); } template @@ -413,7 +414,7 @@ class CDataStream { // Special case: stream << stream concatenates like stream += stream if (!vch.empty()) - s.write((char*)vch.data(), vch.size() * sizeof(value_type)); + s.write(MakeByteSpan(vch)); } template @@ -444,7 +445,7 @@ class CDataStream } for (size_type i = 0, j = 0; i != size(); i++) { - vch[i] ^= key[j++]; + vch[i] ^= std::byte{key[j++]}; // This potentially acts on very many bytes of data, so it's // important that we calculate `j`, i.e. the `key` index in this @@ -617,12 +618,13 @@ class CAutoFile int GetType() const { return nType; } int GetVersion() const { return nVersion; } - void read(char* pch, size_t nSize) + void read(Span dst) { if (!file) throw std::ios_base::failure("CAutoFile::read: file handle is nullptr"); - if (fread(pch, 1, nSize, file) != nSize) + if (fread(dst.data(), 1, dst.size(), file) != dst.size()) { throw std::ios_base::failure(feof(file) ? "CAutoFile::read: end of file" : "CAutoFile::read: fread failed"); + } } void ignore(size_t nSize) @@ -638,12 +640,13 @@ class CAutoFile } } - void write(const char* pch, size_t nSize) + void write(Span src) { if (!file) throw std::ios_base::failure("CAutoFile::write: file handle is nullptr"); - if (fwrite(pch, 1, nSize, file) != nSize) + if (fwrite(src.data(), 1, src.size(), file) != src.size()) { throw std::ios_base::failure("CAutoFile::write: write failed"); + } } template @@ -684,7 +687,7 @@ class CBufferedFile uint64_t nReadPos; //!< how many bytes have been read from this uint64_t nReadLimit; //!< up to which position we're allowed to read uint64_t nRewind; //!< how many bytes we guarantee to rewind - std::vector vchBuf; //!< the buffer + std::vector vchBuf; //!< the buffer protected: //! read data from the source to fill the buffer @@ -705,8 +708,8 @@ class CBufferedFile } public: - CBufferedFile(FILE *fileIn, uint64_t nBufSize, uint64_t nRewindIn, int nTypeIn, int nVersionIn) : - nType(nTypeIn), nVersion(nVersionIn), nSrcPos(0), nReadPos(0), nReadLimit(std::numeric_limits::max()), nRewind(nRewindIn), vchBuf(nBufSize, 0) + CBufferedFile(FILE* fileIn, uint64_t nBufSize, uint64_t nRewindIn, int nTypeIn, int nVersionIn) + : nType(nTypeIn), nVersion(nVersionIn), nSrcPos(0), nReadPos(0), nReadLimit(std::numeric_limits::max()), nRewind(nRewindIn), vchBuf(nBufSize, std::byte{0}) { if (nRewindIn >= nBufSize) throw std::ios_base::failure("Rewind limit must be less than buffer size"); @@ -739,22 +742,23 @@ class CBufferedFile } //! read a number of bytes - void read(char *pch, size_t nSize) { - if (nSize + nReadPos > nReadLimit) + void read(Span dst) + { + if (dst.size() + nReadPos > nReadLimit) { throw std::ios_base::failure("Read attempted past buffer limit"); - while (nSize > 0) { + } + while (dst.size() > 0) { if (nReadPos == nSrcPos) Fill(); unsigned int pos = nReadPos % vchBuf.size(); - size_t nNow = nSize; + size_t nNow = dst.size(); if (nNow + pos > vchBuf.size()) nNow = vchBuf.size() - pos; if (nNow + nReadPos > nSrcPos) nNow = nSrcPos - nReadPos; - memcpy(pch, &vchBuf[pos], nNow); + memcpy(dst.data(), &vchBuf[pos], nNow); nReadPos += nNow; - pch += nNow; - nSize -= nNow; + dst = dst.subspan(nNow); } } @@ -797,12 +801,14 @@ class CBufferedFile } //! search for a given byte in the stream, and remain positioned on it - void FindByte(char ch) { + void FindByte(uint8_t ch) + { while (true) { if (nReadPos == nSrcPos) Fill(); - if (vchBuf[nReadPos % vchBuf.size()] == ch) + if (vchBuf[nReadPos % vchBuf.size()] == std::byte{ch}) { break; + } nReadPos++; } } diff --git a/src/support/allocators/zeroafterfree.h b/src/support/allocators/zeroafterfree.h index 623a936fe2a4..a782a4ffab04 100644 --- a/src/support/allocators/zeroafterfree.h +++ b/src/support/allocators/zeroafterfree.h @@ -41,6 +41,6 @@ struct zero_after_free_allocator : public std::allocator { }; /** Byte-vector that clears its contents before deletion. */ -using SerializeData = std::vector>; +using SerializeData = std::vector>; #endif // BITCOIN_SUPPORT_ALLOCATORS_ZEROAFTERFREE_H diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index 620299922911..da08a9b30b21 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -41,8 +41,9 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize) stream << filter; std::vector expected = ParseHex("03614e9b050000000000000001"); + auto result{MakeUCharSpan(stream)}; - BOOST_CHECK_EQUAL_COLLECTIONS(stream.begin(), stream.end(), expected.begin(), expected.end()); + BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(), expected.begin(), expected.end()); BOOST_CHECK_MESSAGE( filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter doesn't contain just-inserted object!"); } @@ -67,8 +68,9 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize_with_tweak) stream << filter; std::vector expected = ParseHex("03ce4299050000000100008001"); + auto result{MakeUCharSpan(stream)}; - BOOST_CHECK_EQUAL_COLLECTIONS(stream.begin(), stream.end(), expected.begin(), expected.end()); + BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(), expected.begin(), expected.end()); } BOOST_AUTO_TEST_CASE(bloom_create_insert_key) @@ -87,8 +89,9 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_key) stream << filter; std::vector expected = ParseHex("038fc16b080000000000000001"); + auto result{MakeUCharSpan(stream)}; - BOOST_CHECK_EQUAL_COLLECTIONS(stream.begin(), stream.end(), expected.begin(), expected.end()); + BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(), expected.begin(), expected.end()); } BOOST_AUTO_TEST_CASE(bloom_match) @@ -443,8 +446,9 @@ BOOST_AUTO_TEST_CASE(merkle_block_3_and_serialize) merkleStream << merkleBlock; std::vector expected = ParseHex("0100000079cda856b143d9db2c1caff01d1aecc8630d30625d10e8b4b8b0000000000000b50cc069d6a3e33e3ff84a5c41d9d3febe7c770fdcc96b2c3ff60abe184f196367291b4d4c86041b8fa45d630100000001b50cc069d6a3e33e3ff84a5c41d9d3febe7c770fdcc96b2c3ff60abe184f19630101"); + auto result{MakeUCharSpan(merkleStream)}; - BOOST_CHECK_EQUAL_COLLECTIONS(expected.begin(), expected.end(), merkleStream.begin(), merkleStream.end()); + BOOST_CHECK_EQUAL_COLLECTIONS(expected.begin(), expected.end(), result.begin(), result.end()); } BOOST_AUTO_TEST_CASE(merkle_block_4) diff --git a/src/test/fuzz/autofile.cpp b/src/test/fuzz/autofile.cpp index c43e5b90e886..bd7c9162cc40 100644 --- a/src/test/fuzz/autofile.cpp +++ b/src/test/fuzz/autofile.cpp @@ -22,16 +22,16 @@ FUZZ_TARGET(autofile) CallOneOf( fuzzed_data_provider, [&] { - std::array arr{}; + std::array arr{}; try { - auto_file.read((char*)arr.data(), fuzzed_data_provider.ConsumeIntegralInRange(0, 4096)); + auto_file.read({arr.data(), fuzzed_data_provider.ConsumeIntegralInRange(0, 4096)}); } catch (const std::ios_base::failure&) { } }, [&] { - const std::array arr{}; + const std::array arr{}; try { - auto_file.write((const char*)arr.data(), fuzzed_data_provider.ConsumeIntegralInRange(0, 4096)); + auto_file.write({arr.data(), fuzzed_data_provider.ConsumeIntegralInRange(0, 4096)}); } catch (const std::ios_base::failure&) { } }, diff --git a/src/test/fuzz/buffered_file.cpp b/src/test/fuzz/buffered_file.cpp index b32a59986aaa..5d6a9164c15f 100644 --- a/src/test/fuzz/buffered_file.cpp +++ b/src/test/fuzz/buffered_file.cpp @@ -33,9 +33,9 @@ FUZZ_TARGET(buffered_file) CallOneOf( fuzzed_data_provider, [&] { - std::array arr{}; + std::array arr{}; try { - opt_buffered_file->read((char*)arr.data(), fuzzed_data_provider.ConsumeIntegralInRange(0, 4096)); + opt_buffered_file->read({arr.data(), fuzzed_data_provider.ConsumeIntegralInRange(0, 4096)}); } catch (const std::ios_base::failure&) { } }, @@ -53,7 +53,7 @@ FUZZ_TARGET(buffered_file) return; } try { - opt_buffered_file->FindByte(fuzzed_data_provider.ConsumeIntegral()); + opt_buffered_file->FindByte(fuzzed_data_provider.ConsumeIntegral()); } catch (const std::ios_base::failure&) { } }, diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index 6a4d7896b082..e96e556b9cd4 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -215,11 +215,6 @@ FUZZ_TARGET_INIT(integer, initialize_integer) stream >> deserialized_i8; assert(i8 == deserialized_i8 && stream.empty()); - char deserialized_ch; - stream << ch; - stream >> deserialized_ch; - assert(ch == deserialized_ch && stream.empty()); - bool deserialized_b; stream << b; stream >> deserialized_b; diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index d16014148e7d..832cc1d107b3 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -498,7 +498,6 @@ void WriteToStream(FuzzedDataProvider& fuzzed_data_provider, Stream& stream) noe CallOneOf( fuzzed_data_provider, WRITE_TO_STREAM_CASE(bool, fuzzed_data_provider.ConsumeBool()), - WRITE_TO_STREAM_CASE(char, fuzzed_data_provider.ConsumeIntegral()), WRITE_TO_STREAM_CASE(int8_t, fuzzed_data_provider.ConsumeIntegral()), WRITE_TO_STREAM_CASE(uint8_t, fuzzed_data_provider.ConsumeIntegral()), WRITE_TO_STREAM_CASE(int16_t, fuzzed_data_provider.ConsumeIntegral()), @@ -508,7 +507,7 @@ void WriteToStream(FuzzedDataProvider& fuzzed_data_provider, Stream& stream) noe WRITE_TO_STREAM_CASE(int64_t, fuzzed_data_provider.ConsumeIntegral()), WRITE_TO_STREAM_CASE(uint64_t, fuzzed_data_provider.ConsumeIntegral()), WRITE_TO_STREAM_CASE(std::string, fuzzed_data_provider.ConsumeRandomLengthString(32)), - WRITE_TO_STREAM_CASE(std::vector, ConsumeRandomLengthIntegralVector(fuzzed_data_provider))); + WRITE_TO_STREAM_CASE(std::vector, ConsumeRandomLengthIntegralVector(fuzzed_data_provider))); } catch (const std::ios_base::failure&) { break; } @@ -528,7 +527,6 @@ void ReadFromStream(FuzzedDataProvider& fuzzed_data_provider, Stream& stream) no CallOneOf( fuzzed_data_provider, READ_FROM_STREAM_CASE(bool), - READ_FROM_STREAM_CASE(char), READ_FROM_STREAM_CASE(int8_t), READ_FROM_STREAM_CASE(uint8_t), READ_FROM_STREAM_CASE(int16_t), @@ -538,7 +536,7 @@ void ReadFromStream(FuzzedDataProvider& fuzzed_data_provider, Stream& stream) no READ_FROM_STREAM_CASE(int64_t), READ_FROM_STREAM_CASE(uint64_t), READ_FROM_STREAM_CASE(std::string), - READ_FROM_STREAM_CASE(std::vector)); + READ_FROM_STREAM_CASE(std::vector)); } catch (const std::ios_base::failure&) { break; } diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index a24d313e5e65..57760235754a 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -147,7 +147,7 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, uint32_t flag uint32_t libconsensus_flags{flags & dashconsensus_SCRIPT_FLAGS_VERIFY_ALL}; if (libconsensus_flags == flags) { int expectedSuccessCode = expect ? 1 : 0; - BOOST_CHECK_MESSAGE(dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), stream.data(), stream.size(), 0, libconsensus_flags, nullptr) == expectedSuccessCode, message); + BOOST_CHECK_MESSAGE(dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), UCharCast(stream.data()), stream.size(), 0, libconsensus_flags, nullptr) == expectedSuccessCode, message); } #endif } @@ -1421,7 +1421,7 @@ BOOST_AUTO_TEST_CASE(dashconsensus_verify_script_returns_true) stream << spendTx; dashconsensus_error err; - int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), stream.data(), stream.size(), nIn, libconsensus_flags, &err); + int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), UCharCast(stream.data()), stream.size(), nIn, libconsensus_flags, &err); BOOST_CHECK_EQUAL(result, 1); BOOST_CHECK_EQUAL(err, dashconsensus_ERR_OK); } @@ -1443,7 +1443,7 @@ BOOST_AUTO_TEST_CASE(dashconsensus_verify_script_tx_index_err) stream << spendTx; dashconsensus_error err; - int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), stream.data(), stream.size(), nIn, libconsensus_flags, &err); + int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), UCharCast(stream.data()), stream.size(), nIn, libconsensus_flags, &err); BOOST_CHECK_EQUAL(result, 0); BOOST_CHECK_EQUAL(err, dashconsensus_ERR_TX_INDEX); } @@ -1465,7 +1465,7 @@ BOOST_AUTO_TEST_CASE(dashconsensus_verify_script_tx_size) stream << spendTx; dashconsensus_error err; - int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), stream.data(), stream.size() * 2, nIn, libconsensus_flags, &err); + int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), UCharCast(stream.data()), stream.size() * 2, nIn, libconsensus_flags, &err); BOOST_CHECK_EQUAL(result, 0); BOOST_CHECK_EQUAL(err, dashconsensus_ERR_TX_SIZE_MISMATCH); } @@ -1487,7 +1487,7 @@ BOOST_AUTO_TEST_CASE(dashconsensus_verify_script_tx_serialization) stream << 0xffffffff; dashconsensus_error err; - int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), stream.data(), stream.size(), nIn, libconsensus_flags, &err); + int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), UCharCast(stream.data()), stream.size(), nIn, libconsensus_flags, &err); BOOST_CHECK_EQUAL(result, 0); BOOST_CHECK_EQUAL(err, dashconsensus_ERR_TX_DESERIALIZE); } @@ -1509,7 +1509,7 @@ BOOST_AUTO_TEST_CASE(dashconsensus_verify_script_invalid_flags) stream << spendTx; dashconsensus_error err; - int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), stream.data(), stream.size(), nIn, libconsensus_flags, &err); + int result = dashconsensus_verify_script(scriptPubKey.data(), scriptPubKey.size(), UCharCast(stream.data()), stream.size(), nIn, libconsensus_flags, &err); BOOST_CHECK_EQUAL(result, 0); BOOST_CHECK_EQUAL(err, dashconsensus_ERR_INVALID_FLAGS); } diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 0f532f91d8c6..fbc17bc8ab2e 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -61,7 +61,7 @@ class CSerializeMethodsTestMany : public CSerializeMethodsTestSingle BOOST_AUTO_TEST_CASE(sizes) { - BOOST_CHECK_EQUAL(sizeof(char), GetSerializeSize(char(0), 0)); + BOOST_CHECK_EQUAL(sizeof(unsigned char), GetSerializeSize((unsigned char)0, 0)); BOOST_CHECK_EQUAL(sizeof(int8_t), GetSerializeSize(int8_t(0), 0)); BOOST_CHECK_EQUAL(sizeof(uint8_t), GetSerializeSize(uint8_t(0), 0)); BOOST_CHECK_EQUAL(sizeof(int16_t), GetSerializeSize(int16_t(0), 0)); @@ -74,7 +74,7 @@ BOOST_AUTO_TEST_CASE(sizes) BOOST_CHECK_EQUAL(sizeof(uint8_t), GetSerializeSize(bool(0), 0)); // Sanity-check GetSerializeSize and c++ type matching - BOOST_CHECK_EQUAL(GetSerializeSize(char(0), 0), 1U); + BOOST_CHECK_EQUAL(GetSerializeSize((unsigned char)0, 0), 1U); BOOST_CHECK_EQUAL(GetSerializeSize(int8_t(0), 0), 1U); BOOST_CHECK_EQUAL(GetSerializeSize(uint8_t(0), 0), 1U); BOOST_CHECK_EQUAL(GetSerializeSize(int16_t(0), 0), 2U); @@ -186,76 +186,78 @@ BOOST_AUTO_TEST_CASE(noncanonical) std::vector::size_type n; // zero encoded with three bytes: - ss.write("\xfd\x00\x00", 3); + ss.write(MakeByteSpan("\xfd\x00\x00").first(3)); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // 0xfc encoded with three bytes: - ss.write("\xfd\xfc\x00", 3); + ss.write(MakeByteSpan("\xfd\xfc\x00").first(3)); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // 0xfd encoded with three bytes is OK: - ss.write("\xfd\xfd\x00", 3); + ss.write(MakeByteSpan("\xfd\xfd\x00").first(3)); n = ReadCompactSize(ss); BOOST_CHECK(n == 0xfd); // zero encoded with five bytes: - ss.write("\xfe\x00\x00\x00\x00", 5); + ss.write(MakeByteSpan("\xfe\x00\x00\x00\x00").first(5)); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // 0xffff encoded with five bytes: - ss.write("\xfe\xff\xff\x00\x00", 5); + ss.write(MakeByteSpan("\xfe\xff\xff\x00\x00").first(5)); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // zero encoded with nine bytes: - ss.write("\xff\x00\x00\x00\x00\x00\x00\x00\x00", 9); + ss.write(MakeByteSpan("\xff\x00\x00\x00\x00\x00\x00\x00\x00").first(9)); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); // 0x01ffffff encoded with nine bytes: - ss.write("\xff\xff\xff\xff\x01\x00\x00\x00\x00", 9); + ss.write(MakeByteSpan("\xff\xff\xff\xff\x01\x00\x00\x00\x00").first(9)); BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); } BOOST_AUTO_TEST_CASE(insert_delete) { + constexpr auto B2I{[](std::byte b) { return std::to_integer(b); }}; + // Test inserting/deleting bytes. CDataStream ss(SER_DISK, 0); BOOST_CHECK_EQUAL(ss.size(), 0U); - ss.write("\x00\x01\x02\xff", 4); + ss.write(MakeByteSpan("\x00\x01\x02\xff").first(4)); BOOST_CHECK_EQUAL(ss.size(), 4U); - char c = (char)11; + uint8_t c{11}; // Inserting at beginning/end/middle: - ss.insert(ss.begin(), c); + ss.insert(ss.begin(), std::byte{c}); BOOST_CHECK_EQUAL(ss.size(), 5U); - BOOST_CHECK_EQUAL(ss[0], c); - BOOST_CHECK_EQUAL(ss[1], 0); + BOOST_CHECK_EQUAL(B2I(ss[0]), c); + BOOST_CHECK_EQUAL(B2I(ss[1]), 0); - ss.insert(ss.end(), c); + ss.insert(ss.end(), std::byte{c}); BOOST_CHECK_EQUAL(ss.size(), 6U); - BOOST_CHECK_EQUAL(ss[4], 0xff); - BOOST_CHECK_EQUAL(ss[5], c); + BOOST_CHECK_EQUAL(B2I(ss[4]), 0xff); + BOOST_CHECK_EQUAL(B2I(ss[5]), c); - ss.insert(ss.begin()+2, c); + ss.insert(ss.begin() + 2, std::byte{c}); BOOST_CHECK_EQUAL(ss.size(), 7U); - BOOST_CHECK_EQUAL(ss[2], c); + BOOST_CHECK_EQUAL(B2I(ss[2]), c); // Delete at beginning/end/middle ss.erase(ss.begin()); BOOST_CHECK_EQUAL(ss.size(), 6U); - BOOST_CHECK_EQUAL(ss[0], 0); + BOOST_CHECK_EQUAL(B2I(ss[0]), 0); ss.erase(ss.begin()+ss.size()-1); BOOST_CHECK_EQUAL(ss.size(), 5U); - BOOST_CHECK_EQUAL(ss[4], 0xff); + BOOST_CHECK_EQUAL(B2I(ss[4]), 0xff); ss.erase(ss.begin()+1); BOOST_CHECK_EQUAL(ss.size(), 4U); - BOOST_CHECK_EQUAL(ss[0], 0); - BOOST_CHECK_EQUAL(ss[1], 1); - BOOST_CHECK_EQUAL(ss[2], 2); - BOOST_CHECK_EQUAL(ss[3], 0xff); + BOOST_CHECK_EQUAL(B2I(ss[0]), 0); + BOOST_CHECK_EQUAL(B2I(ss[1]), 1); + BOOST_CHECK_EQUAL(B2I(ss[2]), 2); + BOOST_CHECK_EQUAL(B2I(ss[3]), 0xff); } // Change struct size and check if it can be deserialized diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 5a4c5fc988e4..e22f97384836 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -160,7 +160,7 @@ BOOST_AUTO_TEST_CASE(bitstream_reader_writer) BOOST_AUTO_TEST_CASE(streams_serializedata_xor) { - std::vector in; + std::vector in; std::vector expected_xor; std::vector key; CDataStream ds(in, 0, 0); @@ -174,8 +174,8 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor) std::string(expected_xor.begin(), expected_xor.end()), ds.str()); - in.push_back('\x0f'); - in.push_back('\xf0'); + in.push_back(std::byte{0x0f}); + in.push_back(std::byte{0xf0}); expected_xor.push_back('\xf0'); expected_xor.push_back('\x0f'); @@ -195,8 +195,8 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor) in.clear(); expected_xor.clear(); - in.push_back('\xf0'); - in.push_back('\x0f'); + in.push_back(std::byte{0xf0}); + in.push_back(std::byte{0x0f}); expected_xor.push_back('\x0f'); expected_xor.push_back('\x00'); diff --git a/src/txdb.cpp b/src/txdb.cpp index d4ca0656c811..9018be6849dd 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -191,7 +191,7 @@ class CCoinsViewDBCursor: public CCoinsViewCursor private: std::unique_ptr pcursor; - std::pair keyTmp; + std::pair keyTmp; friend class CCoinsViewDB; }; @@ -299,7 +299,7 @@ bool CBlockTreeDB::ReadAddressUnspentIndex(uint160 addressHash, AddressType type pcursor->Seek(std::make_pair(DB_ADDRESSUNSPENTINDEX, CAddressIndexIteratorKey(type, addressHash))); while (pcursor->Valid()) { - std::pair key; + std::pair key; if (pcursor->GetKey(key) && key.first == DB_ADDRESSUNSPENTINDEX && key.second.m_address_bytes == addressHash) { CAddressUnspentValue nValue; if (pcursor->GetValue(nValue)) { @@ -343,7 +343,7 @@ bool CBlockTreeDB::ReadAddressIndex(uint160 addressHash, AddressType type, } while (pcursor->Valid()) { - std::pair key; + std::pair key; if (pcursor->GetKey(key) && key.first == DB_ADDRESSINDEX && key.second.m_address_bytes == addressHash) { if (end > 0 && key.second.m_block_height > end) { break; @@ -383,7 +383,7 @@ bool CBlockTreeDB::ReadTimestampIndex(const unsigned int &high, const unsigned i pcursor->Seek(std::make_pair(DB_TIMESTAMPINDEX, CTimestampIndexIteratorKey(low))); while (pcursor->Valid()) { - std::pair key; + std::pair key; if (pcursor->GetKey(key) && key.first == DB_TIMESTAMPINDEX && key.second.m_block_time <= high) { hashes.push_back(key.second.m_block_hash); pcursor->Next(); diff --git a/src/uint256.h b/src/uint256.h index b8a0646ce560..761893768e6f 100644 --- a/src/uint256.h +++ b/src/uint256.h @@ -78,13 +78,13 @@ class base_blob template void Serialize(Stream& s) const { - s.write((char*)m_data.data(), sizeof(m_data)); + s.write(MakeByteSpan(m_data)); } template void Unserialize(Stream& s) { - s.read((char*)m_data.data(), sizeof(m_data)); + s.read(MakeWritableByteSpan(m_data)); } }; diff --git a/src/validation.cpp b/src/validation.cpp index a381a6b55562..bced403ed16c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5179,7 +5179,7 @@ void CChainState::LoadExternalBlockFile(FILE* fileIn, FlatFilePos* dbp) try { // locate a header unsigned char buf[CMessageHeader::MESSAGE_START_SIZE]; - blkdat.FindByte(char(m_params.MessageStart()[0])); + blkdat.FindByte(m_params.MessageStart()[0]); nRewind = blkdat.GetPos() + 1; blkdat >> buf; if (memcmp(buf, m_params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) { diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 640c99e564f9..5b21fe4e24f0 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -680,10 +680,10 @@ bool BerkeleyBatch::ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& // Convert to streams ssKey.SetType(SER_DISK); ssKey.clear(); - ssKey.write((char*)datKey.get_data(), datKey.get_size()); + ssKey.write({BytePtr(datKey.get_data()), datKey.get_size()}); ssValue.SetType(SER_DISK); ssValue.clear(); - ssValue.write((char*)datValue.get_data(), datValue.get_size()); + ssValue.write({BytePtr(datValue.get_data()), datValue.get_size()}); return true; } @@ -755,7 +755,7 @@ bool BerkeleyBatch::ReadKey(CDataStream&& key, CDataStream& value) SafeDbt datValue; int ret = pdb->get(activeTxn, datKey, datValue, 0); if (ret == 0 && datValue.get_data() != nullptr) { - value.write((char*)datValue.get_data(), datValue.get_size()); + value.write({BytePtr(datValue.get_data()), datValue.get_size()}); return true; } return false; diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index 23e55d376cfc..9c442f5fa4b4 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -394,9 +394,9 @@ bool SQLiteBatch::ReadKey(CDataStream&& key, CDataStream& value) return false; } // Leftmost column in result is index 0 - const char* data = reinterpret_cast(sqlite3_column_blob(m_read_stmt, 0)); - int data_size = sqlite3_column_bytes(m_read_stmt, 0); - value.write(data, data_size); + const std::byte* data{BytePtr(sqlite3_column_blob(m_read_stmt, 0))}; + size_t data_size(sqlite3_column_bytes(m_read_stmt, 0)); + value.write({data, data_size}); sqlite3_clear_bindings(m_read_stmt); sqlite3_reset(m_read_stmt); @@ -511,12 +511,12 @@ bool SQLiteBatch::ReadAtCursor(CDataStream& key, CDataStream& value, bool& compl } // Leftmost column in result is index 0 - const char* key_data = reinterpret_cast(sqlite3_column_blob(m_cursor_stmt, 0)); - int key_data_size = sqlite3_column_bytes(m_cursor_stmt, 0); - key.write(key_data, key_data_size); - const char* value_data = reinterpret_cast(sqlite3_column_blob(m_cursor_stmt, 1)); - int value_data_size = sqlite3_column_bytes(m_cursor_stmt, 1); - value.write(value_data, value_data_size); + const std::byte* key_data{BytePtr(sqlite3_column_blob(m_cursor_stmt, 0))}; + size_t key_data_size(sqlite3_column_bytes(m_cursor_stmt, 0)); + key.write({key_data, key_data_size}); + const std::byte* value_data{BytePtr(sqlite3_column_blob(m_cursor_stmt, 1))}; + size_t value_data_size(sqlite3_column_bytes(m_cursor_stmt, 1)); + value.write({value_data, value_data_size}); return true; } From baf8dd65cdf31ba674dcd1dd09329a8c5a9394dc Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 28 Jan 2022 10:29:49 +0100 Subject: [PATCH 08/17] merge bitcoin#24190: Fix sanitizer suppresions in streams_tests --- src/test/streams_tests.cpp | 18 ++++-------------- test/sanitizer_suppressions/ubsan | 1 - 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index e22f97384836..3054ad2f6d53 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -162,14 +162,10 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor) { std::vector in; std::vector expected_xor; - std::vector key; CDataStream ds(in, 0, 0); // Degenerate case - - key.push_back('\x00'); - key.push_back('\x00'); - ds.Xor(key); + ds.Xor({0x00, 0x00}); BOOST_CHECK_EQUAL( std::string(expected_xor.begin(), expected_xor.end()), ds.str()); @@ -183,10 +179,8 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor) ds.clear(); ds.insert(ds.begin(), in.begin(), in.end()); - key.clear(); - key.push_back('\xff'); - ds.Xor(key); + ds.Xor({0xff}); BOOST_CHECK_EQUAL( std::string(expected_xor.begin(), expected_xor.end()), ds.str()); @@ -203,11 +197,7 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor) ds.clear(); ds.insert(ds.begin(), in.begin(), in.end()); - key.clear(); - key.push_back('\xff'); - key.push_back('\x0f'); - - ds.Xor(key); + ds.Xor({0xff, 0x0f}); BOOST_CHECK_EQUAL( std::string(expected_xor.begin(), expected_xor.end()), ds.str()); @@ -418,7 +408,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand) size_t find = currentPos + InsecureRandRange(8); if (find >= fileSize) find = fileSize - 1; - bf.FindByte(static_cast(find)); + bf.FindByte(uint8_t(find)); // The value at each offset is the offset. BOOST_CHECK_EQUAL(bf.GetPos(), find); currentPos = find; diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index 3a8048e58c58..a306ca62a6c4 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -70,7 +70,6 @@ implicit-integer-sign-change:test/pow_tests.cpp implicit-integer-sign-change:test/prevector_tests.cpp implicit-integer-sign-change:test/sighash_tests.cpp implicit-integer-sign-change:test/skiplist_tests.cpp -implicit-integer-sign-change:test/streams_tests.cpp implicit-integer-sign-change:test/transaction_tests.cpp implicit-integer-sign-change:txmempool.cpp implicit-integer-sign-change:util/strencodings.cpp From 24af37256f5a7dd3ca11f37deed69aa65909b1b1 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 3 Feb 2022 19:45:45 +0100 Subject: [PATCH 09/17] merge bitcoin#24253: Remove broken and unused CDataStream methods --- src/streams.h | 67 ------------------------------------ src/test/serialize_tests.cpp | 45 ------------------------ src/test/streams_tests.cpp | 43 +++++++++-------------- 3 files changed, 17 insertions(+), 138 deletions(-) diff --git a/src/streams.h b/src/streams.h index af7e49d0753a..090609442363 100644 --- a/src/streams.h +++ b/src/streams.h @@ -263,76 +263,9 @@ class CDataStream const_reference operator[](size_type pos) const { return vch[pos + nReadPos]; } reference operator[](size_type pos) { return vch[pos + nReadPos]; } void clear() { vch.clear(); nReadPos = 0; } - iterator insert(iterator it, const value_type x) { return vch.insert(it, x); } - void insert(iterator it, size_type n, const value_type x) { vch.insert(it, n, x); } value_type* data() { return vch.data() + nReadPos; } const value_type* data() const { return vch.data() + nReadPos; } - void insert(iterator it, std::vector::const_iterator first, std::vector::const_iterator last) - { - if (last == first) return; - assert(last - first > 0); - if (it == vch.begin() + nReadPos && (unsigned int)(last - first) <= nReadPos) - { - // special case for inserting at the front when there's room - nReadPos -= (last - first); - memcpy(&vch[nReadPos], &first[0], last - first); - } - else - vch.insert(it, first, last); - } - - void insert(iterator it, const value_type* first, const value_type* last) - { - if (last == first) return; - assert(last - first > 0); - if (it == vch.begin() + nReadPos && (unsigned int)(last - first) <= nReadPos) - { - // special case for inserting at the front when there's room - nReadPos -= (last - first); - memcpy(&vch[nReadPos], &first[0], last - first); - } - else - vch.insert(it, first, last); - } - - iterator erase(iterator it) - { - if (it == vch.begin() + nReadPos) - { - // special case for erasing from the front - if (++nReadPos >= vch.size()) - { - // whenever we reach the end, we take the opportunity to clear the buffer - nReadPos = 0; - return vch.erase(vch.begin(), vch.end()); - } - return vch.begin() + nReadPos; - } - else - return vch.erase(it); - } - - iterator erase(iterator first, iterator last) - { - if (first == vch.begin() + nReadPos) - { - // special case for erasing from the front - if (last == vch.end()) - { - nReadPos = 0; - return vch.erase(vch.begin(), vch.end()); - } - else - { - nReadPos = (last - vch.begin()); - return last; - } - } - else - return vch.erase(first, last); - } - inline void Compact() { vch.erase(vch.begin(), vch.begin() + nReadPos); diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index fbc17bc8ab2e..92212e8d8e92 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -215,51 +215,6 @@ BOOST_AUTO_TEST_CASE(noncanonical) BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); } -BOOST_AUTO_TEST_CASE(insert_delete) -{ - constexpr auto B2I{[](std::byte b) { return std::to_integer(b); }}; - - // Test inserting/deleting bytes. - CDataStream ss(SER_DISK, 0); - BOOST_CHECK_EQUAL(ss.size(), 0U); - - ss.write(MakeByteSpan("\x00\x01\x02\xff").first(4)); - BOOST_CHECK_EQUAL(ss.size(), 4U); - - uint8_t c{11}; - - // Inserting at beginning/end/middle: - ss.insert(ss.begin(), std::byte{c}); - BOOST_CHECK_EQUAL(ss.size(), 5U); - BOOST_CHECK_EQUAL(B2I(ss[0]), c); - BOOST_CHECK_EQUAL(B2I(ss[1]), 0); - - ss.insert(ss.end(), std::byte{c}); - BOOST_CHECK_EQUAL(ss.size(), 6U); - BOOST_CHECK_EQUAL(B2I(ss[4]), 0xff); - BOOST_CHECK_EQUAL(B2I(ss[5]), c); - - ss.insert(ss.begin() + 2, std::byte{c}); - BOOST_CHECK_EQUAL(ss.size(), 7U); - BOOST_CHECK_EQUAL(B2I(ss[2]), c); - - // Delete at beginning/end/middle - ss.erase(ss.begin()); - BOOST_CHECK_EQUAL(ss.size(), 6U); - BOOST_CHECK_EQUAL(B2I(ss[0]), 0); - - ss.erase(ss.begin()+ss.size()-1); - BOOST_CHECK_EQUAL(ss.size(), 5U); - BOOST_CHECK_EQUAL(B2I(ss[4]), 0xff); - - ss.erase(ss.begin()+1); - BOOST_CHECK_EQUAL(ss.size(), 4U); - BOOST_CHECK_EQUAL(B2I(ss[0]), 0); - BOOST_CHECK_EQUAL(B2I(ss[1]), 1); - BOOST_CHECK_EQUAL(B2I(ss[2]), 2); - BOOST_CHECK_EQUAL(B2I(ss[3]), 0xff); -} - // Change struct size and check if it can be deserialized // from old version archive and vice versa struct old_version diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 3054ad2f6d53..ee88e6d580d8 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -7,6 +7,8 @@ #include +using namespace std::string_literals; + BOOST_FIXTURE_TEST_SUITE(streams_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(streams_vector_writer) @@ -161,46 +163,35 @@ BOOST_AUTO_TEST_CASE(bitstream_reader_writer) BOOST_AUTO_TEST_CASE(streams_serializedata_xor) { std::vector in; - std::vector expected_xor; - CDataStream ds(in, 0, 0); // Degenerate case - ds.Xor({0x00, 0x00}); - BOOST_CHECK_EQUAL( - std::string(expected_xor.begin(), expected_xor.end()), - ds.str()); + { + CDataStream ds{in, 0, 0}; + ds.Xor({0x00, 0x00}); + BOOST_CHECK_EQUAL(""s, ds.str()); + } in.push_back(std::byte{0x0f}); in.push_back(std::byte{0xf0}); - expected_xor.push_back('\xf0'); - expected_xor.push_back('\x0f'); // Single character key - - ds.clear(); - ds.insert(ds.begin(), in.begin(), in.end()); - - ds.Xor({0xff}); - BOOST_CHECK_EQUAL( - std::string(expected_xor.begin(), expected_xor.end()), - ds.str()); + { + CDataStream ds{in, 0, 0}; + ds.Xor({0xff}); + BOOST_CHECK_EQUAL("\xf0\x0f"s, ds.str()); + } // Multi character key in.clear(); - expected_xor.clear(); in.push_back(std::byte{0xf0}); in.push_back(std::byte{0x0f}); - expected_xor.push_back('\x0f'); - expected_xor.push_back('\x00'); - - ds.clear(); - ds.insert(ds.begin(), in.begin(), in.end()); - ds.Xor({0xff, 0x0f}); - BOOST_CHECK_EQUAL( - std::string(expected_xor.begin(), expected_xor.end()), - ds.str()); + { + CDataStream ds{in, 0, 0}; + ds.Xor({0xff, 0x0f}); + BOOST_CHECK_EQUAL("\x0f\x00"s, ds.str()); + } } BOOST_AUTO_TEST_CASE(streams_buffered_file) From 5fe72bbb8e96c0aeee6ddf65551528292a7f60b8 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 28 Jan 2022 15:29:44 +0100 Subject: [PATCH 10/17] merge bitcoin#24231: Fix read-past-the-end and integer overflows --- src/streams.h | 101 ++++++++++++++++++++++++-------------------------- 1 file changed, 49 insertions(+), 52 deletions(-) diff --git a/src/streams.h b/src/streams.h index 090609442363..c4b9afa103a7 100644 --- a/src/streams.h +++ b/src/streams.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -209,7 +210,7 @@ class CDataStream protected: using vector_type = SerializeData; vector_type vch; - unsigned int nReadPos{0}; + vector_type::size_type m_read_pos{0}; int nType; int nVersion; @@ -252,37 +253,37 @@ class CDataStream // // Vector subset // - const_iterator begin() const { return vch.begin() + nReadPos; } - iterator begin() { return vch.begin() + nReadPos; } + const_iterator begin() const { return vch.begin() + m_read_pos; } + iterator begin() { return vch.begin() + m_read_pos; } const_iterator end() const { return vch.end(); } iterator end() { return vch.end(); } - size_type size() const { return vch.size() - nReadPos; } - bool empty() const { return vch.size() == nReadPos; } - void resize(size_type n, value_type c = value_type{}) { vch.resize(n + nReadPos, c); } - void reserve(size_type n) { vch.reserve(n + nReadPos); } - const_reference operator[](size_type pos) const { return vch[pos + nReadPos]; } - reference operator[](size_type pos) { return vch[pos + nReadPos]; } - void clear() { vch.clear(); nReadPos = 0; } - value_type* data() { return vch.data() + nReadPos; } - const value_type* data() const { return vch.data() + nReadPos; } + size_type size() const { return vch.size() - m_read_pos; } + bool empty() const { return vch.size() == m_read_pos; } + void resize(size_type n, value_type c = value_type{}) { vch.resize(n + m_read_pos, c); } + void reserve(size_type n) { vch.reserve(n + m_read_pos); } + const_reference operator[](size_type pos) const { return vch[pos + m_read_pos]; } + reference operator[](size_type pos) { return vch[pos + m_read_pos]; } + void clear() { vch.clear(); m_read_pos = 0; } + value_type* data() { return vch.data() + m_read_pos; } + const value_type* data() const { return vch.data() + m_read_pos; } inline void Compact() { - vch.erase(vch.begin(), vch.begin() + nReadPos); - nReadPos = 0; + vch.erase(vch.begin(), vch.begin() + m_read_pos); + m_read_pos = 0; } bool Rewind(std::optional n = std::nullopt) { // Total rewind if no size is passed if (!n) { - nReadPos = 0; + m_read_pos = 0; return true; } // Rewind by n characters if the buffer hasn't been compacted yet - if (*n > nReadPos) + if (*n > m_read_pos) return false; - nReadPos -= *n; + m_read_pos -= *n; return true; } @@ -304,36 +305,32 @@ class CDataStream if (dst.size() == 0) return; // Read from the beginning of the buffer - unsigned int nReadPosNext = nReadPos + dst.size(); - if (nReadPosNext > vch.size()) { + auto next_read_pos{CheckedAdd(m_read_pos, dst.size())}; + if (!next_read_pos.has_value() || next_read_pos.value() > vch.size()) { throw std::ios_base::failure("CDataStream::read(): end of data"); } - memcpy(dst.data(), &vch[nReadPos], dst.size()); - if (nReadPosNext == vch.size()) - { - nReadPos = 0; + memcpy(dst.data(), &vch[m_read_pos], dst.size()); + if (next_read_pos.value() == vch.size()) { + m_read_pos = 0; vch.clear(); return; } - nReadPos = nReadPosNext; + m_read_pos = next_read_pos.value(); } - void ignore(int nSize) + void ignore(size_t num_ignore) { // Ignore from the beginning of the buffer - if (nSize < 0) { - throw std::ios_base::failure("CDataStream::ignore(): nSize negative"); + auto next_read_pos{CheckedAdd(m_read_pos, num_ignore)}; + if (!next_read_pos.has_value() || next_read_pos.value() > vch.size()) { + throw std::ios_base::failure("CDataStream::ignore(): end of data"); } - unsigned int nReadPosNext = nReadPos + nSize; - if (nReadPosNext >= vch.size()) - { - if (nReadPosNext > vch.size()) - throw std::ios_base::failure("CDataStream::ignore(): end of data"); - nReadPos = 0; + if (next_read_pos.value() == vch.size()) { + m_read_pos = 0; vch.clear(); return; } - nReadPos = nReadPosNext; + m_read_pos = next_read_pos.value(); } void write(Span src) @@ -617,7 +614,7 @@ class CBufferedFile FILE *src; //!< source file uint64_t nSrcPos; //!< how many bytes have been read from source - uint64_t nReadPos; //!< how many bytes have been read from this + uint64_t m_read_pos; //!< how many bytes have been read from this uint64_t nReadLimit; //!< up to which position we're allowed to read uint64_t nRewind; //!< how many bytes we guarantee to rewind std::vector vchBuf; //!< the buffer @@ -627,7 +624,7 @@ class CBufferedFile bool Fill() { unsigned int pos = nSrcPos % vchBuf.size(); unsigned int readNow = vchBuf.size() - pos; - unsigned int nAvail = vchBuf.size() - (nSrcPos - nReadPos) - nRewind; + unsigned int nAvail = vchBuf.size() - (nSrcPos - m_read_pos) - nRewind; if (nAvail < readNow) readNow = nAvail; if (readNow == 0) @@ -642,7 +639,7 @@ class CBufferedFile public: CBufferedFile(FILE* fileIn, uint64_t nBufSize, uint64_t nRewindIn, int nTypeIn, int nVersionIn) - : nType(nTypeIn), nVersion(nVersionIn), nSrcPos(0), nReadPos(0), nReadLimit(std::numeric_limits::max()), nRewind(nRewindIn), vchBuf(nBufSize, std::byte{0}) + : nType(nTypeIn), nVersion(nVersionIn), nSrcPos(0), m_read_pos(0), nReadLimit(std::numeric_limits::max()), nRewind(nRewindIn), vchBuf(nBufSize, std::byte{0}) { if (nRewindIn >= nBufSize) throw std::ios_base::failure("Rewind limit must be less than buffer size"); @@ -671,33 +668,33 @@ class CBufferedFile //! check whether we're at the end of the source file bool eof() const { - return nReadPos == nSrcPos && feof(src); + return m_read_pos == nSrcPos && feof(src); } //! read a number of bytes void read(Span dst) { - if (dst.size() + nReadPos > nReadLimit) { + if (dst.size() + m_read_pos > nReadLimit) { throw std::ios_base::failure("Read attempted past buffer limit"); } while (dst.size() > 0) { - if (nReadPos == nSrcPos) + if (m_read_pos == nSrcPos) Fill(); - unsigned int pos = nReadPos % vchBuf.size(); + unsigned int pos = m_read_pos % vchBuf.size(); size_t nNow = dst.size(); if (nNow + pos > vchBuf.size()) nNow = vchBuf.size() - pos; - if (nNow + nReadPos > nSrcPos) - nNow = nSrcPos - nReadPos; + if (nNow + m_read_pos > nSrcPos) + nNow = nSrcPos - m_read_pos; memcpy(dst.data(), &vchBuf[pos], nNow); - nReadPos += nNow; + m_read_pos += nNow; dst = dst.subspan(nNow); } } //! return the current reading position uint64_t GetPos() const { - return nReadPos; + return m_read_pos; } //! rewind to a given reading position @@ -705,22 +702,22 @@ class CBufferedFile size_t bufsize = vchBuf.size(); if (nPos + bufsize < nSrcPos) { // rewinding too far, rewind as far as possible - nReadPos = nSrcPos - bufsize; + m_read_pos = nSrcPos - bufsize; return false; } if (nPos > nSrcPos) { // can't go this far forward, go as far as possible - nReadPos = nSrcPos; + m_read_pos = nSrcPos; return false; } - nReadPos = nPos; + m_read_pos = nPos; return true; } //! prevent reading beyond a certain position //! no argument removes the limit bool SetLimit(uint64_t nPos = std::numeric_limits::max()) { - if (nPos < nReadPos) + if (nPos < m_read_pos) return false; nReadLimit = nPos; return true; @@ -737,12 +734,12 @@ class CBufferedFile void FindByte(uint8_t ch) { while (true) { - if (nReadPos == nSrcPos) + if (m_read_pos == nSrcPos) Fill(); - if (vchBuf[nReadPos % vchBuf.size()] == std::byte{ch}) { + if (vchBuf[m_read_pos % vchBuf.size()] == std::byte{ch}) { break; } - nReadPos++; + m_read_pos++; } } }; From 95b5850434cedf13eb3d4469ec99a8532f583946 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 23 Feb 2024 06:52:24 +0000 Subject: [PATCH 11/17] partial bitcoin#25001: Modernize util/strencodings and util/string: string_view and optional includes: - c1d165a8c2678c31aced5e1d46231d9996b0774a - 40062997f223d88d4f92aaae4622a31476686163 - 963bc9b576f0a62caffede2ce32830aef3473995 - d648b5120b2fefa9e599898bd26f05ecf4428fac - a4377a0843636eae0aaf698510fc6518582545db --- src/httprpc.cpp | 4 ++- src/rpc/evo.cpp | 6 +++- src/test/base32_tests.cpp | 4 ++- src/test/base64_tests.cpp | 4 ++- src/util/strencodings.cpp | 75 +++++++++++++++------------------------ src/util/strencodings.h | 17 +++++---- 6 files changed, 51 insertions(+), 59 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 9856235ad68e..499025936560 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -132,7 +132,9 @@ static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUserna if (strAuth.substr(0, 6) != "Basic ") return false; std::string strUserPass64 = TrimString(strAuth.substr(6)); - std::string strUserPass = DecodeBase64(strUserPass64); + bool invalid; + std::string strUserPass = DecodeBase64(strUserPass64, &invalid); + if (invalid) return false; if (strUserPass.find(':') != std::string::npos) strAuthUsernameOut = strUserPass.substr(0, strUserPass.find(':')); diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 99639cb1ea6f..a93daf2e0ebb 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -855,7 +855,11 @@ static UniValue protx_register_submit(const JSONRPCRequest& request, const Chain throw JSONRPCError(RPC_INVALID_PARAMETER, "payload signature not empty"); } - ptx.vchSig = DecodeBase64(request.params[1].get_str().c_str()); + bool decode_fail{false}; + ptx.vchSig = DecodeBase64(request.params[1].get_str().c_str(), &decode_fail); + if (decode_fail) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "malformed base64 encoding"); + } SetTxPayload(tx, ptx); return SignAndSendSpecialTx(request, chainman, tx); diff --git a/src/test/base32_tests.cpp b/src/test/base32_tests.cpp index 3b44564ddbc7..f9d1c83fcb7f 100644 --- a/src/test/base32_tests.cpp +++ b/src/test/base32_tests.cpp @@ -23,7 +23,9 @@ BOOST_AUTO_TEST_CASE(base32_testvectors) BOOST_CHECK_EQUAL(strEnc, vstrOut[i]); strEnc = EncodeBase32(vstrIn[i], false); BOOST_CHECK_EQUAL(strEnc, vstrOutNoPadding[i]); - std::string strDec = DecodeBase32(vstrOut[i]); + bool invalid; + std::string strDec = DecodeBase32(vstrOut[i], &invalid); + BOOST_CHECK(!invalid); BOOST_CHECK_EQUAL(strDec, vstrIn[i]); } diff --git a/src/test/base64_tests.cpp b/src/test/base64_tests.cpp index 99f3ebdfa120..4d8304bf7deb 100644 --- a/src/test/base64_tests.cpp +++ b/src/test/base64_tests.cpp @@ -20,7 +20,9 @@ BOOST_AUTO_TEST_CASE(base64_testvectors) { std::string strEnc = EncodeBase64(vstrIn[i]); BOOST_CHECK_EQUAL(strEnc, vstrOut[i]); - std::string strDec = DecodeBase64(strEnc); + bool invalid; + std::string strDec = DecodeBase64(strEnc, &invalid); + BOOST_CHECK(!invalid); BOOST_CHECK_EQUAL(strDec, vstrIn[i]); } diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index ed4624c3c595..08ea6910a51a 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -23,15 +23,15 @@ static const std::string SAFE_CHARS[] = CHARS_ALPHA_NUM + "!*'();:@&=+$,/?#[]-_.~%", // SAFE_CHARS_URI }; -std::string SanitizeString(const std::string& str, int rule) +std::string SanitizeString(std::string_view str, int rule) { - std::string strResult; - for (std::string::size_type i = 0; i < str.size(); i++) - { - if (SAFE_CHARS[rule].find(str[i]) != std::string::npos) - strResult.push_back(str[i]); + std::string result; + for (char c : str) { + if (SAFE_CHARS[rule].find(c) != std::string::npos) { + result.push_back(c); + } } - return strResult; + return result; } const signed char p_util_hexdigit[256] = @@ -57,55 +57,42 @@ signed char HexDigit(char c) return p_util_hexdigit[(unsigned char)c]; } -bool IsHex(const std::string& str) +bool IsHex(std::string_view str) { - for(std::string::const_iterator it(str.begin()); it != str.end(); ++it) - { - if (HexDigit(*it) < 0) - return false; + for (char c : str) { + if (HexDigit(c) < 0) return false; } return (str.size() > 0) && (str.size()%2 == 0); } -bool IsHexNumber(const std::string& str) +bool IsHexNumber(std::string_view str) { - size_t starting_location = 0; - if (str.size() > 2 && *str.begin() == '0' && *(str.begin()+1) == 'x') { - starting_location = 2; - } - for (const char c : str.substr(starting_location)) { + if (str.substr(0, 2) == "0x") str.remove_prefix(2); + for (char c : str) { if (HexDigit(c) < 0) return false; } // Return false for empty string or "0x". - return (str.size() > starting_location); + return str.size() > 0; } -std::vector ParseHex(const char* psz) +std::vector ParseHex(std::string_view str) { // convert hex dump to vector std::vector vch; - while (true) - { - while (IsSpace(*psz)) - psz++; - signed char c = HexDigit(*psz++); - if (c == (signed char)-1) - break; - auto n{uint8_t(c << 4)}; - c = HexDigit(*psz++); - if (c == (signed char)-1) - break; - n |= c; - vch.push_back(n); + auto it = str.begin(); + while (it != str.end() && it + 1 != str.end()) { + if (IsSpace(*it)) { + ++it; + continue; + } + auto c1 = HexDigit(*(it++)); + auto c2 = HexDigit(*(it++)); + if (c1 < 0 || c2 < 0) break; + vch.push_back(uint8_t(c1 << 4) | c2); } return vch; } -std::vector ParseHex(const std::string& str) -{ - return ParseHex(str.c_str()); -} - void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut) { size_t colon = in.find_last_of(':'); @@ -179,7 +166,7 @@ std::vector DecodeBase64(const char* p, bool* pf_invalid) ++p; } valid = valid && (p - e) % 4 == 0 && p - q < 4; - if (pf_invalid) *pf_invalid = !valid; + *pf_invalid = !valid; return ret; } @@ -187,9 +174,7 @@ std::vector DecodeBase64(const char* p, bool* pf_invalid) std::string DecodeBase64(const std::string& str, bool* pf_invalid) { if (!ValidAsCString(str)) { - if (pf_invalid) { - *pf_invalid = true; - } + *pf_invalid = true; return {}; } std::vector vchRet = DecodeBase64(str.c_str(), pf_invalid); @@ -257,7 +242,7 @@ std::vector DecodeBase32(const char* p, bool* pf_invalid) ++p; } valid = valid && (p - e) % 8 == 0 && p - q < 8; - if (pf_invalid) *pf_invalid = !valid; + *pf_invalid = !valid; return ret; } @@ -265,9 +250,7 @@ std::vector DecodeBase32(const char* p, bool* pf_invalid) std::string DecodeBase32(const std::string& str, bool* pf_invalid) { if (!ValidAsCString(str)) { - if (pf_invalid) { - *pf_invalid = true; - } + *pf_invalid = true; return {}; } std::vector vchRet = DecodeBase32(str.c_str(), pf_invalid); diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 9f26e77911bc..543cc4ef00fe 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -35,24 +35,23 @@ enum SafeChars * @param[in] rule The set of safe chars to choose (default: least restrictive) * @return A new string without unsafe chars */ -std::string SanitizeString(const std::string& str, int rule = SAFE_CHARS_DEFAULT); -std::vector ParseHex(const char* psz); -std::vector ParseHex(const std::string& str); +std::string SanitizeString(std::string_view str, int rule = SAFE_CHARS_DEFAULT); +std::vector ParseHex(std::string_view str); signed char HexDigit(char c); /* Returns true if each character in str is a hex character, and has an even * number of hex digits.*/ -bool IsHex(const std::string& str); +bool IsHex(std::string_view str); /** * Return true if the string is a hex number, optionally prefixed with "0x" */ -bool IsHexNumber(const std::string& str); -std::vector DecodeBase64(const char* p, bool* pf_invalid = nullptr); -std::string DecodeBase64(const std::string& str, bool* pf_invalid = nullptr); +bool IsHexNumber(std::string_view str); +std::vector DecodeBase64(const char* p, bool* pf_invalid); +std::string DecodeBase64(const std::string& str, bool* pf_invalid); std::string EncodeBase64(Span input); inline std::string EncodeBase64(Span input) { return EncodeBase64(MakeUCharSpan(input)); } inline std::string EncodeBase64(const std::string& str) { return EncodeBase64(MakeUCharSpan(str)); } -std::vector DecodeBase32(const char* p, bool* pf_invalid = nullptr); -std::string DecodeBase32(const std::string& str, bool* pf_invalid = nullptr); +std::vector DecodeBase32(const char* p, bool* pf_invalid); +std::string DecodeBase32(const std::string& str, bool* pf_invalid); /** * Base32 encode. From eab031a9b0cc9c341279ac2616218384c59a3b0f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 7 Jun 2022 18:14:06 +0200 Subject: [PATCH 12/17] merge bitcoin#26258: Remove unused CDataStream::rdbuf method --- src/streams.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/streams.h b/src/streams.h index c4b9afa103a7..b0937fd0a838 100644 --- a/src/streams.h +++ b/src/streams.h @@ -292,7 +292,6 @@ class CDataStream // Stream subset // bool eof() const { return size() == 0; } - CDataStream* rdbuf() { return this; } int in_avail() const { return size(); } void SetType(int n) { nType = n; } From e4091aa477295f992eb1a707608e9184572f165b Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 3 Jan 2023 12:43:48 +0100 Subject: [PATCH 13/17] partial bitcoin#25296: Add DataStream without ser-type and ser-version includes: - fa9becfe1cea5040e7cea36324d1b0789cbbd25d --- src/streams.h | 83 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 30 deletions(-) diff --git a/src/streams.h b/src/streams.h index b0937fd0a838..eda75652ba89 100644 --- a/src/streams.h +++ b/src/streams.h @@ -205,16 +205,13 @@ class SpanReader * >> and << read and write unformatted data using the above serialization templates. * Fills with data in linear time; some stringstream implementations take N^2 time. */ -class CDataStream +class DataStream { protected: using vector_type = SerializeData; vector_type vch; vector_type::size_type m_read_pos{0}; - int nType; - int nVersion; - public: typedef vector_type::allocator_type allocator_type; typedef vector_type::size_type size_type; @@ -226,23 +223,9 @@ class CDataStream typedef vector_type::const_iterator const_iterator; typedef vector_type::reverse_iterator reverse_iterator; - explicit CDataStream(int nTypeIn, int nVersionIn) - : nType{nTypeIn}, - nVersion{nVersionIn} {} - - explicit CDataStream(Span sp, int type, int version) : CDataStream{AsBytes(sp), type, version} {} - explicit CDataStream(Span sp, int nTypeIn, int nVersionIn) - : vch(sp.data(), sp.data() + sp.size()), - nType{nTypeIn}, - nVersion{nVersionIn} {} - - template - CDataStream(int nTypeIn, int nVersionIn, Args&&... args) - : nType{nTypeIn}, - nVersion{nVersionIn} - { - ::SerializeMany(*this, std::forward(args)...); - } + explicit DataStream() {} + explicit DataStream(Span sp) : DataStream{AsBytes(sp)} {} + explicit DataStream(Span sp) : vch(sp.data(), sp.data() + sp.size()) {} std::string str() const { @@ -294,11 +277,6 @@ class CDataStream bool eof() const { return size() == 0; } int in_avail() const { return size(); } - void SetType(int n) { nType = n; } - int GetType() const { return nType; } - void SetVersion(int n) { nVersion = n; } - int GetVersion() const { return nVersion; } - void read(Span dst) { if (dst.size() == 0) return; @@ -306,7 +284,7 @@ class CDataStream // Read from the beginning of the buffer auto next_read_pos{CheckedAdd(m_read_pos, dst.size())}; if (!next_read_pos.has_value() || next_read_pos.value() > vch.size()) { - throw std::ios_base::failure("CDataStream::read(): end of data"); + throw std::ios_base::failure("DataStream::read(): end of data"); } memcpy(dst.data(), &vch[m_read_pos], dst.size()); if (next_read_pos.value() == vch.size()) { @@ -322,7 +300,7 @@ class CDataStream // Ignore from the beginning of the buffer auto next_read_pos{CheckedAdd(m_read_pos, num_ignore)}; if (!next_read_pos.has_value() || next_read_pos.value() > vch.size()) { - throw std::ios_base::failure("CDataStream::ignore(): end of data"); + throw std::ios_base::failure("DataStream::ignore(): end of data"); } if (next_read_pos.value() == vch.size()) { m_read_pos = 0; @@ -347,7 +325,7 @@ class CDataStream } template - CDataStream& operator<<(const T& obj) + DataStream& operator<<(const T& obj) { // Serialize to this stream ::Serialize(*this, obj); @@ -355,7 +333,7 @@ class CDataStream } template - CDataStream& operator>>(T&& obj) + DataStream& operator>>(T&& obj) { // Unserialize from this stream ::Unserialize(*this, obj); @@ -386,6 +364,51 @@ class CDataStream } }; +class CDataStream : public DataStream +{ +private: + int nType; + int nVersion; + +public: + explicit CDataStream(int nTypeIn, int nVersionIn) + : nType{nTypeIn}, + nVersion{nVersionIn} {} + + explicit CDataStream(Span sp, int type, int version) : CDataStream{AsBytes(sp), type, version} {} + explicit CDataStream(Span sp, int nTypeIn, int nVersionIn) + : DataStream{sp}, + nType{nTypeIn}, + nVersion{nVersionIn} {} + + template + CDataStream(int nTypeIn, int nVersionIn, Args&&... args) + : nType{nTypeIn}, + nVersion{nVersionIn} + { + ::SerializeMany(*this, std::forward(args)...); + } + + void SetType(int n) { nType = n; } + int GetType() const { return nType; } + void SetVersion(int n) { nVersion = n; } + int GetVersion() const { return nVersion; } + + template + CDataStream& operator<<(const T& obj) + { + ::Serialize(*this, obj); + return *this; + } + + template + CDataStream& operator>>(T&& obj) + { + ::Unserialize(*this, obj); + return *this; + } +}; + template class BitStreamReader { From cf4522f845640f84a6ed611c344d6e0dafa5b2fe Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 23 Feb 2024 18:16:48 +0000 Subject: [PATCH 14/17] partial bitcoin#23595: Add ParseHex() helper excludes: - facd1fb911abfc595a3484ee53397eff515d4c40 --- src/test/fuzz/hex.cpp | 2 ++ src/test/util_tests.cpp | 10 +++++++++- src/util/strencodings.cpp | 10 ++++++---- src/util/strencodings.h | 4 +++- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/test/fuzz/hex.cpp b/src/test/fuzz/hex.cpp index 5917fbb96d46..99ce4da764ac 100644 --- a/src/test/fuzz/hex.cpp +++ b/src/test/fuzz/hex.cpp @@ -20,6 +20,8 @@ FUZZ_TARGET(hex) { const std::string random_hex_string(buffer.begin(), buffer.end()); const std::vector data = ParseHex(random_hex_string); + const std::vector bytes{ParseHex(random_hex_string)}; + assert(AsBytes(Span{data}) == Span{bytes}); const std::string hex_data = HexStr(data); if (IsHex(random_hex_string)) { assert(ToLower(random_hex_string) == hex_data); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index d741c1bc259d..a0f743a6e764 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -149,7 +149,7 @@ static const unsigned char ParseHex_expected[65] = { 0xde, 0x5c, 0x38, 0x4d, 0xf7, 0xba, 0x0b, 0x8d, 0x57, 0x8a, 0x4c, 0x70, 0x2b, 0x6b, 0xf1, 0x1d, 0x5f }; -BOOST_AUTO_TEST_CASE(util_ParseHex) +BOOST_AUTO_TEST_CASE(parse_hex) { std::vector result; std::vector expected(ParseHex_expected, ParseHex_expected + sizeof(ParseHex_expected)); @@ -165,6 +165,14 @@ BOOST_AUTO_TEST_CASE(util_ParseHex) result = ParseHex(" 89 34 56 78"); BOOST_CHECK(result.size() == 4 && result[0] == 0x89 && result[1] == 0x34 && result[2] == 0x56 && result[3] == 0x78); + // Embedded null is treated as end + const std::string with_embedded_null{" 11 "s + " \0 " + " 22 "s}; + BOOST_CHECK_EQUAL(with_embedded_null.size(), 11); + result = ParseHex(with_embedded_null); + BOOST_CHECK(result.size() == 1 && result[0] == 0x11); + // Stop parsing at invalid value result = ParseHex("1234 invalid 1234"); BOOST_CHECK(result.size() == 2 && result[0] == 0x12 && result[1] == 0x34); diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 08ea6910a51a..4fb69b95ea37 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -75,10 +75,10 @@ bool IsHexNumber(std::string_view str) return str.size() > 0; } -std::vector ParseHex(std::string_view str) +template +std::vector ParseHex(std::string_view str) { - // convert hex dump to vector - std::vector vch; + std::vector vch; auto it = str.begin(); while (it != str.end() && it + 1 != str.end()) { if (IsSpace(*it)) { @@ -88,10 +88,12 @@ std::vector ParseHex(std::string_view str) auto c1 = HexDigit(*(it++)); auto c2 = HexDigit(*(it++)); if (c1 < 0 || c2 < 0) break; - vch.push_back(uint8_t(c1 << 4) | c2); + vch.push_back(Byte(c1 << 4) | Byte(c2)); } return vch; } +template std::vector ParseHex(std::string_view); +template std::vector ParseHex(std::string_view); void SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut) { diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 543cc4ef00fe..9cb5b30df9b9 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -36,7 +36,9 @@ enum SafeChars * @return A new string without unsafe chars */ std::string SanitizeString(std::string_view str, int rule = SAFE_CHARS_DEFAULT); -std::vector ParseHex(std::string_view str); +/** Parse the hex string into bytes (uint8_t or std::byte). Ignores whitespace. */ +template +std::vector ParseHex(std::string_view str); signed char HexDigit(char c); /* Returns true if each character in str is a hex character, and has an even * number of hex digits.*/ From cb2fa8360a519081b05d6c26deecb79c5b7a1039 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 23 Feb 2024 18:14:50 +0000 Subject: [PATCH 15/17] test: place the std::ostream operator<< definition in namespace std this is required to prevent the tests introduced in bitcoin#27927 from failing to compile because boost::has_left_shift::value returns false for the std::byte specialization, resulting in a static assertion failure in Boost.Test this change was introduced in f7086fd in bitcoin#23497 --- src/test/util/setup_common.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index d96a4458fe33..c0a7c7fc824d 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -25,11 +25,14 @@ extern const std::function G_TEST_LOG_FUN; // Enable BOOST_CHECK_EQUAL for enum class types +namespace std { template std::ostream& operator<<(typename std::enable_if::value, std::ostream>::type& stream, const T& e) { return stream << static_cast::type>(e); } +} // namespace std + /** * This global and the helpers that use it are not thread-safe. * From 4eeafa267c23bebe63889a8798d060459a06af0d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 21 Jun 2023 10:13:08 +0200 Subject: [PATCH 16/17] partial bitcoin#27927: Allow std::byte and char Span serialization includes: - fa257bc8312b91c2d281f48ca2500d9cba353cc5 --- src/serialize.h | 7 ++++--- src/test/serialize_tests.cpp | 9 +++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/serialize.h b/src/serialize.h index 3be28c7e56eb..c07cc81073e2 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -192,6 +192,7 @@ template const X& ReadWriteAsHelper(const X& x) { return x; } } \ FORMATTER_METHODS(cls, obj) +// clang-format off #ifndef CHAR_EQUALS_INT8 template void Serialize(Stream&, char) = delete; // char serialization forbidden. Use uint8_t or int8_t #endif @@ -205,8 +206,7 @@ template inline void Serialize(Stream& s, int64_t a ) { ser_wri template inline void Serialize(Stream& s, uint64_t a) { ser_writedata64(s, a); } template inline void Serialize(Stream& s, const char (&a)[N]) { s.write(MakeByteSpan(a)); } template inline void Serialize(Stream& s, const unsigned char (&a)[N]) { s.write(MakeByteSpan(a)); } -template inline void Serialize(Stream& s, const Span& span) { s.write(AsBytes(span)); } -template inline void Serialize(Stream& s, const Span& span) { s.write(AsBytes(span)); } +template void Serialize(Stream& s, Span span) { (void)/* force byte-type */UCharCast(span.data()); s.write(AsBytes(span)); } #ifndef CHAR_EQUALS_INT8 template void Unserialize(Stream&, char) = delete; // char serialization forbidden. Use uint8_t or int8_t @@ -221,10 +221,11 @@ template inline void Unserialize(Stream& s, int64_t& a ) { a = template inline void Unserialize(Stream& s, uint64_t& a) { a = ser_readdata64(s); } template inline void Unserialize(Stream& s, char (&a)[N]) { s.read(MakeWritableByteSpan(a)); } template inline void Unserialize(Stream& s, unsigned char (&a)[N]) { s.read(MakeWritableByteSpan(a)); } -template inline void Unserialize(Stream& s, Span& span) { s.read(AsWritableBytes(span)); } +template void Unserialize(Stream& s, Span span) { (void)/* force byte-type */UCharCast(span.data()); s.read(AsWritableBytes(span)); } template inline void Serialize(Stream& s, bool a) { uint8_t f = a; ser_writedata8(s, f); } template inline void Unserialize(Stream& s, bool& a) { uint8_t f = ser_readdata8(s); a = f; } +// clang-format on /** diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 92212e8d8e92..0dcf9f4ec463 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -292,6 +292,15 @@ BOOST_AUTO_TEST_CASE(class_methods) CDataStream ss2(SER_DISK, PROTOCOL_VERSION, intval, boolval, stringval, charstrval, txval); ss2 >> methodtest3; BOOST_CHECK(methodtest3 == methodtest4); + { + DataStream ds; + const std::string in{"ab"}; + ds << Span{in}; + std::array out; + ds >> Span{out}; + BOOST_CHECK_EQUAL(out.at(0), std::byte{'a'}); + BOOST_CHECK_EQUAL(out.at(1), std::byte{'b'}); + } } BOOST_AUTO_TEST_SUITE_END() From 2b26a87874264498810848a79188e58a6e40a6ef Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 30 Jun 2023 11:30:59 +0200 Subject: [PATCH 17/17] merge bitcoin#28012: Allow FastRandomContext::randbytes for std::byte, Allow std::byte serialization --- src/random.cpp | 9 ++++++--- src/random.h | 3 ++- src/serialize.h | 2 ++ src/test/serialize_tests.cpp | 6 ++++-- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/random.cpp b/src/random.cpp index da564309209a..8b6ea488bfc5 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -630,15 +630,18 @@ uint256 FastRandomContext::rand256() noexcept return ret; } -std::vector FastRandomContext::randbytes(size_t len) +template +std::vector FastRandomContext::randbytes(size_t len) { if (requires_seed) RandomSeed(); - std::vector ret(len); + std::vector ret(len); if (len > 0) { - rng.Keystream(ret.data(), len); + rng.Keystream(UCharCast(ret.data()), len); } return ret; } +template std::vector FastRandomContext::randbytes(size_t); +template std::vector FastRandomContext::randbytes(size_t); void FastRandomContext::fillrand(Span output) { diff --git a/src/random.h b/src/random.h index f1b31aae660f..6b096b477664 100644 --- a/src/random.h +++ b/src/random.h @@ -200,7 +200,8 @@ class FastRandomContext } /** Generate random bytes. */ - std::vector randbytes(size_t len); + template + std::vector randbytes(size_t len); /** Fill a byte Span with random bytes. */ void fillrand(Span output); diff --git a/src/serialize.h b/src/serialize.h index c07cc81073e2..ef3053b717ff 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -196,6 +196,7 @@ template const X& ReadWriteAsHelper(const X& x) { return x; } #ifndef CHAR_EQUALS_INT8 template void Serialize(Stream&, char) = delete; // char serialization forbidden. Use uint8_t or int8_t #endif +template void Serialize(Stream& s, std::byte a) { ser_writedata8(s, uint8_t(a)); } template inline void Serialize(Stream& s, int8_t a ) { ser_writedata8(s, a); } template inline void Serialize(Stream& s, uint8_t a ) { ser_writedata8(s, a); } template inline void Serialize(Stream& s, int16_t a ) { ser_writedata16(s, a); } @@ -211,6 +212,7 @@ template void Serialize(Stream& s, Span span) { #ifndef CHAR_EQUALS_INT8 template void Unserialize(Stream&, char) = delete; // char serialization forbidden. Use uint8_t or int8_t #endif +template void Unserialize(Stream& s, std::byte& a) { a = std::byte{ser_readdata8(s)}; } template inline void Unserialize(Stream& s, int8_t& a ) { a = ser_readdata8(s); } template inline void Unserialize(Stream& s, uint8_t& a ) { a = ser_readdata8(s); } template inline void Unserialize(Stream& s, int16_t& a ) { a = ser_readdata16(s); } diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 0dcf9f4ec463..a91fa9297e25 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -295,11 +295,13 @@ BOOST_AUTO_TEST_CASE(class_methods) { DataStream ds; const std::string in{"ab"}; - ds << Span{in}; + ds << Span{in} << std::byte{'c'}; std::array out; - ds >> Span{out}; + std::byte out_3; + ds >> Span{out} >> out_3; BOOST_CHECK_EQUAL(out.at(0), std::byte{'a'}); BOOST_CHECK_EQUAL(out.at(1), std::byte{'b'}); + BOOST_CHECK_EQUAL(out_3, std::byte{'c'}); } }