💬 MarcoFalke 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-1683390638)
`Error: RPC command "sendmsgtopeer" not found in RPC_COMMANDS_SAFE_FOR_FUZZING or RPC_COMMANDS_NOT_SAFE_FOR_FUZZING. Please update test/fuzz/rpc.cpp.`

💬 MarcoFalke commented on pull request "rpc: remove one more quote from non-string oneline description":
(https://github.com/bitcoin/bitcoin/pull/28289#issuecomment-1683391746)
lgtm ACK 239431444216850b63ecf01c3b5c5d6d24230d08

✅ MarcoFalke closed a pull request: "ci: Disable --coverage temporarily"
(https://github.com/bitcoin/bitcoin/pull/28285)

💬 MarcoFalke commented on pull request "rpc: removed StrFormatInternalBug quote delimitation":
(https://github.com/bitcoin/bitcoin/pull/28291#issuecomment-1683400075)
review ACK 6e8f6468cbf1320b70cf01333002a31b44cb7c33

Makes sense to use a single function consistently. This will now also print `file, line, func` of where the *check* is located, not where the presumed *bug* is located, but this seems fine, because the bug could also be in the check. In theory, there could be a flag to disable `file, line, func`, but that seems over-optimizing for developer-only code paths that are rarely hit.

📝 hebasto opened a pull request: "ci: Disable cache save for pull requests in GitHub Actions"
(https://github.com/bitcoin/bitcoin/pull/28292)
This PR disable cache save for pull requests in GitHub Actions.

Otherwise, multiple pull requests fill GitHub Actions cache quota shortly.

See a discussion [here](https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1295459732).

---

**NOTE** for the maintainers with "owner" permissions.

This PR needs the `actions/cache/restore@*` and `actions/cache/save@*` acrions to be explicitly allowed in the repository's Actions permissions.

💬 MarcoFalke commented on issue "Intermittent failure in feature_config_args.py":
(https://github.com/bitcoin/bitcoin/issues/28290#issuecomment-1683415295)
I also see this locally on valgrind.

I think the general idea is that we can't measure wall clock time and assume some operation is *faster* than a specified wall clock duration (60 seconds in this case).

Not sure what to do here, other than removing this check?

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298058749)
why does this specify both keys?

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298058494)
Shouldn't the key be just the task name? There should be no reason to bloat the cache directory with entries that won't ever be used again.

See https://github.com/bitcoin/bitcoin/actions/caches

🤔 MarcoFalke reviewed a pull request: "ci: Disable cache save for pull requests in GitHub Actions"
(https://github.com/bitcoin/bitcoin/pull/28292#pullrequestreview-1583930912)
concept ACK

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298060542)
Maybe also disable it for branches other than `master`, to make sure that storage is constant, even if there was an intermittent push to a backport branch?

⚠️ mrnadyy opened an issue: " mrmohamednadi@gmail.com"
(https://github.com/bitcoin/bitcoin/issues/28293)

💬 hebasto commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298063957)
While we are interested in the `restore-keys` input only, the `key` input is [required](https://github.com/actions/cache/blob/f7ebb81a3f195b4fb88dab7c14e2f7aff52045aa/restore/action.yml), thus, it cannot be avoided.

💬 MarcoFalke commented on pull request "lint: fix custom mypy cache dir setting":
(https://github.com/bitcoin/bitcoin/pull/28184#discussion_r1298064385)
```suggestion
base_dir = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
```

The two should be the same, no? Seems better to just use one way to get one thing, instead of two.

✅ MarcoFalke closed an issue: " mrmohamednadi@gmail.com"
(https://github.com/bitcoin/bitcoin/issues/28293)

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298066081)
ok, maybe use the same value for both, or is there a reason to use different values?

💬 hebasto commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298067481)
GitHub Actions caches are immutable. Therefore, [this](https://github.com/actions/cache/blob/main/tips-and-workarounds.md#update-a-cache) pattern has been used for Ccache caches.

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298068770)
ugh, fair enough

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298069742)
ok, nvm. Looks like caches are immutable anyway, so this can't be achieved, I guess.

👍 MarcoFalke approved a pull request: "ci: Disable cache save for pull requests in GitHub Actions"
(https://github.com/bitcoin/bitcoin/pull/28292#pullrequestreview-1583948672)
lgtm ACK

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298070445)
```suggestion
key: ${{ github.job }}-ccache-${{ github.run_id }} # https://github.com/actions/cache/blob/main/tips-and-workarounds.md#update-a-cache
```

💬 MarcoFalke commented on pull request "ci: Disable cache save for pull requests in GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28292#discussion_r1298072161)
Maybe it is still better to only have caches for a single branch, as they can't be re-used for other branches anyway?