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

📝 mzumsande opened a pull request: "rpc: remove one more quote from non-string oneline description"
(https://github.com/bitcoin/bitcoin/pull/28289)
This fixes a silent conflict between https://github.com/bitcoin/bitcoin/pull/28123 (which removed all `\"options\"`) and https://github.com/bitcoin/bitcoin/pull/27460 (which added a new one).

It should fix the current CI failures.

💬 mzumsande commented on pull request "ci: Disable --coverage temporarily":
(https://github.com/bitcoin/bitcoin/pull/28285#issuecomment-1682928094)
The root cause for the CI failures is silent merge conflict that should be fixed by #28289 (we might still disable the check though, no opinion on that).

💬 jonatack commented on issue "test: RPC coverage check doesn't work?":
(https://github.com/bitcoin/bitcoin/issues/27593#issuecomment-1682930814)
Verified the current CI issue (red on all pulls) locally

```bash
$ test/functional/test_runner.py --jobs=11 --timeout-factor=0 --coverage

.../...

Uncovered RPC commands:
- Internal
```

An initial fix

```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()

...

💬 fjahr commented on pull request "lint: fix custom mypy cache dir setting":
(https://github.com/bitcoin/bitcoin/pull/28184#discussion_r1297712132)
I see, I simply misread. I changed this to get the base path relative to file as a fallback. We could also simply use `.` or fall back to not setting a `cache_dir` at all but this seemed the most robust option to me.

🤔 jonatack reviewed a pull request: "rpc: remove one more quote from non-string oneline description"
(https://github.com/bitcoin/bitcoin/pull/28289#pullrequestreview-1583436980)
ACK 239431444216850b63ecf01c3b5c5d6d24230d08

With this change, `./test/functional/test_runner.py --jobs=11 --timeout-factor=0 --coverage` no longer warns as it does on current master:

```
Uncovered RPC commands:
- Internal
```

💬 andrewtoth commented on pull request "validation: Flush state after initial sync":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1297745509)
Hmm it seems like this is the ordering for all methods in this class definition.

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297744944)
> Why stop doing extra block-relay peers in this situation?

Because, in the stalling scenario, we not only need to progress at the chain level, we might also need to receive new addresses from the network. And block-relay-only connections are limited in that sense. The node rejects addresses coming from them.

> If for whatever reason our current peers aren't able to send us the blocks to get us out of this situation, maybe trying an additional one will.

Thats basically what we currently
...

⚠️ mzumsande opened an issue: "Intermittent failure in feature_config_args.py"
(https://github.com/bitcoin/bitcoin/issues/28290)
https://cirrus-ci.com/task/5383892962639872?logs=ci#L7698

```
2023-08-17T20:59:43.754000Z TestFramework (INFO): Test seed peers
2023-08-17T21:03:45.922000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main
self.run_test()
File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/feature_config_
...

💬 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_r1297759286)
Update in e5f20cb26848c97d48c6a479c4680f6823f2ec1e