🤔 jonatack reviewed a pull request: "doc: clarify cookie generation in JSON-RPC-interface.md"
(https://github.com/bitcoin/bitcoin/pull/28283#pullrequestreview-1583026183)
Concept ACK

💬 jonatack commented on pull request "doc: clarify cookie generation in JSON-RPC-interface.md":
(https://github.com/bitcoin/bitcoin/pull/28283#discussion_r1297476742)
```suggestion
- **Secure authentication:** By default, when no `-rpcpassword` configuration option is specified, Bitcoin Core generates unique
```

----

Unrelated, is this still planned?

`src/httprpc.cpp:251: LogPrintf("Config options rpcuser and rpcpassword will soon be deprecated. Locally-run instances may remove rpcuser to use cookie-based auth, or may be replaced with rpcauth. Please see share/rpcauth for rpcauth auth generation.\n");
`

💬 jonatack commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1297489117)
If you retouch, the transition to severity-based logging will be easier if used when adding new ones. Probably choose between error, warning, or info level here; all 3 will be logged unconditionally by default, unless the user sets a custom severity level.

```suggestion
LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "%s\n", "test"); });
LogPrintLevel(BCLog::BLOCKSTORAGE, "Failed to flush previous block file %05i (finalize=%i, finalize_undo=%i) before opening new bl
...

💬 jonatack commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1297490048)
Idem as https://github.com/bitcoin/bitcoin/pull/27866/files#r1297489117 if you retouch.

💬 sr-gi commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1682645449)
> Maybe the test? Though, I haven't looked at it recently.

That may make sense. I'll take a look and rebase

📝 MarcoFalke opened a pull request: "ci: Disable --coverage temporarily"
(https://github.com/bitcoin/bitcoin/pull/28285)
Looks like this causes all CI builds to be red, and doesn't work anyway, see https://github.com/bitcoin/bitcoin/issues/27593 . Temporarily disable it to allow for more time to rework it from scratch.

💬 MarcoFalke commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#issuecomment-1682662462)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

💬 jonatack commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297514131)
This is already included 3 lines higher.

💬 jonatack commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297514797)
```suggestion
constexpr int DEFAULT_TOR_CONTROL_PORT = 9051;
```

📝 crywolf opened a pull request: "test: Minor fix in test - locale in terminal"
(https://github.com/bitcoin/bitcoin/pull/28286)
Running tests with non-English locale set in Linux terminal makes system tests (`system_tests/run_command`) fail. Setting the locale in failing test explicitly to 'C' fixes the failure.

_How to reproduce test failure:_
Setting LC_ALL=cs_CZ.UTF-8 (`export LC_ALL=cs_CZ.UTF-8`) in terminal and then running `make check` should be enough.

_Fix:_
My terminal is by default set to non-English locale and the test failed because it checks for English error message, but the terminal produces error
...

💬 kevkevinpal commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#issuecomment-1682682090)
> Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

rebased properly in [5d94b1f](https://github.com/bitcoin/bitcoin/pull/28101/commits/5d94b1faecd8fc98017510d86f6ea5dc002ec0a3)

💬 kevkevinpal commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297526595)
thanks, addressed in [5d94b1f](https://github.com/bitcoin/bitcoin/pull/28101/commits/5d94b1faecd8fc98017510d86f6ea5dc002ec0a3)

💬 crywolf commented on pull request "test: Minor fix in test - locale in terminal":
(https://github.com/bitcoin/bitcoin/pull/28286#issuecomment-1682682796)
During the workshop wit Gloria @glozow on Dev/Hack/Day (part of BTC Prague conference) my tests failed. Later that day I found what the problem was and now I am finally presenting my fix as an attempt to make my first PR in this repo.

💬 kevkevinpal commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297527127)
thanks didnt know that fixed in

[5d94b1f](https://github.com/bitcoin/bitcoin/pull/28101/commits/5d94b1faecd8fc98017510d86f6ea5dc002ec0a3)

💬 jonatack commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297532538)
Second sentence seems redundant:
```
$ ./src/bitcoind -h | grep -A3 torcontrol
-torcontrol=<ip>:<port>
Tor control host and port to use if onion listening enabled (default:
127.0.0.1:9051). If no port is specified, the default port will
be used (default: 9051).
```

```suggestion
+ argsman.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control host and port to use if onion listening enabled (default: %s). If no port is specified, the default port of %i will
...

💬 jonatack commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#issuecomment-1682692907)
Approach ACK modulo https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297532538

💬 MarcoFalke commented on pull request "ci: Use qemu-user through container engine":
(https://github.com/bitcoin/bitcoin/pull/28087#issuecomment-1682703777)
> Had GCC segfault more than once when cross-compiling s390x.

I also ran into this on aarch64, and I presume it is just the CPU overheating, because it passed with `MAKEJOBS="-j1"`

💬 MarcoFalke commented on pull request "ci: Disable --coverage temporarily":
(https://github.com/bitcoin/bitcoin/pull/28285#issuecomment-1682709592)
Looks like CI is red since https://github.com/bitcoin/bitcoin/commit/ecb20563b6a0cdd615628da9caa1e7389998119c, but on the pull, it passed last week: https://cirrus-ci.com/task/5524872076460032

💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1297560652)
Fixed, thank you.

💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1682735790)
Updated per `git range-diff 6ce5e8f ce8faad 05143b2` to take review feedback by @ajtowns (thanks!) and rebase for CI.