Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/init/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ void AddLoggingArgs(ArgsManager& argsman)
ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-debugexclude=<category>", "Exclude debug and trace logging for a category. Can be used in conjunction with -debug=1 to output debug and trace logging for all categories except the specified category. This option can be specified multiple times to exclude multiple categories.", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-logips", strprintf("Include IP addresses in debug output (default: %u)", DEFAULT_LOGIPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-loglevel=<level>|<category>:<level>", strprintf("Set the global or per-category severity level for logging categories enabled with the -debug configuration option or the logging RPC: %s (default=%s); warning and error levels are always logged. If <category>:<level> is supplied, the setting will override the global one and may be specified multiple times to set multiple category-specific levels. <category> can be: %s.", LogInstance().LogLevelsString(), LogInstance().LogLevelToStr(BCLog::DEFAULT_LOG_LEVEL), LogInstance().LogCategoriesString()), ArgsManager::DISALLOW_NEGATION | ArgsManager::DISALLOW_ELISION | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-loglevel=<level>|<category>:<level>", strprintf("Set the global or per-category severity level for logging categories enabled with the -debug configuration option or the logging RPC. Possible values are %s (default=%s). The following levels are always logged: error, warning, info. If <category>:<level> is supplied, the setting will override the global one and may be specified multiple times to set multiple category-specific levels. <category> can be: %s.", LogInstance().LogLevelsString(), LogInstance().LogLevelToStr(BCLog::DEFAULT_LOG_LEVEL), LogInstance().LogCategoriesString()), ArgsManager::DISALLOW_NEGATION | ArgsManager::DISALLOW_ELISION | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-logtimestamps", strprintf("Prepend debug output with timestamp (default: %u)", DEFAULT_LOGTIMESTAMPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-logthreadnames", strprintf("Prepend debug output with name of the originating thread (only available on platforms supporting thread_local) (default: %u)", DEFAULT_LOGTHREADNAMES), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-logsourcelocations", strprintf("Prepend debug output with name of the originating source location (source file, line number and function name) (default: %u)", DEFAULT_LOGSOURCELOCATIONS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
Expand Down
26 changes: 21 additions & 5 deletions src/test/fuzz/minisketch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,27 @@
#include <map>
#include <numeric>

using node::MakeMinisketch32;
namespace {

Minisketch MakeFuzzMinisketch32(size_t capacity, uint32_t impl)
{
return Assert(Minisketch(32, impl, capacity));
}

} // namespace

FUZZ_TARGET(minisketch)
{
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};

const auto capacity{fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 200)};
Minisketch sketch_a{Assert(MakeMinisketch32(capacity))};
Minisketch sketch_b{Assert(MakeMinisketch32(capacity))};
const uint32_t impl{fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(0, Minisketch::MaxImplementation())};
if (!Minisketch::ImplementationSupported(32, impl)) return;

Minisketch sketch_a{MakeFuzzMinisketch32(capacity, impl)};
Minisketch sketch_b{MakeFuzzMinisketch32(capacity, impl)};
sketch_a.SetSeed(fuzzed_data_provider.ConsumeIntegral<uint64_t>());
sketch_b.SetSeed(fuzzed_data_provider.ConsumeIntegral<uint64_t>());

// Fill two sets and keep the difference in a map
std::map<uint32_t, bool> diff;
Expand Down Expand Up @@ -47,8 +60,11 @@ FUZZ_TARGET(minisketch)
}
const auto num_diff{std::accumulate(diff.begin(), diff.end(), size_t{0}, [](auto n, const auto& e) { return n + e.second; })};

Minisketch sketch_ar{MakeMinisketch32(capacity)};
Minisketch sketch_br{MakeMinisketch32(capacity)};
Minisketch sketch_ar{MakeFuzzMinisketch32(capacity, impl)};
Minisketch sketch_br{MakeFuzzMinisketch32(capacity, impl)};
sketch_ar.SetSeed(fuzzed_data_provider.ConsumeIntegral<uint64_t>());
sketch_br.SetSeed(fuzzed_data_provider.ConsumeIntegral<uint64_t>());

sketch_ar.Deserialize(sketch_a.Serialize());
sketch_br.Deserialize(sketch_b.Serialize());

Expand Down
37 changes: 37 additions & 0 deletions src/test/fuzz/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,40 @@ FUZZ_TARGET(net, .init = initialize_net)
(void)node.HasPermission(net_permission_flags);
(void)node.ConnectedThroughNetwork();
}

FUZZ_TARGET(local_address, .init = initialize_net)
{
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
CService service{ConsumeService(fuzzed_data_provider)};
CNode node{ConsumeNode(fuzzed_data_provider)};
{
LOCK(g_maplocalhost_mutex);
mapLocalHost.clear();
}
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
CallOneOf(
fuzzed_data_provider,
[&] {
service = ConsumeService(fuzzed_data_provider);
},
[&] {
const bool added{AddLocal(service, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, LOCAL_MAX - 1))};
if (!added) return;
assert(service.IsRoutable());
assert(IsLocal(service));
assert(SeenLocal(service));
},
[&] {
(void)RemoveLocal(service);
},
[&] {
(void)SeenLocal(service);
},
[&] {
(void)IsLocal(service);
},
[&] {
(void)GetLocalAddress(node);
});
}
}
3 changes: 3 additions & 0 deletions src/txdb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, boo
// We may be in the middle of replaying.
std::vector<uint256> old_heads = GetHeadBlocks();
if (old_heads.size() == 2) {
if (old_heads[0] != hashBlock) {
LogPrintLevel(BCLog::COINDB, BCLog::Level::Error, "The coins database detected an inconsistent state, likely due to a previous crash or shutdown. You will need to restart bitcoind with the -reindex-chainstate or -reindex configuration option.\n");
}
assert(old_heads[0] == hashBlock);
old_tip = old_heads[1];
}
Expand Down
11 changes: 11 additions & 0 deletions src/wallet/transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ struct TxStateConfirmed {
int position_in_block;

explicit TxStateConfirmed(const uint256& block_hash, int height, int index) : confirmed_block_hash(block_hash), confirmed_block_height(height), position_in_block(index) {}
std::string toString() const { return strprintf("Confirmed (block=%s, height=%i, index=%i)", confirmed_block_hash.ToString(), confirmed_block_height, position_in_block); }
};

//! State of transaction added to mempool.
struct TxStateInMempool {
std::string toString() const { return strprintf("InMempool"); }
};

//! State of rejected transaction that conflicts with a confirmed block.
Expand All @@ -40,6 +42,7 @@ struct TxStateConflicted {
int conflicting_block_height;

explicit TxStateConflicted(const uint256& block_hash, int height) : conflicting_block_hash(block_hash), conflicting_block_height(height) {}
std::string toString() const { return strprintf("Conflicted (block=%s, height=%i)", conflicting_block_hash.ToString(), conflicting_block_height); }
};

//! State of transaction not confirmed or conflicting with a known block and
Expand All @@ -50,6 +53,7 @@ struct TxStateInactive {
bool abandoned;

explicit TxStateInactive(bool abandoned = false) : abandoned(abandoned) {}
std::string toString() const { return strprintf("Inactive (abandoned=%i)", abandoned); }
};

//! State of transaction loaded in an unrecognized state with unexpected hash or
Expand All @@ -61,6 +65,7 @@ struct TxStateUnrecognized {
int index;

TxStateUnrecognized(const uint256& block_hash, int index) : block_hash(block_hash), index(index) {}
std::string toString() const { return strprintf("Unrecognized (block=%s, index=%i)", block_hash.ToString(), index); }
};

//! All possible CWalletTx states
Expand Down Expand Up @@ -108,6 +113,12 @@ static inline int TxStateSerializedIndex(const TxState& state)
}, state);
}

//! Return TxState or SyncTxState as a string for logging or debugging.
template<typename T>
std::string TxStateString(const T& state)
{
return std::visit([](const auto& s) { return s.toString(); }, state);
}

typedef std::map<std::string, std::string> mapValue_t;

Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const
LockProTxCoins(candidates, &batch);

//// debug print
WalletLogPrintf("AddToWallet %s %s%s\n", hash.ToString(), (fInsertedNew ? "new" : ""), (fUpdated ? "update" : ""));
WalletLogPrintf("AddToWallet %s %s%s %s\n", hash.ToString(), (fInsertedNew ? "new" : ""), (fUpdated ? "update" : ""), TxStateString(state));

// Write to disk
if (fInsertedNew || fUpdated)
Expand Down
3 changes: 3 additions & 0 deletions test/functional/interface_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,9 @@ def run_test(self):
assert_greater_than(json_obj['bytes'], 240)

mempool_info = self.nodes[0].getmempoolinfo()
# pop unstable unbroadcastcount before check
for obj in [json_obj, mempool_info]:
obj.pop("unbroadcastcount")
Comment on lines +339 to +341
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid KeyError when field is absent

json_obj or mempool_info may lack "unbroadcastcount". Use a default.

-        for obj in [json_obj, mempool_info]:
-            obj.pop("unbroadcastcount")
+        for obj in [json_obj, mempool_info]:
+            obj.pop("unbroadcastcount", None)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# pop unstable unbroadcastcount before check
for obj in [json_obj, mempool_info]:
obj.pop("unbroadcastcount")
# pop unstable unbroadcastcount before check
for obj in [json_obj, mempool_info]:
obj.pop("unbroadcastcount", None)
🤖 Prompt for AI Agents
In test/functional/interface_rest.py around lines 339 to 341, popping
"unbroadcastcount" directly can raise KeyError when json_obj or mempool_info
doesn't contain that key; change the pop calls to use a default value (e.g.,
obj.pop("unbroadcastcount", None)) or guard with an if "unbroadcastcount" in obj
to safely remove the key only when present.

assert_equal(json_obj, mempool_info)

# Check that there are our submitted transactions in the TX memory pool
Expand Down
9 changes: 9 additions & 0 deletions test/functional/p2p_addr_relay.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,15 @@ def getaddr_tests(self):
assert_equal(block_relay_peer.num_ipv4_received, 0)
assert inbound_peer.num_ipv4_received > 100

self.log.info('Check that we answer getaddr messages only once per connection')
received_addrs_before = inbound_peer.num_ipv4_received
with self.nodes[0].assert_debug_log(['Ignoring repeated "getaddr".']):
inbound_peer.send_and_ping(msg_getaddr())
self.bump_mocktime(10 * 60)
inbound_peer.sync_with_ping()
received_addrs_after = inbound_peer.num_ipv4_received
assert_equal(received_addrs_before, received_addrs_after)

self.nodes[0].disconnect_p2ps()

def blocksonly_mode_tests(self):
Expand Down
10 changes: 10 additions & 0 deletions test/functional/rpc_blockchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
TIME_RANGE_MTP = TIME_GENESIS_BLOCK + (HEIGHT - 6) * TIME_RANGE_STEP
TIME_RANGE_TIP = TIME_GENESIS_BLOCK + (HEIGHT - 1) * TIME_RANGE_STEP
TIME_RANGE_END = TIME_GENESIS_BLOCK + HEIGHT * TIME_RANGE_STEP
DIFFICULTY_ADJUSTMENT_INTERVAL = 2016


class BlockchainTest(BitcoinTestFramework):
Expand Down Expand Up @@ -431,6 +432,15 @@ def _test_getnetworkhashps(self):
# This should be 2 hashes every 2.6 minutes (156 seconds) or 1/78
assert abs(hashes_per_second * 78 - 1) < 0.0001

# Test setting the first param of getnetworkhashps to negative value returns the average network
# hashes per second from the last difficulty change.
current_block_height = self.nodes[0].getmininginfo()['blocks']
blocks_since_last_diff_change = current_block_height % DIFFICULTY_ADJUSTMENT_INTERVAL + 1
expected_hashes_per_second_since_diff_change = self.nodes[0].getnetworkhashps(blocks_since_last_diff_change)

assert_equal(self.nodes[0].getnetworkhashps(-1), expected_hashes_per_second_since_diff_change)
assert_equal(self.nodes[0].getnetworkhashps(-2), expected_hashes_per_second_since_diff_change)

def _test_stopatheight(self):
assert_equal(self.nodes[0].getblockcount(), HEIGHT)
self.generate(self.wallet, 6)
Expand Down
9 changes: 3 additions & 6 deletions test/sanitizer_suppressions/ubsan
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
# Suppressions should use `sanitize-type:ClassName::MethodName`.

# -fsanitize=undefined suppressions
# =================================
# The suppressions would be `sanitize-type:ClassName::MethodName`,
# however due to a bug in clang the symbolizer is disabled and thus no symbol
# names can be used.
# See https://github.com/google/sanitizers/issues/1364

# -fsanitize=integer suppressions
# ===============================
# Dependencies
# ------------
# Suppressions in dependencies that are developed outside this repository.
unsigned-integer-overflow:*/include/c++/
# unsigned-integer-overflow in FuzzedDataProvider's ConsumeIntegralInRange
unsigned-integer-overflow:FuzzedDataProvider.h
unsigned-integer-overflow:FuzzedDataProvider::ConsumeIntegralInRange
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Appropriate suppression for fuzzing infrastructure.

The explicit suppression for FuzzedDataProvider::ConsumeIntegralInRange is correct, as this LLVM libFuzzer function can intentionally trigger unsigned integer overflow during range calculations. This is expected behavior in fuzzing infrastructure.

Since this is a backport PR, please verify that this change matches the upstream Bitcoin Core commits referenced in the PR description.


🏁 Script executed:

#!/bin/bash
# Description: Verify the suppression entry matches upstream Bitcoin Core changes.

# Search for FuzzedDataProvider suppressions in the repository
rg -n "FuzzedDataProvider" test/sanitizer_suppressions/

Length of output: 105


The search reveals a significant finding: upstream uses unsigned-integer-overflow:FuzzedDataProvider.h (file-level suppression), while your repository uses unsigned-integer-overflow:FuzzedDataProvider::ConsumeIntegralInRange (function-level suppression) on line 12.

I found a discrepancy: upstream Bitcoin Core uses unsigned-integer-overflow:FuzzedDataProvider.h (file-level suppression), while your repository uses unsigned-integer-overflow:FuzzedDataProvider::ConsumeIntegralInRange (function-level suppression) on line 12.

Both formats are valid UBSAN suppression syntax. However, since this is a backport PR, there's a format mismatch with upstream that should be addressed for consistency.

Adjust suppression format to match upstream Bitcoin Core.

The suppression on line 12 should use the file-based format instead of the function-qualified format to maintain backport fidelity:

unsigned-integer-overflow:FuzzedDataProvider.h
🤖 Prompt for AI Agents
In test/sanitizer_suppressions/ubsan around line 12, update the UBSAN
suppression entry which currently targets the function
"unsigned-integer-overflow:FuzzedDataProvider::ConsumeIntegralInRange" to the
file-level format used upstream "unsigned-integer-overflow:FuzzedDataProvider.h"
so the backport matches upstream; replace the function-qualified suppression
with the file-based suppression on that line and ensure there are no duplicate
or conflicting entries elsewhere in the file.

unsigned-integer-overflow:leveldb/
unsigned-integer-overflow:minisketch/
unsigned-integer-overflow:secp256k1/
Expand Down
5 changes: 5 additions & 0 deletions test/util/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ def bctest(testDir, testObj, buildenv):
"""
# Get the exec names and arguments
execprog = os.path.join(buildenv["BUILDDIR"], "src", testObj["exec"] + buildenv["EXEEXT"])
if testObj["exec"] == "./dash-util":
execprog = os.getenv("DASHUTIL", default=execprog)
elif testObj["exec"] == "./dash-tx":
execprog = os.getenv("DASHTX", default=execprog)

execargs = testObj['args']
execrun = [execprog] + execargs

Expand Down
Loading