💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741737264)
I think the convention is:
```suggestion
// Copyright (c) 2024-present The Bitcoin Core developers
```

💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741725078)
I know this line didn't change here, I'm looking for answers:
I find it quite confusing that most of the side-effects of this method are actually done inside if conditions - I wanted to understand whether the name `fDoFullFlush` is accurate, but had a hard time understanding which methods are just checking state (e.g. `CheckDiskSpace`, I assume) and which ones are changing state (e.g. `empty_cache ? !CoinsTip().Flush() : !CoinsTip().Sync()`).
So my question is whether it's fair to call the con
...

💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741741467)
we do more than `add randomness to periodic write interval` in this commit now, could you please split it up a bit more?

💬 ismaelsadeeq commented on pull request "Fee Estimation via Fee rate Forecasters":
(https://github.com/bitcoin/bitcoin/pull/30157#issuecomment-2326075219)
Rebased and added newly introduced files to cmakelist file instead makefile

📝 l0rinc converted_to_draft a pull request: "refactor: Extend CScript with `operator<<` for `std::array`"
(https://github.com/bitcoin/bitcoin/pull/30765)
Split out of https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1722326803.

Extracted the existing `CScript` vector serialization into two private methods (for size and data handling), reducing duplication between `vector` and `array` methods (relying only on their size, begin, and end methods).

This change enables the use of `CScript() << "..."_hex_u8`, skipping the `vector` conversion before serializing to the prevector in `CScript`.

Note that after this change, it would techni
...

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-2326119633)
Rebased and added newly introduced files to cmakelist file instead of makefile

💬 hebasto commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#issuecomment-2326144002)
> > Sets the "Debug" build configuration when depends are built with DEBUG=1, ensuring that linux_debug_CPPFLAGS from depends are passed to the main build system.

> Is this not a bug?

No, it is not. Rather it is a design feature.

> If depends is built (in any configuration/with any options/flags) then the expectation would be that everything is passed to CMake (regardless of the CMake build type).

While both depends and CMake have configuration-specific flags, it seems unreasonable t
...

📝 0xB10C converted_to_draft a pull request: "tracing: Only prepare tracepoint arguments when actually tracing"
(https://github.com/bitcoin/bitcoin/pull/26593)
Currently, if the tracepoints are compiled (e.g. in depends and release builds), we always prepare the tracepoint arguments regardless of the tracepoints being used or not. We made sure that the argument preparation is as cheap as possible, but we can almost completely eliminate any overhead for users not interested in the tracepoints (the vast majority), by gating the tracepoint argument preparation with an `if(something is attached to this tracepoint)`. To achieve this, we use the optional sem
...

💬 0xB10C commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2326162347)
Rebased this on autotools removal, but the new way of detecting if the systemtap headers are present doesn't seem to work (I didn't do `cmake -B build -DWITH_USDT=ON` locally). Marking this as draft until I figure out how it works and rebase this on https://github.com/bitcoin/bitcoin/pull/30741

💬 fanquake commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#issuecomment-2326165639)
> it seems unreasonable to expect from CMake configured for the "Release" build type pull in *_debug_* flags from depends.

CMake shouldn't have a concept of "depends debug" flags (it shouldn't really even know about depends). It should just get flags/options passed to it in a toolchain file (which is the interface between the two), and it should use them. Having any special coupling/behaviour based on if things are a "depends" build or not, removes the ability for anyone else to bring a CMak
...

💬 vasild commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#issuecomment-2326170413)
`68dfd4270c...f85814052d`: rebase due to conflicts and to pick CMake

💬 vasild commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#issuecomment-2326175963)
`5956668ecd...f93fab4c58`: rebase to pick CMake (even though this PR only contains changes to `.py` files)

💬 l0rinc commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#issuecomment-2326176023)
> 2\. but `BOOST_CHECK` is deprecated

I don't see the deprecation https://live.boost.org/doc/libs/1_86_0/libs/test/doc/html/boost_test/utf_reference/testing_tool_ref/assertion_boost_level.html, I think it's still useful for truthy checks (e.g. empty optionals).

> one can compare optional against a value without .value() (C++17 addition).

Yes, you're right here, I wrongly assumed some BOOST magic was done in this case instead.

💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2326186779)
`3769f89a78...e59097a0a5`: rebase to pick CMake

💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2326190100)
@TheCharlatan oops, I meant #30750.

💬 fanquake commented on pull request "depends: build libevent with `-D_GNU_SOURCE`":
(https://github.com/bitcoin/bitcoin/pull/30743#issuecomment-2326193470)
Guix Build (aarch64):
```bash
27bc371752bd40a5f634cc8769c860dae2faec5f71976dba4fef0b0b22c69dc4 guix-build-556775408797/output/aarch64-linux-gnu/SHA256SUMS.part
a0e74a766c104f2e64196c6711e9eb895786847ff08a92a2c3b253f8b2611725 guix-build-556775408797/output/aarch64-linux-gnu/bitcoin-556775408797-aarch64-linux-gnu-debug.tar.gz
0c2099f20a86aff4766e6762b3a69780d72af0f5b26a8579e614085035461a90 guix-build-556775408797/output/aarch64-linux-gnu/bitcoin-556775408797-aarch64-linux-gnu.tar.gz
17c466
...

👍 fanquake approved a pull request: "build: Fix linking for `fuzz` target when building with MSan"
(https://github.com/bitcoin/bitcoin/pull/30778#pullrequestreview-2277100490)
ACK 787dfaf084a3952319778da9cbcda9d7d619e4ee - as a follow up it would be good to:
* Actually figure out why this fix works, and why the other msan build wasn't broken: https://github.com/bitcoin/bitcoin/pull/30778#discussion_r1741597310.
* Update oss-fuzz to match what is being done here? Now that they've diverged.
* Open an issue to figure out expectations around depends-with-cmake behaviour.

💬 maflcko commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#issuecomment-2326233559)
For reference, before (https://cirrus-ci.com/task/6668539271053312?logs=ci#L1064):

```
Cross compiling ....................... FALSE
C++ compiler .......................... Clang 18.1.3, /usr/bin/clang++
CMAKE_BUILD_TYPE ...................... RelWithDebInfo
Preprocessor defined macros ...........
C++ compiler flags .................... -fsanitize=memory -fsanitize-memory-track-origins=2 -fno-omit-frame-pointer -g -O1 -fno-optimize-sibling-calls -nostdinc++ -nostdlib++ -isystem /msan/cx
...

✅ fanquake closed an issue: "ci: fuzz_msan failed with ==4201==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x55f0c9bdeffb in SetArgs"
(https://github.com/bitcoin/bitcoin/issues/30760)

🚀 fanquake merged a pull request: "build: Fix linking for `fuzz` target when building with MSan"
(https://github.com/bitcoin/bitcoin/pull/30778)