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

📝 mzumsande opened a pull request: "rpc, test: add `sendmsgtopeer` rpc and a test for net-level deadlock situation"
(https://github.com/bitcoin/bitcoin/pull/28287)
This adds a `sendmsgtopeer` rpc (for testing only) that allows a node to send a message (provided in hex) to a peer.
While we would usually use a `p2p` object instead of a node for this in the test framework, that isn't possible in situations where this message needs to trigger an actual interaction of multiple nodes.

Use this rpc to add test coverage for the bug fixed in #27981 (that just got merged):
The test lets two nodes (almost) simultaneously send a single large (4MB) p2p message to
...

💬 luke-jr commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#discussion_r1276773348)
Extraneous `\"` shouldn't be there, this is an array

💬 sipa commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1297598582)
I believe that's true, though only due to optimistic send itself (which guarantees that as soon as something is added to `vSendMsg`, it's immediately at least attempted to be moved to the transport). Without it, if there were two consecutive `PushMessage` without intervening `SocketSendData`, `vSendMsg` would already have something in it on the second one.

Because of that, I think it's more "obviously correct" to do the full check here.

💬 sipa commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1297627349)
Right, so I think `!node.m_sock` only happens when the node is already disconnected, or will never actually be connected to anything (in tests). I've added the following comment:

> There is no socket in case we've already disconnected, or in test cases without real connections. In these cases, we bail out immediately and just leave things in the send queue and transport.

Further up, above the `SetMessageToSend` call, I've added:

> This fails when there is an existing message still being
...

📝 brunoerg opened a pull request: "test: fix 'unknown named parameter' test in `wallet_basic`"
(https://github.com/bitcoin/bitcoin/pull/28288)
This PR removes loop when testing an unknown named parameter. They don't have any effect.

💬 brunoerg commented on pull request "rpc, test: add `sendmsgtopeer` rpc and a test for net-level deadlock situation":
(https://github.com/bitcoin/bitcoin/pull/28287#issuecomment-1682814806)
Concept ACK

💬 AndriiHlukhovskyi commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1682836022)
Спасибо огромное очень познавательно я думаю что я тоже что то пропустил
!!! 🔥

чт, 17 авг. 2023 г., 12:58 mehran636311 ***@***.***>:

> Pellse full item my account open and stat
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1682073596>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BAMCSAPI2J4K356QRG4TQDLXVX2PDANCNFSM6AAAAAAW6JC3HY>
> .
> You are receiving this because you commented.M
...

💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1297666902)
Fixed.

💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1297666973)
Done.

💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#discussion_r1297667502)
Good catch, yes. I've added a commit that makes `ChaCha20::SetKey` wipe the buffer.

I don't think we should *in general* wipe the buffer whenever m_bufleft=0, but on a rekey operation it makes sense.

💬 sipa commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#issuecomment-1682883296)
Addressed review comments by @stratospher. Note: new last commit.

💬 jonatack commented on pull request "test: Minor fix in test - locale in terminal":
(https://github.com/bitcoin/bitcoin/pull/28286#issuecomment-1682901139)
Concept ACK, though I'm unable to reproduce this on macOS, which makes sense per `src/common/system.cpp#L66-73`:

```cpp
// On most POSIX systems (e.g. Linux, but not BSD) the environment's locale
// may be invalid, in which case the "C.UTF-8" locale is used as fallback.
#if !defined(WIN32) && !defined(MAC_OSX) && !defined(__FreeBSD__) && !defined(__OpenBSD__) && !defined(__NetBSD__)
try {
std::locale(""); // Raises a runtime error if current locale is invalid
} c
...

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1297701571)
@sipa done! Thank you for the elegant solution.

🤔 jonatack reviewed a pull request: "ci: Disable --coverage temporarily"
(https://github.com/bitcoin/bitcoin/pull/28285#pullrequestreview-1583409232)
ACK based on green CI

Perhaps the following would be better instead, so `test/functional/test_runner.py --coverage` run locally (as well as in the CI) doesn't needlessly report missing coverage.

```diff
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index d93e6fd6da2..14e3a140485 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -847,7 +576,7 @@ class RPCCoverage():
all_cmds = set()
# Consider RPC gene
...