💬 maflcko commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2196975759)
yeah, i think the `std::hash` can be dropped here

🚀 fanquake merged a pull request: "test: Turn rpcauth.py test into functional test"
(https://github.com/bitcoin/bitcoin/pull/32881)

📝 maflcko opened a pull request: "test: Add missing convert_to_json_for_cli"
(https://github.com/bitcoin/bitcoin/pull/32932)

💬 maflcko commented on pull request "wallet, test: best block locator matches scan state follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32580#issuecomment-3056445715)
silent conflict fixed in https://github.com/bitcoin/bitcoin/pull/32932

📝 maflcko opened a pull request: "log: Properly log warnings with warn loglevel in addrdb"
(https://github.com/bitcoin/bitcoin/pull/32933)
The logging in addrdb is confusing, because it uses `LogPrintf` (info level) to log warnings.

Fix this by properly using the `warn` level, where needed. Also, drop unused trailing `\n` while touching the lines.

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2197128817)
> rename from xor to obfuscation is really needed or meaningful

My thinking was that `xor` describes the implementation while `obfuscation` describes the purpose - there are many xor uses throughout the codebase, but fewer places where we specifically need obfuscation.
I also noticed that our existing patterns already lean toward the obfuscation terminology: the printed messages reference `obfuscation key`, the database entry is `obfuscate_key`, and the parameter is `.obfuscate`.

> xor ap
...

⚠️ cyjseagull opened an issue: "Why bitcoin use so many RecursiveMutex"
(https://github.com/bitcoin/bitcoin/issues/32934)
### Please describe the feature you'd like to see added.

I have noticed that in the Bitcoin source code, almost all thread-safe sections use `RecursiveGuard`.
As we all know, `RecursiveGuard` is **exclusive**, which can **impact system performance and concurrency**.

At the same time, I have also noticed [another issue](https://github.com/bitcoin/bitcoin/issues/19303) specifically tracking the replacement of `RecursiveMutex` with `Mutex`, which would help reduce some of the locking overhead.


...

👍 fanquake approved a pull request: "test: Add missing convert_to_json_for_cli"
(https://github.com/bitcoin/bitcoin/pull/32932#pullrequestreview-3004869160)
ACK fa0528479d5e37833fa66395c94d4611aa9270f6

🚀 fanquake merged a pull request: "test: Add missing convert_to_json_for_cli"
(https://github.com/bitcoin/bitcoin/pull/32932)

💬 fanquake commented on pull request "qa: Avoid knock-on exception in assert_start_raises_init_error":
(https://github.com/bitcoin/bitcoin/pull/32929#issuecomment-3056708522)
(Test failure here was fixed in #32932)

💬 maflcko commented on pull request "clang-format: align brace-after-struct and *-class formatting":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2197233259)
in 7c9b3e1eae8f206753457149f1b1c837f6627d6d: How to reproduce the commit on non-macos? I don't have an apple, so I tried:

```
# clang-format-16 --version
Ubuntu clang-format version 16.0.6 (23ubuntu4)
```

However, the command:

```
clang-format-16 -dump-config > .clang-format
```

produces a different result:

```diff
# git diff -U0
diff --git a/src/.clang-format b/src/.clang-format
index 2f3f96ae2e..0822db5d11 100644
--- a/src/.clang-format
+++ b/src/.clang-format
@@ -0
...

⚠️ olegrok opened an issue: "ci: don't pass GIT_EXEC_PATH inside test container"
(https://github.com/bitcoin/bitcoin/issues/32935)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

https://github.com/bitcoin/bitcoin/blob/83ae7802fe14af7eed9edd830f51a3269cc2bd23/ci/test/02_run_container.sh#L15C22-L15C24

If host system is not ubuntu it could set GIT_EXEC_PATH to /usr/libexec/git-core (it should be /usr/lib/git-core) that causes `git: 'remote-https' is not a git command. See 'git --help'.` error.

You need to change it to something like:
```bash
python3 -c '
import o
...

💬 l0rinc commented on pull request "clang-format: align brace-after-struct and *-class formatting":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2197286287)
> How to reproduce the commit on non-macos

Please see the commit messages - should I move the instructions to the PR description as well?

> produces a different result

I think you have dumped the defaults that don't load our config first, which are indeed different from what we have (hence our own config).
```bash
$ clang-format-16 --version
Ubuntu clang-format version 16.0.6 (23ubuntu4)
$ clang-format-16 -dump-config | grep AfterClass
AfterClass: false
$ clang-format-16 -d
...

💬 maflcko commented on issue "ci: don't pass GIT_EXEC_PATH inside test container":
(https://github.com/bitcoin/bitcoin/issues/32935#issuecomment-3056903564)
> ### Steps to reproduce
>
> Run CI script on e.g. awslinux and try to clone something inside build scripts

You'll have to include the full and exact steps to reproduce.

Did you use https://github.com/bitcoin/bitcoin/blob/c4f90900b55f1a77fa68b627f73a494fab0ed98d/ci/README.md#L46 ?



> exclude

I don't think a list with excludes is maintainable. It is never going to be complete anyway. (see e.g. https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2586828644, or any linux system that s
...

⚠️ hebasto opened an issue: "SegFault in `coinstatsindex_tests`"
(https://github.com/bitcoin/bitcoin/issues/32936)
System NetBSD 10.1, GCC 14.2.0, using system packages:
```
29/141 Test #32: coinstatsindex_tests .................***Exception: SegFault 1.20 sec
```

Full log is available [here](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/16185521919/job/45690339085).

💬 stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2197391680)
> So, I think, the error should say: "Both blockhashes and descriptors arrays must be specified", and the `.empty()` check should be removed for both.

I think I agree with that. It seems that the RPC works fine (did not properly inspect the call graph/code) with empty blockhashes and descriptors (modulo not calling `.get_array()`):

```
% cli getdescriptoractivity '[]' '[]'
{
"activity": [
]
}
```

I don't think we need to protect the user against providing empty arrays if it is
...

💬 maflcko commented on pull request "clang-format: align brace-after-struct and *-class formatting":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2197393675)
> `clang-format-16 -dump-config > .clang-format`

to answer my own question, I guess redirecting `>` is the problem here. It works fine when redirecting to a tmp file first: `( clang-format-16 -dump-config > tmp ) && mv tmp ./.clang-format`

📝 fanquake opened a pull request: "Enable `-Werror=dev` in CI & Guix"
(https://github.com/bitcoin/bitcoin/pull/32937)
Pass `-Werror=dev` in the CI, Guix and the `dev-mode` preset.

https://cmake.org/cmake/help/latest/manual/cmake.1.html#cmdoption-cmake-Werror:
> Make developer warnings errors.
> Make warnings that are meant for the author of the CMakeLists.txt files errors. By default this will also turn on deprecated warnings as errors.

Pulled out of #32865.

👍 maflcko approved a pull request: "clang-format: align brace-after-struct and *-class formatting"
(https://github.com/bitcoin/bitcoin/pull/32813#pullrequestreview-3005160856)
I haven't checked if this fixes the issues with your IDE, but I guess it makes sense either way to get the clang-format config up to date. clang-16 is the minimum required for about a year now, so every developer should have access to clang-format-16 in some way or another.

I've recreated the commits on Ubuntu and reviewed the diff:

review ACK 94364a7d5403d9d480ebc065f8709c6dd21e543f 🥙

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secr
...

💬 hebasto commented on pull request "cmake: Use `AUTHOR_WARNING` for warnings":
(https://github.com/bitcoin/bitcoin/pull/32865#discussion_r2197410784)
> > Reintroducing this block + your patch will only do something if we also reintroduce the custom warning handling (`configure_warnings`
>
> Yes, that was my suggestion: Replace the commit with a trivial one-line patch.

I support @maflcko's suggestion.