💬 theuni commented on pull request "RFC: depends: add release type to CMake builds":
(https://github.com/bitcoin/bitcoin/pull/29962#issuecomment-2077772971)
Ping @hebasto

💬 achow101 commented on pull request "test: Extends wait_for_getheaders so a specific block hash can be checked":
(https://github.com/bitcoin/bitcoin/pull/29736#issuecomment-2077785663)
ACK c4f857cc301d856f3c60acbe6271d3fe19441c7a

💬 hebasto commented on pull request "RFC: depends: add release type to CMake builds":
(https://github.com/bitcoin/bitcoin/pull/29962#issuecomment-2077790003)
The defaults for the "Makefile" generator are:
- "RelWithDebInfo" : `-O2 -g -DNDEBUG`
- "Debug': `-g`

That differs from our flags, which are `-O2` and `-O1 -g` for Linux respectively.

And CMake can changes its defaults at any time (

However, it would be nice to be explicit about the used build type, for example, `-DCMAKE_BUILD_TYPE=None`.

✅ achow101 closed an issue: "test: Check hash in wait_for_get* helpers"
(https://github.com/bitcoin/bitcoin/issues/18614)

🚀 achow101 merged a pull request: "test: Extends wait_for_getheaders so a specific block hash can be checked"
(https://github.com/bitcoin/bitcoin/pull/29736)

👍 brunoerg approved a pull request: "net: Make AddrFetch connections to fixed seeds"
(https://github.com/bitcoin/bitcoin/pull/26114#pullrequestreview-2023175574)
ACK d8df9f94f933c9d5fd19a58a4b1473ea9becd362

💬 theuni commented on pull request "RFC: depends: add release type to CMake builds":
(https://github.com/bitcoin/bitcoin/pull/29962#issuecomment-2077817173)
Right, but a few more things to consider:

An upstream build could be [following the docs](https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#build-configurations) and doing something like:
```cmake
# Works correctly for both single and multi-config generators
target_compile_definitions(exe1 PRIVATE
$<$<CONFIG:Debug>:DEBUG_BUILD>
)
```
In this case, we wouldn't currently pick up the extra debug opts.

The other thing to consider that is that we could be using
`CMAK
...

💬 achow101 commented on pull request "test: fix accurate multisig sigop count (BIP16), add unit test":
(https://github.com/bitcoin/bitcoin/pull/29615#issuecomment-2077824397)
ACK 3e9c736a26724ffe3b70b387995fbf48c06300e2

💬 achow101 commented on pull request "tests: fix `OP_1NEGATE` handling in `CScriptOp`":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2077837600)
I concur with @dgpv that these functions should behave the same as the C++ functions.

However, I think it would be okay for both the C++ and the test functions to be modified to handle `OP_1NEGATE`, although that may touch consensus code so the bar for merging will be higher.

🚀 achow101 merged a pull request: "test: fix accurate multisig sigop count (BIP16), add unit test"
(https://github.com/bitcoin/bitcoin/pull/29615)

💬 achow101 commented on pull request "contrib: rpcauth.py - Add new option (-json) to output text in json format":
(https://github.com/bitcoin/bitcoin/pull/29433#issuecomment-2077844472)
ACK 9adf949d2aa6d199b85295b18c08967395b5570a

🚀 achow101 merged a pull request: "contrib: rpcauth.py - Add new option (-json) to output text in json format"
(https://github.com/bitcoin/bitcoin/pull/29433)

💬 laanwj commented on pull request "RFC: depends: add release type to CMake builds":
(https://github.com/bitcoin/bitcoin/pull/29962#issuecomment-2077848518)
i think `-O2 -g` (RelWIthDebInfo) for the release makes sense, it means that the distributed binary will be optimized, but still get (as far as possible) descriptive debug information such as line numbers and symbols in case it's necessary for troubleshooting, getting a traceback, figuring out where some crash address comes from, and so on.

💬 achow101 commented on pull request "index: race fix, lock cs_main while 'm_synced' is subject to change":
(https://github.com/bitcoin/bitcoin/pull/29867#issuecomment-2077860885)
ACK 65951e0418c53cbbf30b9ee85e24ccaf729088a1

✅ achow101 closed an issue: "ci: failure in `rpc_scanblocks.py`"
(https://github.com/bitcoin/bitcoin/issues/29831)

🚀 achow101 merged a pull request: "index: race fix, lock cs_main while 'm_synced' is subject to change"
(https://github.com/bitcoin/bitcoin/pull/29867)

💬 achow101 commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#issuecomment-2077879385)
ACK 7c0c599f1e6d412b3cb92cd67aa46644a8286cb6

💬 sr-gi commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1579936778)
Nice catch!

💬 sr-gi commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#issuecomment-2077888934)
Rebased on top of https://github.com/bitcoin/bitcoin/pull/29736. Now this is using `wait_for_getheaders` to reduce the boilerplate of having to manually pop the results from `last_message`.

cc/ @stratospher

💬 achow101 commented on pull request "Wallet: (Refactor) GetBalance to calculate used balance":
(https://github.com/bitcoin/bitcoin/pull/29062#discussion_r1579949478)
It would be a bit easier to understand if this subtraction was done with each addition during the loop, rather than all at the end. Also a comment for why this is being done would be useful, as it was not immediately obvious to me at first that the purpose of this is to remove the overlap.