💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647316744)
Adding `cirrus1` to `/etc/sudoers` and running both commands with `sudo` works. But running only the `--reset` command with `sudo` is not enough.

Not really a solution, but at least I can run the CI job once to see what the performance is like.

💬 vasild commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1647331068)
I think I see your point, you prefer 1. over 2., assuming we got all the bytes we asked for from a read, then:

1. Ignore possible IO error by not checking for it
2. Check for an error and behave as if the read failed

I dropped this change.

💬 ismaelsadeeq commented on pull request "Fee Estimation: change `estimatesmartfee` default mode to `economical`":
(https://github.com/bitcoin/bitcoin/pull/30275#discussion_r1647332466)
I now understand thanks for clarifying.
yes it does it will even be better to document the short description of both modes and indicate `economical` is the default ?

💬 vasild commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1647333555)
> if (m_was_written) Assume(IsNull());

Done.

💬 vasild commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1647335718)
Resolving this discussion. Changed the semantic of `AutoFile` to require all write users to call `fclose()`.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647348857)
Uhhh ... so I did a reboot. And now `podman run --rm -ti "docker.io/arm64v8/debian:bookworm" uname --machine` just works, doesn't need the reset command.

👍 vasild approved a pull request: "init: fixes file descriptor accounting"
(https://github.com/bitcoin/bitcoin/pull/30065#pullrequestreview-2130057647)
ACK fe76929d4d68a4ca6d27c58c1e243a64fdc70b2a

💬 m3dwards commented on pull request "ci: add option for running tests without volume":
(https://github.com/bitcoin/bitcoin/pull/30310#discussion_r1647363601)
The script makes sure the directories exist to mount into the container:

`mkdir -p "${DEPENDS_DIR}/built"`

This directory has to be in a location that GHA allows you to create a directory such as ${RUNNER_TEMP}. The default specified in `00_setup_env.sh` would be `/ci_container_base/depends`.

So even though this specific job doesn't use depends this was done to make sure the container could mount all of the directories.

I will look at solving this in a less confusing way, perhaps jus
...

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647365952)
If the job finishes in reasonable time, I'll add something like this to the documentation: https://github.com/Sjors/bitcoin/commit/d065dc11fb862f48fc88e674a95b7daf68269ca0

It seems that this either works trivially or is really hard to debug, so I prefer to keep `NO_ARM=1` around as an alternative.

💬 dergoegge commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1647358794)
Is clearing the filters is necessary in IBD?

💬 dergoegge commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1645820087)
Why is UpdatedBlockTipSync called but not UpdatedBlockTip?

🤔 dergoegge reviewed a pull request: "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip"
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2127714476)
> This is a synchronous version of UpdatedBlockTip.

Given that this is the goal, it's a little weird that `UpdatedBlockTipSync` has a different signature and is called in different locations.


If it wasn't for the zmq notifier we could probably just make `UpdateBlockTip` synchronous instead of having two versions.

💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2180376101)
fa660a266b3aa03f40c47a9330e3bce8be89bb54 -> f946b083fe1431fb8d5d2c080ed3c2938116c37a ([noGlobalScriptCache_5](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_5) -> [noGlobalScriptCache_6](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_5..noGlobalScriptCache_6))

* Addressed @maflcko's [nit](https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1646381635), removing unnecessa
...

💬 m3dwards commented on issue "ci: Move more tasks to GHA?":
(https://github.com/bitcoin/bitcoin/issues/30304#issuecomment-2180498915)
One nice aspect of GHA is the dev experience for contributors means they can easily have CI run on their personal forks before submitting a PR upstream. Yes it should be possible for someone to self host a runner but realistically how many would?

Flocking to free provider could leave the project vulnerable to a bait and switch and perhaps a bit of vendor lock in but it also would have a democratising effect; putting CI in the hands of any fork by default.

🤔 maflcko reviewed a pull request: "util: explicitly close all AutoFiles that have been written"
(https://github.com/bitcoin/bitcoin/pull/29307#pullrequestreview-2130240983)
lgtm ACK 11be9f4103ea219f801a1f0fe1385f66ca70ad22

Will do some testing later

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647474063)
1 hour 42 minutes, but that was without using any cache.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2180529210)
I turns out it's fairly straight-forward to run the arm native job on an x86_64 machine using `qemu-user-static` (ht @maflcko). I added a note on how to do that. But I'm keeping 247b5823b2d5810ac9ddb8c8562232ff3eeb0344 around as an alternative.

🤔 marcofleon reviewed a pull request: "fuzz: Improve stability for txorphan and mini_miner harnesses"
(https://github.com/bitcoin/bitcoin/pull/30306#pullrequestreview-2130271741)
Tested ACK e009bf681c0e38a6451afa594ba3c7c8861f61c3. Using afl++, stability for `txorphan` went from 82% to ~94% and for `mini_miner` it went from 84% to 97%. I ran them both using the corpora in qa-assets.

💬 Sjors commented on issue "ci: Move more tasks to GHA?":
(https://github.com/bitcoin/bitcoin/issues/30304#issuecomment-2180535361)
> the only practical need I have currently is to run the native ARM job on Github CI

Which I've now solved with the magic power of `qemu-user-static`.

💬 murchandamus commented on pull request "fuzz: have package_rbf always make small txns":
(https://github.com/bitcoin/bitcoin/pull/30300#issuecomment-2180538394)
This change in the fuzzer seems to have reduced the coverage of the `package_rbf` fuzzer with the current `qa-assets` fuzz inputs by about 600 LOC. I’ll make a PR to update the fuzz inputs.