💬 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.

✅ vasild closed a pull request: "i2p: make a time gap between creating transient sessions and using them"
(https://github.com/bitcoin/bitcoin/pull/32065)

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2005472525)
A new comment has been posted to that bug report.

💬 0xB10C commented on issue "ci: failure in interface_usdt_utxocache.py":
(https://github.com/bitcoin/bitcoin/issues/31951#issuecomment-2740221745)
> To change only this test is relatviely simple: https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:usdt-order-independant but it will be more work to apply to all the tests...

This was the first, and for now, the only time we saw this test fail due to ordering problems. I think we can't completely rule out ordering problems, but IMO it would be good to see this happening in some other test(s) before we start changing them.

💬 ismaelsadeeq commented on pull request "mining: drop unused -nFees and sigops from CBlockTemplate":
(https://github.com/bitcoin/bitcoin/pull/31897#issuecomment-2740221944)
re-ACK 226d81f8b708ea1c3a39ec58446e291fe7440fdd

Latest change added functional test coverage for the `sigops` and `fee` field of txs returned from `getblocktemplate` RPC

💬 vasild commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#issuecomment-2740226777)
`a22a3b3c4c...48a636cdde`: rebase due to conflicts and reduce the pre-created sessions to 1, given the above. Some logs from running this:

```
974be1f338 startup +129s: made new session
974be1f338 startup +290s, 161s after creation: connect failed
974be1f338 startup +372s, 243s after creation: connect failed
974be1f338 startup +678s, 549s after creation: connect ok, connection duration: 1305s, destroyed after that

55723ba1fc startup +1385s: made new session
55723ba1fc startup +1441s,
...