💬 amitiuttarwar commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#discussion_r1278512102)
```suggestion
{RPCResult::Type::NUM, "total", "total number of addresses in both new/tried tables"},
```

having the clause "from a network" only on total field (vs also on new and tried) is confusing. a bit redundant anyways since these fields are all nested within the networks

💬 amitiuttarwar commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#discussion_r1278512493)
```suggestion
{RPCResult::Type::NUM, "tried", "number of addresses in the tried table, which represent peers with whom the node has successfully connected to in the past."},
```

if you take the previous suggestion, I'd update this too for consistency

💬 amitiuttarwar commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#discussion_r1278516272)
reading the chatGPT suggestion, if you want more detail, I think it'd make more sense to use the part that says they are recently discovered but have not yet been successfully connected to.

💬 amitiuttarwar commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1657055010)
I think this PR should be in draft right now since it builds on top of #27511 & has some outdated changes / the two commits are represented as a "merge" commit here instead of separately. I would either put it into draft or keep it up-to-date in a form that would be ready for master assuming sufficient review.

💬 MarcoFalke commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1278521260)
It may be good to enumerate why this is needed (if at all).

💬 MarcoFalke commented on pull request "qa, doc: Fix comment":
(https://github.com/bitcoin/bitcoin/pull/28181#discussion_r1278522512)
```suggestion
Check that there all python files in this directory are categorized as a test script or meta script.
```

💬 YellowRoseCx commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1657075910)
Replace by fee makes double spending easier and harm's Bitcoin's ability to be used as a currency in my opinion

⚠️ MarcoFalke opened an issue: "lint: MYPY_CACHE_DIR is unused"
(https://github.com/bitcoin/bitcoin/issues/28183)
See also the discussion in https://github.com/bitcoin/bitcoin/pull/24794/files#r844848504 . However, the variable is completely unused:

```
$ git grep MYPY_CACHE_DIR
test/lint/lint-python.py:MYPY_CACHE_DIR = f"{os.getenv('BASE_ROOT_DIR', '')}/test/.mypy_cache"
```

Not sure if it was ever needed. If not, it should be removed. cc @fjahr

💬 hebasto commented on pull request "qa, doc: Fix comment":
(https://github.com/bitcoin/bitcoin/pull/28181#discussion_r1278534555)
Thanks! Updated.

📝 fjahr opened a pull request: "lint: fix custom mypy cache dir setting"
(https://github.com/bitcoin/bitcoin/pull/28184)
fixes #28183

The custom cache dir for `mypy` can only be set via an environment variable, setting the `MYPY_CACHE_DIR` variable in the program is not sufficient. This error was introduced while translating the shell script to python.

See also the mypy documentation: https://mypy.readthedocs.io/en/stable/config_file.html#confval-cache_dir

🤔 MarcoFalke reviewed a pull request: "lint: fix custom mypy cache dir setting"
(https://github.com/bitcoin/bitcoin/pull/28184#pullrequestreview-1553612167)
left some questions

💬 MarcoFalke commented on pull request "lint: fix custom mypy cache dir setting":
(https://github.com/bitcoin/bitcoin/pull/28184#discussion_r1278539461)
Why is this needed?

💬 MarcoFalke commented on pull request "lint: fix custom mypy cache dir setting":
(https://github.com/bitcoin/bitcoin/pull/28184#discussion_r1278539545)
This will write to `/test/` if a user is running this script:

```
# ./test/lint/lint-python.py
Success: no issues found in 269 source files
# ls /test/.mypy_cache/
3.11 CACHEDIR.TAG

📝 MarcoFalke opened a pull request: " ci: Use hard-coded root path for CI containers (bugfix)"
(https://github.com/bitcoin/bitcoin/pull/28185)
Currently the CI system will fail if the git folder that holds the Bitcoin Core source is moved from one location to another.

Fix this by just using a single hard-coded root path *inside* the CI system containers.

Steps to test:

* Fresh install of your operating system, or alternatively, clear the depends cache with: `docker volume rm ci_win64_depends`
* Run the CI system: `MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_win64.sh" ./ci/test_run_all.sh`
* Move the git folder: `p
...

💬 MarcoFalke commented on pull request "ci: Switch more tasks to self-hosted":
(https://github.com/bitcoin/bitcoin/pull/21652#issuecomment-1657098200)
Looks like there is an intermittent issue, which is fixed in podman 4.1:

```
#135258 REDUCE cov: 2818 ft: 7924 corp: 483/10261b lim: 254 exec/s: 3468 rss: 251Mb L: 15/177 MS: 1 EraseBytes-
#135494 NEWError: timed out waiting for file /var/lib/containers/storage/overlay-containers/2b5173104c7716f28471c2aed46932cd57b0904326ef3b5e3c9c0462dad553a6/userdata/75976ef6638693018268a2b4187292027a833405bd50e9ab4a3ddccae789be0b/exit/2b5173104c7716f28471c2aed46932cd57b0904326ef3b5e3c9c0462dad553a6: inte
...

💬 fanquake commented on pull request "qa, doc: Fix comment":
(https://github.com/bitcoin/bitcoin/pull/28181#discussion_r1278542958)
```suggestion
Check that all python files in this directory are categorized
```

💬 MarcoFalke commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1657098924)
(Feel free to push the `chattr` changes here in the meantime. Obviously the CI won't pass, but people can start initial review)

💬 hebasto commented on pull request "qa, doc: Fix comment":
(https://github.com/bitcoin/bitcoin/pull/28181#discussion_r1278543285)
Done.

🚀 fanquake merged a pull request: "test: Add SyncWithValidationInterfaceQueue to mockscheduler RPC"
(https://github.com/bitcoin/bitcoin/pull/28118)

💬 ekzyis commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1657103010)
Concept ACK