💬 musaHaruna commented on pull request "test: Add test coverage for rpcwhitelistdefault when unset":
(https://github.com/bitcoin/bitcoin/pull/32079#discussion_r2005097804)
Thanks for the clarification! Yeah the previous is making it redundant. Hence please @naiyoma ignore my nit

🤔 ajtowns reviewed a pull request: "cluster mempool: introduce TxGraph"
(https://github.com/bitcoin/bitcoin/pull/31363#pullrequestreview-2701725334)
ReACK 63b8f96e67f9ad649070a231532d48fb6c3573e4 . Needs a rebase anyway though.

The main/staging changes read better to me fwiw. The level stuff in the locators could be ungeneralised as well I suppose, but that doesn't seem likely to be much of an improvement.

💬 ajtowns commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2005075531)
Could tidy this up:

```c++
if (!m_main_clusterset.empty()) return;
if (m_staging_clusterset.has_value() && !m_staging_clusterset->empty()) return;

bool ClusterSet::empty() {
return m_deps_to_add.empty() && m_to_remove.empty() && m_removed.empty();
}
```

💬 l0rinc commented on pull request "leveldb: pull upstream C++23 changes":
(https://github.com/bitcoin/bitcoin/pull/31766#issuecomment-2739679633)
`reindex-chainstate` finished successfully

<details>
<summary>Details</summary>

```bash
COMMITS="5e934ebc82ca239cd98eed1689f7cdc2c672a9ae"; \
STOP_HEIGHT=888000; DBCACHE=5000; \
C_COMPILER=gcc; CXX_COMPILER=g++; \
git fetch --all -q && (for c in $COMMITS; do git log -1 --oneline $c || exit 1; done) && \
hyperfine \
--export-json "/mnt/my_storage/rdx-${COMMITS// /-}-${STOP_HEIGHT}-${DBCACHE}-${C_COMPILER}.json" \
--runs 1 \
--parameter-list COMMIT ${COMMITS// /,} \
--prepa
...

💬 l0rinc commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r2005143296)
Related to https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1926634436 - it's more uniform now

💬 l0rinc commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#issuecomment-2739703340)
ACK c6eca6f3961d780d6a395c3346e44be1a0072f08

💬 Sjors commented on issue "Linux download needs installation instructions":
(https://github.com/bitcoin/bitcoin/issues/32097#issuecomment-2739711382)
That's a useful reference. But the concern there is about security, depending on operating system libraries. Here I'm just worried about the practically of installing. As linux desktop gets more user friendly, I expect fewer people to know how to figure out which libraries they're missing and how to install them.

💬 TheCharlatan commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2005190423)
I was thinking that this might be interesting to add to the getchainstates rpc call.

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2005207380)
"acceptable quality" is more than just a topologically valid order.
It refers to a linearization that may not be optimal but is the best we can achieve for that cluster within resource limitations.

💬 Sjors commented on pull request "cmake: Add `NO_CACHE_IF_FAILED` option for checking linker flags":
(https://github.com/bitcoin/bitcoin/pull/32027#issuecomment-2739809540)
This seems useful, but I'm having a hard time reproducing the issue, or something similar, on macOS, because XCode 15.2 ships with most of the sanitizers.

I can hit an earlier error:

```
cmake -B build -DSANITIZERS=memory
...
-- Performing Test CXX_SUPPORTS__FSANITIZE_MEMORY - Failed
CMake Error at CMakeLists.txt:362 (message):
Compiler did not accept requested flags.
```

But then I'd probably have to recompile LLVM.

Might be easier to reproduce on a linux distro, like the or
...

💬 Sjors commented on pull request "test, refactor: Add TestNode.binaries to hold binary paths":
(https://github.com/bitcoin/bitcoin/pull/31866#issuecomment-2739873757)
ACK d190f0facc8379da7610d7161e532d57c6a7eb96

I verified that the commits here match 095801b8999851b10e43567389cd293fd7957497 and 45d439dab13153a3b570957a9eab63e3e6274611 in #31375 where I reviewed them.

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2005273964)
In 101a8ee3280e50c3272a80939b46a67faca838e4 "txgraph: (optimization) avoid per-group vectors for clusters & dependencies"
```suggestion
/** Where the dependencies for this cluster group in ClusterSet::m_deps_to_add start. */

```

👍 pablomartin4btc approved a pull request: "refactor: Remove redundant and confusing calls to IsArgSet"
(https://github.com/bitcoin/bitcoin/pull/31896#pullrequestreview-2702084258)
re-ACK fa0cf040baa74b161b4b00ddea95bfb5aac0bb53

Since my last review: outdated TODO [comment](https://github.com/bitcoin/bitcoin/pull/31896#discussion_r1997633914) was removed (on new commit) - no impact.

💬 Sjors commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2739919199)
re-ACK a9935d4dae8847d0549ec51ddc4349c7e8c61763

Briefly tested another guix build on Windows.

📝 ismaelsadeeq opened a pull request: "doc: clarify the documentation of `Assume` assertion"
(https://github.com/bitcoin/bitcoin/pull/32100)
Statements inside `Assume` may be optimized away in production builds when the compiler proves they are side-effect-free.
This makes `Assume` useful for testing invariants that are always evaluated in debug builds, allowing violations to be caught during fuzzing with the hope that it wont incur runtime cost in release builds.
This use case is demonstrated in #31363 and is suggested to be documented in https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2736410023.

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2740049673)
> That makes sense. Might be a good idea to update doc/developer-notes.md pointing out that use case? It currently says:

Done in #32100

💬 l0rinc commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#discussion_r2005441297)
Now that #31519 was merged, I've rebased an moved it out of draft.

👋 l0rinc's pull request is ready for review: "[IBD] specialize block serialization"
(https://github.com/bitcoin/bitcoin/pull/31868)

💬 willcl-ark commented on issue "ci: failure in interface_usdt_utxocache.py":
(https://github.com/bitcoin/bitcoin/issues/31951#issuecomment-2740183900)
From reading up on this a little it seems to be possible that if the tracepoint:

- includes runtime [lookups](https://github.com/bitcoin/bitcoin/blob/aa87e0b44600a32b32a4b123d4f90d097f1f106f/src/txmempool.cpp#L513-L518)
- is executed in a multi-threaded (or multi-CPU) environment
- (possibly other factors)

...then it may not be possible to totally guarantee ordering at userspace collection points. One can use BPF [queues](https://docs.kernel.org/bpf/map_queue_stack.html), but I suspect this wo
...

🤔 ismaelsadeeq reviewed a pull request: "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods"
(https://github.com/bitcoin/bitcoin/pull/31785#pullrequestreview-2702397335)
Concept ACK

In 64a2795fd4fe223a55564c31e9fa36264e79ac22 "rpc: handle shutdown during long poll and wait methods"

is a use for returning the last known tip?

If not, should we simplify the behavior by always returning nullopt when the tip has not changed or no better block is available?

Or It will be more consistent to to return the last known tip if it is set, when this edge case happen handle it by returning nullopt.