🤔 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

💬 murchandamus commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1297760127)
Okay, I replaced `GetSummedBumpFees()` with `GetTotalBumpFees()` which deducts the `bump_fee_discount` if it has been set.

💬 murchandamus commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1297768542)
Removed SSFO from this test

💬 murchandamus commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#issuecomment-1683003630)
I believe that I have now addressed all open feedback. Ready for review

🤔 jonatack reviewed a pull request: "init: changing -torcontrol help to specify that a default port is used"
(https://github.com/bitcoin/bitcoin/pull/28101#pullrequestreview-1583526315)
ACK e5f20cb26848c97d48c6a479c4680f6823f2ec1e

It might be good to prefix the pull title with `doc, refactor:` instead of `init`.

💬 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_r1297772924)
If you retouch, can remove this added newline.

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297791137)
Yeah. Sure for both.
Not sure why I hardcoded the number..

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297796093)
> Was it just outdated already? I'm confused by what NODE_NONE meant here before this change.

It was talking about the input `ServiceFlags`. Not the `GetDesirableServiceFlags()` output.

Basically, we expect to have all the hardcoded seeds supporting `NODE_NETWORK` and `NODE_WITNESS`.

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297797152)
yeah sure.

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297802187)
> The comment ("shortcut for...") was helpful for me, any reason to drop it?

That was because the method was moved to the net interface ([see](https://github.com/bitcoin/bitcoin/blob/1428b7f4e8c648fdc68ba5744d640d120d5d78eb/src/net.h#L673)). And there, this is no longer a shortcut. It is the only available method.

👍 theStack approved a pull request: "crypto: more `Span<std::byte>` modernization & follow-ups"
(https://github.com/bitcoin/bitcoin/pull/28100#pullrequestreview-1583609952)
re-ACK 57cc136282c38825e97bbf85728df4bdf1ccc648

(CI failure is unrelated)