Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 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.
💬 fjahr commented on issue "RFC: Assumeutxo and large forks and reorgs":
(https://github.com/bitcoin/bitcoin/issues/30288#issuecomment-2180558022)
I first tried to approach this question from a users view and which action would make more sense for them. But I think there are many possible scenarios and depending on the mindset of the user one or the other action may make sense to them.

> Considering that AssumeUtxo sync is meant to be an optional and temporary optimization, and that large reorgs should be very infrequent, it could also make sense to abandon the AssumeUtxo sync, delete the snapshot chainstate and revert to normal sync as
...
📝 fanquake opened a pull request: "contrib: add R(UN)PATH check to ELF symbol-check"
(https://github.com/bitcoin/bitcoin/pull/30312)
Our binaries shouldn't contains any rpaths, or runpaths, so check that at release time.

Guix build (aarch64):
```bash
14f1b54936f71aaf8fedb987e1c2f5642c34ac35f4856cdbd7bf7b4a9f42507c guix-build-4289dd02cce6/output/aarch64-linux-gnu/SHA256SUMS.part
b120503ac4a37c160aa1bdc662348a2a662cb9b3d0477daa177e92afacab6a27 guix-build-4289dd02cce6/output/aarch64-linux-gnu/bitcoin-4289dd02cce6-aarch64-linux-gnu-debug.tar.gz
6416e3678aa13301ba2327e65dcd83afefd15d21c96c1b574cf616a65466a260 guix-build-
...
💬 maflcko commented on pull request "ci: add option for running tests without volume":
(https://github.com/bitcoin/bitcoin/pull/30310#discussion_r1647523847)
nit: no need to document this. For `DANGER_` options, the user already fully understand what they are doing (brick their system), so no need to hold their hands or otherwise encourage it.
💬 maflcko commented on pull request "fuzz: have package_rbf always make small txns":
(https://github.com/bitcoin/bitcoin/pull/30300#issuecomment-2180601279)
Yes, this is expected, because all lines of code in `ConsumeDeserializable<CMutableTransaction>` can no longer be hit. Also, the fuzz input format changed, but that should fix itself over time, as new fuzz inputs are found.