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

💬 achow101 commented on pull request "Wallet: (Refactor) GetBalance to calculate used balance":
(https://github.com/bitcoin/bitcoin/pull/29062#discussion_r1579945607)
This needs to be in the above `if` so that the min_depth check is still being done.

💬 sr-gi 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-2077906602)
> I wonder if it may be worth splitting the `wait_for_getheaders` function so the internal function can also be used externally, so we can use that to check for whether a certain message exists without waiting for them (e.g. checking for the negative case) in a similar fashion as done in [bc844fd](https://github.com/bitcoin/bitcoin/commit/bc844fd8a7d130bfb7cf598f5b1acd87acd70e58)
>
> This way we would get rid of a bunch of manual checks like:
>
> ```python
> with p2p_lock:
> assert "
...

💬 sr-gi commented on pull request "test: Makes `wait_for_getdata` delete data on checks, plus allows to check the getdata message type":
(https://github.com/bitcoin/bitcoin/pull/29748#issuecomment-2077916121)
Rebased to fixed merge conflicts after #29736

📝 hebasto opened a pull request: "depends: Do not consider `CC` environment variable for detecting system"
(https://github.com/bitcoin/bitcoin/pull/29963)
On the master branch @ 3c88eac28e8984893746caebb313dc3b2fca90db, consider the following commands in the `depends` subdirectory:
```sh
$ make print-build HOST=i686-pc-linux-gnu CC="clang -m32"
build=x86_64-pc-linux-gnu
$ make print-host HOST=i686-pc-linux-gnu CC="clang -m32"
host=i686-pc-linux-gnu
```
The printed variable values are expected.

However, switching the `CC` variable context from Makefile to the shell environment breaks expectations:
```sh
$ CC="clang -m32" make print-buil
...

💬 ryanofsky 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-2077987295)
Note just to clarify issue history: The race condition fixed here was introduced by 0faafb57f8298547949cbc0044ee9e925ed887ba from #28955, which stopped locking `cs_main` while setting `m_synced = true`. It could cause indexes to incorrectly discard block-connected notifications.

This race condition is different from the other race condition recently fixed in #29776, which is a much older bug going back to 4368384f1d267b011e03a805f934f5c47e2ca1b2 from [#14121](https://github.com/bitcoin/bitcoi
...

💬 theuni commented on pull request "depends: Do not consider `CC` environment variable for detecting system":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-2078014617)
Where/why is this an issue?

💬 hebasto commented on pull request "depends: Do not consider `CC` environment variable for detecting system":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-2078027076)
> Where/why is this an issue?

I faced this issue during my work on CMake branch when `CC` environment variable was defined by the CI -- https://github.com/hebasto/bitcoin/actions/runs/8822538616/job/24220973382.

The log shows no cross-compiling, but it is cross-compiling to `i686-pc-linux-gnu`.

💬 achow101 commented on pull request "net: attempts to connect to all resolved addresses when connecting to a node":
(https://github.com/bitcoin/bitcoin/pull/28834#issuecomment-2078063941)
ACK fd81a37239541d0d508402cd4eeb28f38128c1f2