💬 m3dwards commented on pull request "ci: add option for running tests without volume":
(https://github.com/bitcoin/bitcoin/pull/30310#discussion_r1647590152)
`RUNNER_TEMP` was used over `${{ github.workspace }}` (as is used in the MacOS job) because the CI container mounts that as readonly which means it can't be used to write cache items to.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2180745584)
`057c79365c...7a7e7d189d`: rebase due to conflicts

💬 vasild commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#issuecomment-2180755507)
Has two ACKs, @TheCharlatan, maybe you want to take a look at this since you :+1:'ed the OP.

🤔 maflcko reviewed a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2128595258)
lgtm ACK f946b083fe1431fb8d5d2c080ed3c2938116c37a 📪

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: lgtm ACK f946b083fe1431fb
...

💬 maflcko commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1647123546)
nit in 594189567956063fa1a44bef9e86e72c14207027:

```
script/sigcache.h should remove these lines:
- #include <cstdint> // lines 18-18
- #include <utility> // lines 20-20

💬 maflcko commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1647518394)
nit in /f946b083fe1431fb8d5d2c080ed3c2938116c37a: Why use a pointer when a reference is better?

💬 maflcko commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1647514586)
nit in /f946b083fe1431fb8d5d2c080ed3c2938116c37a: Unrelated change? The commit message is "De-globalize signature cache", but this hunk makes the script execution cache private.

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2180775603)
I split the commit that introduces `sv2_messages.h` in two parts:

* 9fcc95d837dbd20c1003b4c2d3c90b202e0629bd: introduces a single simple message `Sv2CoinbaseOutputDataSizeMsg` and the plumbing it needs
* 00d1408cc7cb263fb6e956649a41e0b02e2378aa: adds the other messages

This makes it easier to introduce `Sv2Transport` in a separate PR, which only needs the first commit.

💬 kevkevinpal commented on pull request "ci: add option for running tests without volume":
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2180781920)
Is there a reason why we don't want to just have cirrus behave in the same way we'd do it on GHA, seems like GHA cannot store to shared cache but cirrus can act the same was as GHA. Just thinking maybe theres a way to reduce complexity in `ci/test/02_run_container.sh` and have more shared logic between the two ci's

if I were to set `DANGER_CI_ON_HOST_CACHE_FOLDERS = 1` on the cirrus pipeline would the ci fail?

📝 guilhermelinosp opened a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/30313)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647632350)
58 minutes for the second run, which is acceptable.

✅ achow101 closed a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/30313)

📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/30313)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

📝 Sjors opened a pull request: "ci: clarify Cirrus self-hosted workers setup"
(https://github.com/bitcoin/bitcoin/pull/30314)
Taken from #29274 (except for two paragraphs that require the other commits in that PR).

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2180800535)
@maflcko done in #30314, minus the paragraphs about `NO_ARM` and `NO_BRANCH`.

💬 ryanofsky commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647664881)
In commit "ci: forks can opt-out of CI branch push (Cirrus only)" (1c35e3c47fe814c402d423e247b26cb6da2e960e)

I'm still confused by this. I understand we don't want PR contributors to "initially see all jobs marked as skipped". But what does this cause them to see? Jobs just not showing up at all? Also I don't understand why it says "initially." Would the jobs be only initially skipped and not permanently skipped for some reason?

💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1647665176)
I think we similarly make `CTransaction` a pointer to keep the `CScriptCheck` class `is_nothrow_move_assignable_v`.

🤔 maflcko reviewed a pull request: "doc: clarify Cirrus self-hosted workers setup"
(https://github.com/bitcoin/bitcoin/pull/30314#pullrequestreview-2130605205)
ACK 9f4255ac57929de985425f26ad904a12176a0e85 🌂

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 9f4255ac57929de985425f26ad
...

💬 maflcko commented on pull request "doc: clarify Cirrus self-hosted workers setup":
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1647677834)
```suggestion
# The above machine types are matched to each task by their label. Refer to the
```

Cirrus calls it `task`. In yaml it is `task:`

💬 maflcko commented on pull request "doc: clarify Cirrus self-hosted workers setup":
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1647683866)
nit: empty newline should stay here, because the section ends. you'll have to remove the `#` above instead.