👍 tdb3 approved a pull request: "test: Fix copy-paste in wallet/test/db_tests ostream operator"
(https://github.com/bitcoin/bitcoin/pull/31038#pullrequestreview-2354673777)
code review ACK f50557f5d36f568b7fe65ff5e242303b16b9b258

👍 fanquake approved a pull request: "ci: Double ctest timeout"
(https://github.com/bitcoin/bitcoin/pull/31056#pullrequestreview-2354673955)
ACK fa5ebc99207fb3dc9d052fbd489aa7abbaaf737f

🚀 fanquake merged a pull request: "ci: Double ctest timeout"
(https://github.com/bitcoin/bitcoin/pull/31056)

🚀 fanquake merged a pull request: "test: Fix copy-paste in wallet/test/db_tests ostream operator"
(https://github.com/bitcoin/bitcoin/pull/31038)

👍 instagibbs approved a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2354684021)
reACK 0b3ec8c59b2b6d598531cd4e688eb276e597c825

via `git range-diff master 2b21aebd9754c503bac12d1dbf566b3aa27451e8 0b3ec8c59b2b6d598531cd4e688eb276e597c825`

💬 fanquake commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2400014930)
MSAN failure was spurious, but not the tidy (https://cirrus-ci.com/task/4930868089716736):
```bash
[21:49:26.130] clang-tidy-18 -p=/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu -quiet -load=/tidy-build/libbitcoin-tidy.so /ci_container_base/src/validationinterface.cpp
[21:49:26.130] /ci_container_base/src/validationinterface.cpp:22:13: error: redundant 'RemovalReasonToString' declaration [readability-redundant-declaration,-warnings-as-errors]
[21:49:26.130] 22 | std::string Remov
...

🚀 fanquake merged a pull request: "test: Treat exclude list warning as failure in CI"
(https://github.com/bitcoin/bitcoin/pull/31018)

💬 fanquake commented on pull request "Add -pausebackgroundsync startup option":
(https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2400031610)
I agree with the other commentors here. I don't think we should be trying to extend/re-use a user-facing feature for development purposes, especially given the existence of more accurate/repeatable ways of achieving the same thing. Adding this option for the other uses-cases described just further murkies the function of/complicates this feature.

🚀 fanquake merged a pull request: "ci: Add missing -DWERROR=ON to test-each-commit"
(https://github.com/bitcoin/bitcoin/pull/31045)

💬 joe-bp commented on issue "Embedded ASMap data Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/28794#issuecomment-2400037617)
> > Is there no way to query AS Maps directly from the BGP tables?
>
> Who's BGP table? Can you be a bit more specific what you are thinking of in terms of architecture and trust assumptions?

Hi Fabian, I've been talking to Gary about this a bit. I submitted an issue here: https://github.com/asmap/asmap-data/issues/17

I'd like to propose a way to pull in the actual real-time BGP routing table and use that data for asmap. Multiple people/orgs can do this independently and get the data
...

✅ fanquake closed a pull request: "doc: clarify 'filename' argument in 'loadwallet' RPC"
(https://github.com/bitcoin/bitcoin/pull/31031)

💬 stickies-v commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2400044072)
Force-pushed to rebase addressing a merge conflict, preserve `tfm::format_error` throwing behaviour for `DEBUG` builds to improve visibility into programming errors before they manifest, and [improve an error string](https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2389355374).

---

Thank you for your thoughtful review @ryanofsky. tl;dr I share your concerns, but I think I have a different opinion on the trade-offs made.

I think everyone here is in agreement that compile-time c
...

🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2354786373)
reACK 0b3ec8c59b2b6d598531cd4e688eb276e597c825

👍 fanquake approved a pull request: "test: Remove 0.16.3 test from wallet_backwards_compatibility.py"
(https://github.com/bitcoin/bitcoin/pull/30920#pullrequestreview-2354794825)
ACK fae44c83da982095661b034bdd01afe8ac2fb0a6 - I agree that test seems to have past it's usefulness, and the fact that it otherwise causes intemittent issues is further reason to remove it.

🚀 fanquake merged a pull request: "test: Remove 0.16.3 test from wallet_backwards_compatibility.py"
(https://github.com/bitcoin/bitcoin/pull/30920)

💬 maflcko commented on issue "Disallow building fuzz binary without `-DBUILD_FOR_FUZZING`":
(https://github.com/bitcoin/bitcoin/issues/31057#issuecomment-2400143488)
> Historically, we used to build most binaries by default (including the fuzz binary) to ensure that compile failures are caught early by devs. With the switch to CMake this is no longer the case

Why would it no longer be the case with cmake? Are you proposing that every switches to a multi-config build?



> compile failures would be caught by the CI or other devs that regularly fuzz.

How would the CI catch compile or runtime failures on Windows or macOS, when the fuzz exe isn't compi
...

💬 theuni commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2400155446)
Huh, I guess that happens because pch shuffles around the include order.

The redundant decl indeed seems broken to me. Will PR a quick fix.

👍 ryanofsky approved a pull request: "rpc: add optional blockhash to waitfornewblock"
(https://github.com/bitcoin/bitcoin/pull/30635#pullrequestreview-2354766572)
Code review ACK 13b867fb9b37fef3f14732904172e4ecba6fc973. I think implementation could be a ilttle simpler and more friendly though (see suggestion below)

💬 ryanofsky commented on pull request "rpc: add optional blockhash to waitfornewblock":
(https://github.com/bitcoin/bitcoin/pull/30635#discussion_r1792082432)
I think it would be good to simplify all of this to just:

```c++
// Use tip as reference block, unless a block hash was provided.
uint256 current_tip{request.params[1].isNull()
? miner.getTip().value_or(BlockRef{}).hash
: ParseHashV(request.params[1], "blockhash")};

BlockRef block{timeout > 0
? miner.waitTipChanged(current_tip, std::chrono::milliseconds(timeout))
: miner.waitTipChanged(current_tip)};
```

In addition to being simpler, th
...

💬 ryanofsky commented on pull request "rpc: add optional blockhash to waitfornewblock":
(https://github.com/bitcoin/bitcoin/pull/30635#discussion_r1792041394)
In commit "rpc: add optional blockhash to waitfornewblock" (13b867fb9b37fef3f14732904172e4ecba6fc973)

Maybe use phrasing from fa4c0750331f36121ba92bbc2f22c615b7934e52 and say "Method waits for the chain tip to differ from this." I think that's a little better because it describes behavior in general instead of describing a specific case where it returns immediately. I think it would also be better to call this `current_tip` instead of `blockhash` like the other variable, to be more descriptiv
...