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

📝 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)