👍 andrewtoth approved a pull request: "refactor: migrate `bool GetCoin` to return `optional<Coin>`"
(https://github.com/bitcoin/bitcoin/pull/30849#pullrequestreview-2300543881)
Code review ACK d2146c317b63f635e3683cb6e7d0913be6c96f3f

This change simplifies the `GetCoin` interface, so callers don't need to consider both the return value and an out parameter. This lets us remove test code that exercises different combinations of the return value and out parameter, which will let us make test coverage simpler and more sane.

👍 pablomartin4btc approved a pull request: "test: Wait for local services to update in feature_assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/30880#pullrequestreview-2300586372)
tACK 19f4a7c95a99162122068d4badffeea240967a65

I've managed to reproduce it locally as per instructions in the description, adding a wait for 3 secs and with this PR built there were no errors.

💬 l0rinc commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1757040131)
I'll revert if more changes are needed

⚠️ fanquake opened an issue: "cmake: GenerateHeaderFrom very slow"
(https://github.com/bitcoin/bitcoin/issues/30881)
Looks like a regression from #30842. Reported in that PR: https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2346074452:
```bash
Not sure why, but for me the command took almost an hour:

$ ps -p 816447 --format time,rss,cmd
TIME RSS CMD
00:51:12 117640 /usr/bin/cmake -DRAW_SOURCE_PATH=_/src/bench/data/block413567.raw -DHEADER_PATH=_/bld-cmake/src/bench/data/block413567.raw.h -DRAW_NAMESPACE=benchmark::data -P _/cmake/script/GenerateHeaderFromRaw.cmake

Previously it took o
...

💬 sipa commented on issue "cmake: GenerateHeaderFrom very slow":
(https://github.com/bitcoin/bitcoin/issues/30881#issuecomment-2346542352)
Reverting #30842 fixes the issue for me.

💬 fanquake commented on pull request "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`":
(https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2346542625)
Opened #30881 to track this, given a second dev has reported performance issues.

👍 fanquake approved a pull request: "kernel: Create usable static kernel library"
(https://github.com/bitcoin/bitcoin/pull/30814#pullrequestreview-2300616256)
ACK 0dd16d7118f10ac291a45644769121cbdfd2352f - this looks like a good place to start.

💬 fanquake commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#issuecomment-2346575257)
https://github.com/bitcoin/bitcoin/pull/30866/checks?check_run_id=29955732214:
```bash
In file included from /ci_container_base/src/script/descriptor.cpp:10:
/ci_container_base/src/script/miniscript.h: In instantiation of ‘miniscript::Node<Key> miniscript::Node<Key>::Clone() const [with Key = unsigned int]’:
/ci_container_base/src/script/descriptor.cpp:1363:124: required from here
/ci_container_base/src/script/miniscript.h:535:33: error: moving a local object in a return statement prevent
...

💬 ryanofsky commented on pull request "interfaces: #30697 follow ups":
(https://github.com/bitcoin/bitcoin/pull/30828#discussion_r1757085271)
> I think the only thing that "bothers" me a bit about the nullable class is the different meanings it could have, which require additional clarifications at the API level.

Yeah this is real concern, and I think a common thing that happens with null values, true/false values, empty strings, and special integer values. If you are sloppy with these values you will run into problems. If you are thoughtful you can come up with very clean interfaces, but it is hard to be careful. You can avoid pr
...

📝 dergoegge opened a pull request: "wip: Split fuzz binary (take 2)"
(https://github.com/bitcoin/bitcoin/pull/30882)
Closes https://github.com/bitcoin/bitcoin/issues/28971

In addition to the benefits listed in #28971, this should also enable us to use https://github.com/ossf/fuzz-introspector provided by oss-fuzz. Our current runtime harness selection blocks introspector's static analysis from working properly (e.g. it can't statically determine which functions are reachable by a given harness).

This PR uses the approach suggested here: https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-184620474
...

✅ fanquake closed an issue: "Testnet fixed seeds don't work"
(https://github.com/bitcoin/bitcoin/issues/29574)

💬 fanquake commented on issue "Testnet fixed seeds don't work":
(https://github.com/bitcoin/bitcoin/issues/29574#issuecomment-2346588334)
This was fixed.

💬 dergoegge commented on pull request "wip: Split fuzz binary (take 2)":
(https://github.com/bitcoin/bitcoin/pull/30882#discussion_r1757096120)
Currently an individual `test_fuzz_*` lib and `fuzz_*` binary is produced for each harness. It's kind of ugly to duplicate this loop but I'm not sure to avoid it. Another loop would likely need to be added for the wallet harnesses as well.

@hebasto @fanquake @maflcko Any ideas?

💬 sr-gi commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#discussion_r1757098852)
I moved it up in [07f4ceb](https://github.com/bitcoin/bitcoin/pull/30233/commits/07f4cebe5780f1039541d989e64b70eccc5b4eb5) and renamed the internal `peer` to `peer_ref` to avoid a conflict

💬 sr-gi commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#discussion_r1757099028)
Addressed in [07f4ceb](https://github.com/bitcoin/bitcoin/pull/30233/commits/07f4cebe5780f1039541d989e64b70eccc5b4eb5)

💬 dergoegge commented on pull request "wip: Split fuzz binary (take 2)":
(https://github.com/bitcoin/bitcoin/pull/30882#issuecomment-2346596824)
Trying to produce an introspector report using this branch: https://github.com/dergoegge/oss-fuzz/tree/2024-09-bitcoin-introspector

💬 sr-gi commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#issuecomment-2346597866)
Amended to cover @maflcko nits and rebased to include cmake

💬 hebasto commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757100406)
`CCACHE_BASEDIR=${CMAKE_BINARY_DIR}` is effective when building in different build directories from the same source directory.

`CCACHE_BASEDIR=${CMAKE_SOURCE_DIR}` works for building from different source directories.

We need to choose one of these two options.

👍 ryanofsky approved a pull request: "util: Use consteval checked format string in FatalErrorf, LogConnectFailure"
(https://github.com/bitcoin/bitcoin/pull/30546#pullrequestreview-2300689204)
Code review ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678

Just test code cleanup since last review

👍 ryanofsky approved a pull request: "kernel: Create usable static kernel library"
(https://github.com/bitcoin/bitcoin/pull/30814#pullrequestreview-2300694088)
Code review ACK 0dd16d7118f10ac291a45644769121cbdfd2352f

Just cmake style improvements since last reveiw