From 86d740b5eb7b365648e8ee379819e1f8337cddc2 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 8 Feb 2025 16:05:13 +0000 Subject: [PATCH 1/7] fuzz: fix crash on null pointer in witness_program target --- src/test/fuzz/witness_program.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/fuzz/witness_program.cpp b/src/test/fuzz/witness_program.cpp index 04b0c96fda8..c6516b4f2ee 100644 --- a/src/test/fuzz/witness_program.cpp +++ b/src/test/fuzz/witness_program.cpp @@ -64,7 +64,7 @@ FUZZ_TARGET_INIT(witness_program, initialize_witness_program) if (fuzz_control & 1) { unsigned char hash_program[32]; - CSHA256().Write(&program[0], program.size()).Finalize(hash_program); + CSHA256().Write(program.data(), program.size()).Finalize(hash_program); CScript scriptPubKey = CScript{} << OP_0 << std::vector(hash_program, hash_program + sizeof(hash_program)); witness.stack.push_back(program); From f5ed4e4520550d43e84aacc828fef24fbfda8c92 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 10 Feb 2025 23:17:15 +0000 Subject: [PATCH 2/7] chain: make some integer conversions explicit We have an unsigned constant which we're bit-inverting, or'ing into a signed constant, then assigning back to the signed constant. We should make it explicit wth is going on here. --- src/chain.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chain.h b/src/chain.h index 119b3683a3b..6e023158d12 100644 --- a/src/chain.h +++ b/src/chain.h @@ -469,7 +469,7 @@ class CDiskBlockIndex : public CBlockIndex bool RemoveDynaFedMaskOnSerialize(bool for_read) { if (for_read) { bool is_dyna = nVersion < 0; - nVersion = ~CBlockHeader::DYNAFED_HF_MASK & nVersion; + nVersion = (int32_t) (~CBlockHeader::DYNAFED_HF_MASK & (uint32_t)nVersion); return is_dyna; } else { return is_dynafed_block(); From bd4ae1bcc405c3496c85c3af16c81a0768f82392 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 11 Feb 2025 00:12:35 +0000 Subject: [PATCH 3/7] rpc: fix crash in getcompactsketch This originates in 8723debb3d1f281f26bdc456868f3daaf7c6aa5a which has no PR associated with it. We've really gotta stop putting thousands of unreviewed commits into this project and rebasing the history away.. --- src/rpc/mining.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 193f2f5a757..dd5ae4af44b 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1527,6 +1527,10 @@ static RPCHelpMan getcompactsketch() CDataStream ssBlock(block_bytes, SER_NETWORK, PROTOCOL_VERSION); ssBlock >> block; + if (block.vtx.empty()) { + throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Cannot obtain sketch of empty block."); + } + CBlockHeaderAndShortTxIDs cmpctblock(block, true); CDataStream ssCompactBlock(SER_NETWORK, PROTOCOL_VERSION); From 3e46754507af7acce2c1cc49fa7c2ecbf2efb496 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 21 Mar 2022 11:37:17 +0100 Subject: [PATCH 4/7] Replace struct update_fee_delta with lambda Cherry-pick of https://github.com/bitcoin/bitcoin/pull/24625 (1/2) --- src/txmempool.cpp | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index f84ffe4d232..5a737184e95 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -60,16 +60,6 @@ struct update_ancestor_state int64_t discountSize; }; -struct update_fee_delta -{ - explicit update_fee_delta(int64_t _feeDelta) : feeDelta(_feeDelta) { } - - void operator() (CTxMemPoolEntry &e) { e.UpdateFeeDelta(feeDelta); } - -private: - int64_t feeDelta; -}; - bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp) { AssertLockHeld(cs_main); @@ -520,7 +510,7 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces CAmount delta{0}; ApplyDelta(entry.GetTx().GetHash(), delta); if (delta) { - mapTx.modify(newit, update_fee_delta(delta)); + mapTx.modify(newit, [&delta](CTxMemPoolEntry& e) { e.UpdateFeeDelta(delta); }); } // Update cachedInnerUsage to include contained transaction's usage. @@ -1027,7 +1017,7 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD delta += nFeeDelta; txiter it = mapTx.find(hash); if (it != mapTx.end()) { - mapTx.modify(it, update_fee_delta(delta)); + mapTx.modify(it, [&delta](CTxMemPoolEntry& e) { e.UpdateFeeDelta(delta); }); // Now update all ancestors' modified fees with descendants setEntries setAncestors; uint64_t nNoLimit = std::numeric_limits::max(); From 371718bc99b6958c7a07954900c67c5859d98ff2 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 21 Mar 2022 11:57:21 +0100 Subject: [PATCH 5/7] Use CAmount for fee delta and modified fee Cherry-pick of fa84a49526fcf76e98b0b2f4d4b00b34a8dddf46 https://github.com/bitcoin/bitcoin/pull/24625 (2/2) --- src/node/miner.h | 2 +- src/txmempool.cpp | 2 +- src/txmempool.h | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/node/miner.h b/src/node/miner.h index b0b48e375af..4fb31a859a1 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -46,7 +46,7 @@ struct CTxMemPoolModifiedEntry { nSigOpCostWithAncestors = entry->GetSigOpCostWithAncestors(); } - int64_t GetModifiedFee() const { return iter->GetModifiedFee(); } + CAmount GetModifiedFee() const { return iter->GetModifiedFee(); } uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; } uint64_t GetDiscountSizeWithAncestors() const { return discountSizeWithAncestors; } CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 5a737184e95..14bba2e4caf 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -98,7 +98,7 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, discountSizeWithAncestors{GetDiscountTxSize()}, setPeginsSpent(_setPeginsSpent) {} -void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta) +void CTxMemPoolEntry::UpdateFeeDelta(CAmount newFeeDelta) { nModFeesWithDescendants += newFeeDelta - feeDelta; nModFeesWithAncestors += newFeeDelta - feeDelta; diff --git a/src/txmempool.h b/src/txmempool.h index edfe7be1ad2..e6e3afca5ab 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -102,7 +102,7 @@ class CTxMemPoolEntry const unsigned int entryHeight; //!< Chain height when entering the mempool const bool spendsCoinbase; //!< keep track of transactions that spend a coinbase const int64_t sigOpCost; //!< Total sigop cost - int64_t feeDelta{0}; //!< Used for determining the priority of the transaction for mining in a block + CAmount feeDelta{0}; //!< Used for determining the priority of the transaction for mining in a block LockPoints lockPoints; //!< Track the height and time at which tx was final // Information about descendants of this transaction that are in the @@ -135,7 +135,7 @@ class CTxMemPoolEntry std::chrono::seconds GetTime() const { return std::chrono::seconds{nTime}; } unsigned int GetHeight() const { return entryHeight; } int64_t GetSigOpCost() const { return sigOpCost; } - int64_t GetModifiedFee() const { return nFee + feeDelta; } + CAmount GetModifiedFee() const { return nFee + feeDelta; } size_t DynamicMemoryUsage() const { return nUsageSize; } const LockPoints& GetLockPoints() const { return lockPoints; } @@ -144,8 +144,8 @@ class CTxMemPoolEntry // Adjusts the ancestor state void UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps, int64_t discountSize); // Updates the fee delta used for mining priority score, and the - // modified fees with descendants. - void UpdateFeeDelta(int64_t feeDelta); + // modified fees with descendants/ancestors. + void UpdateFeeDelta(CAmount newFeeDelta); // Update the LockPoints after a reorg void UpdateLockPoints(const LockPoints& lp); From b90ef387277d36070337b4f1ca088179414840e7 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 11 Feb 2025 03:00:36 +0000 Subject: [PATCH 6/7] ubsan: add suppression for simplicity --- test/sanitizer_suppressions/ubsan | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index 682ec7f60bc..a4f7f9822f9 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -81,3 +81,5 @@ implicit-integer-sign-change:primitives/confidential.cpp implicit-integer-sign-change:primitives/confidential.h shift-base:simplicity/sha256.c unsigned-integer-overflow:simplicity/sha256.c +# See comment in simplicity/primitive/elements/env.c line 303 +unsigned-integer-overflow:simplicity/primitive/elements/env.c From 363c5101c235d700a92f8963b44118b6edc813e6 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 11 Feb 2025 02:38:45 +0000 Subject: [PATCH 7/7] fuzz: change int to unsigned in witness_program Avoids a signed/unsigned integer conversion. --- src/test/fuzz/witness_program.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/fuzz/witness_program.cpp b/src/test/fuzz/witness_program.cpp index c6516b4f2ee..8cbabfd1d92 100644 --- a/src/test/fuzz/witness_program.cpp +++ b/src/test/fuzz/witness_program.cpp @@ -45,7 +45,7 @@ FUZZ_TARGET_INIT(witness_program, initialize_witness_program) CScriptWitness witness; int fuzz_control; - int flags; + unsigned flags; ds >> fuzz_control; ds >> witness.stack; ds >> flags;