👍 fanquake approved a pull request: "build: Skip secp256k1 ctime tests when tests are not being built"
(https://github.com/bitcoin/bitcoin/pull/30865#pullrequestreview-2300503583)
ACK 23eedc5d1e24b3d31da7902ece5182b9b24672d8

🚀 fanquake merged a pull request: "build: Skip secp256k1 ctime tests when tests are not being built"
(https://github.com/bitcoin/bitcoin/pull/30865)

💬 l0rinc commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1756990148)
But we don't need `bool` for dirty anymore, only for freshness - so we can even get rid of the enum (moving it over to the test maybe). I'll experiment to see what the next smallest change is before this PR, we need to have tests that are more representative of what's actually possible.

🚀 fanquake merged a pull request: "test: fix exclude parsing for functional runner"
(https://github.com/bitcoin/bitcoin/pull/30872)

💬 fjahr commented on pull request "test: Wait for local services to update in feature_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/30880#issuecomment-2346486397)
Ok, `feature_assumeutxo.py` should be good now with this, ready for review.

💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1757005855)
9378b30774d40c8a093c69d2b1807c9eea32455a: I think you need to wrap this in `if(BUILD_TESTS)`

💬 antonilol commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-2346494798)
> ### [minimal_key_value](https://github.com/bitcoin/bitcoin/compare/20b259332ff1d2e6ca6b291ffeda00b51497bbed...hodlinator:bitcoin:pr/24539_minimal_key_value) / [ced5058](https://github.com/bitcoin/bitcoin/commit/ced505834dd1349bccbceeac3bf5abaa2327bda0)
>
> Minimize key & value footprint
>
> ...

This one (the third) is very similar to the 'spending' index of electrs (https://github.com/romanz/electrs), only different being how the key is calculated, electrs uses the first 8 bytes of th
...

💬 maflcko commented on pull request "test: fix exclude parsing for functional runner":
(https://github.com/bitcoin/bitcoin/pull/30872#discussion_r1757015334)
Another way this would have been caught is by making warnings in the CI fatal. There is `ail_on_warn=args.ci` which can be used.

💬 hebasto commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2346507315)
> > What if you ccache --zero-stats after configuring for a new build tree, just before building?
>
> In that case on Fedora it is `Hits: 446 / 446 (100.0%)`, with `Uncacheable calls: 12 / 458 ( 2.62%)`.

If `ccache --show-stats --verbose` outputs:
```
Uncacheable calls: 12 / 458 ( 2.62%)
Called for linking: 12 / 12 (100.0%)
```

this is expected on systems where `ccache` masquerades as a compiler.

💬 maflcko commented on pull request "test: Wait for local services to update in feature_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/30880#issuecomment-2346509249)
review-only ACK 19f4a7c95a99162122068d4badffeea240967a65

I did not test the reproducer.

💬 andrewtoth commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1757015032)
nit: this one line doesn't need to be changed and have the brace moved.

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