💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712626637)
Resolved in https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2281111100.

💬 paplorinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712626646)
Might be controversial, but other Bitcoin clones seem to split into multiple projects after their CMake migrations:
* https://github.com/Bitcoin-ABC/bitcoin-abc/blob/master/src/bench/CMakeLists.txt#L3
* https://github.com/bitcoin-cash-node/bitcoin-cash-node/blob/master/src/bench/CMakeLists.txt#L3
* https://github.com/bitcoin-sv/bitcoin-sv/blob/master/src/test/CMakeLists.txt#L5

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712626661)
Resolved in https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2281111100.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712626746)
Resolved in https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2281111100.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712627020)
> I wasn't able to run the fuzz tests with cmake

Resolved in https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2281111100.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712627346)
Still curios about:
> What are the benefits of maintaining the build system in separate projects?

💬 paplorinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712628124)
I'm also curious, that's what I'm asking basically, why did the other clones decide to do this differently.

I could only find general `Modularity and Reusability/Scalability/Improved Dependency Management/Isolation of Build Configurations` arguments online, but not sure any applies here, hence my question.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712633831)
> Might be controversial, but other Bitcoin clones seem to split into multiple projects after their CMake migrations:
>
> * https://github.com/Bitcoin-ABC/bitcoin-abc/blob/master/src/bench/CMakeLists.txt#L3

It is not controversial, just broken :)

The `cmake -S src/bench -B build` command fails.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712637404)
See https://github.com/hebasto/bitcoin/pull/320.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712637433)
See https://github.com/hebasto/bitcoin/pull/320.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712637443)
See https://github.com/hebasto/bitcoin/pull/320.

💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2281899749)
This change didn't appear to have any speed impact for a reindex-chainstate using max dbcache. Ran the following benchmark:
```
nohup hyperfine \
--export-markdown ~/bench.md \
--show-output \
--parameter-list commit 81508954f1f89f95b6fdca10c3c471cdb6566c80,27a770b34b8f1dbb84760f442edb3e23a0c2420b \
--prepare 'git checkout {commit} && \
make -j$(nproc) src/bitcoind; \
sync; sudo /sbin/sysctl vm.drop_caches=3;' \
-M 2 \
'echo '{commit}' && \
./src/bitcoind -datadir=/home/user/.bitcoin
...

💬 1440000bytes commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2282178130)
Concept ACK

📝 maflcko opened a pull request: "doc: Remove outdated nTx faking comment"
(https://github.com/bitcoin/bitcoin/pull/30624)
This problematic `nTx` "faking" was removed in commit b50554babdddf452acaa51bac757736766c70e81.

So fix the wrong comment.

Also, address the typo nits from:

* https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1531789314
* https://github.com/bitcoin/bitcoin/pull/30598#discussion_r1711982543

💬 maflcko commented on pull request "assumeutxo: Drop block height from metadata":
(https://github.com/bitcoin/bitcoin/pull/30598#discussion_r1712660234)
Done as part of https://github.com/bitcoin/bitcoin/pull/30624

👍 tdb3 approved a pull request: "cli: restrict multiple exclusive argument usage in bitcoin-cli"
(https://github.com/bitcoin/bitcoin/pull/30148#pullrequestreview-2231464882)
cr re ACK c8e6771af002eaf15567794fcdc57fdb0e3fb140
Diff showed minor changes to comment header of `CheckMultipleCLIArgs()`

💬 agroce commented on issue "fuzz: `script`: Assertion `!extract_destination_ret' failed.":
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2282202407)
This actually leads to a q I was wondering about for OSS-Fuzz: are there configs for appropriate targets with smaller max-lens?

💬 fjahr commented on pull request "doc: Remove outdated nTx faking comment":
(https://github.com/bitcoin/bitcoin/pull/30624#issuecomment-2282212130)
ACK fa04511e44bc6c740ff1c4f912f689b2bc3d1288

🤔 stratospher reviewed a pull request: "addrman: change internal id counting to int64_t"
(https://github.com/bitcoin/bitcoin/pull/30568#pullrequestreview-2231495237)
Concept ACK. there's also if you retouch. https://github.com/bitcoin/bitcoin/blob/c2d15d993ef06d97d4c117012bda6efa3dcbac45/src/test/fuzz/addrman.cpp#L189

💬 gmaxwell commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2282238485)
I didn't comment on IBD because I figured a hit per hour isn't that significant in the context of IBD and likely pays for itself in improving crash robustness. I'm a little surprised that the effect is quite so small, but it does probably improve performance too by making writes a little more concurrent.

Sipa: good point on the injection locking, I've previously wondered if the the 24 hour behavior could sync up.