💬 maflcko commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#issuecomment-2346436631)
re-review ACK 23eedc5d1e24b3d31da7902ece5182b9b24672d8

only change is a comment

💬 achow101 commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2346444421)
Discussion from today's IRC meeting is that we should move forward with @davidgumberg's patch and default to false on Windows for this release.

💬 maflcko commented on pull request "test: fix exclude parsing for functional runner":
(https://github.com/bitcoin/bitcoin/pull/30872#issuecomment-2346447243)
(Let's merge this minimal bugfix first, and then change the output in a follow-up. Otherwise, this will be delayed and running the s390x will not be possible out of the box)

🤔 pablomartin4btc reviewed a pull request: "test: Wait for local services to update in feature_assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/30880#pullrequestreview-2300493751)
ACK d823f59c91cb00fe763b0c036454440f1948bf48

Perhaps also at this line L701 too:

`assert {'NETWORK', 'NETWORK_LIMITED'}.issubset(n2.getnetworkinfo()['localservicesnames'])`

(It also errors there: https://cirrus-ci.com/task/5740917921939456?logs=ci#L3362)

💬 fjahr commented on pull request "test: Wait for local services to update in feature_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/30880#issuecomment-2346457532)
> Perhaps also at this line L701 too:
>
> `assert {'NETWORK', 'NETWORK_LIMITED'}.issubset(n2.getnetworkinfo()['localservicesnames'])`
>
> (It also errors there: https://cirrus-ci.com/task/5740917921939456?logs=ci#L3362)

Yeah, I just saw that after I succeeded to reproduce it locally, so don't merge yet please.

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