💬 sr-gi commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1457621863)
Why is 32 here being passed in every tuple if it really is used as a constant?

💬 sr-gi commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1457652467)
Why is `garbage_terminators` being actively deleted here instead of letting the deletion be managed by the interpreter once we go out of context (and `peer` is deleted as a whole)?

💬 sr-gi commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1462241588)
Sorry, I meant right after peer2, referring to grouping the connections in the test by either inbound or outbound.

peer1 does v2 **from** v2
peer2 does v2 **from** v1
peer3 does v2 **to** v1
[...] the rest are all x **to** y
and then peer7 goes back to peer1 does v1 **from** v2

Certainly not a big deal though

💬 sr-gi commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1457654033)
I initially thought this had to do with:

> \# To achieve forward secrecy we must wipe the key material used to initialize the ciphers

But turns out we are not doing it for the key material.

💬 sr-gi commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1463495866)
I wonder if it'd be worth also checking that `len(self.recvbuf) > 15` to avoid calling `v2_handshake` when we potentially don't have enough data to decide whether this a v1 or v2 connection. This potentially will call `v2_handshake` 16 times, and process processing byte arrays of length from 0..16.

In practice, I don't think this matters, given the logic never triggers: this happens when the python node is the responder, and the Core node will never send a half-baked `magic_bytes + version` (
...

💬 sr-gi commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1457665569)
We could even get rid of this whole re-assignment and just set it in the latter if/else:

```
self.peer['send_garbage_terminator'] = peer['garbage_terminators'][:16]
self.peer['recv_garbage_terminator'] = peer['garbage_terminators'][16:]
```

💬 sr-gi commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1463533897)
Yes! This reads much clearer now :)

💬 maflcko commented on pull request "ci: Rename tasks (previous releases, macOS cross)":
(https://github.com/bitcoin/bitcoin/pull/29218#issuecomment-1906456732)
> Would it maybe make sense to have a wiki page with a reverse chronological list of known CI failures?

I think it would be easier to have each as an issue, which can be reverse chronologically sorted, or in any other way. There is a `Tests` label, but a new label can be added, if a dedicated "category" is needed. See, https://github.com/bitcoin/bitcoin/issues?q=is%3Aissue+is%3Aopen+label%3ATests

Using the issue tracker would also make it easy to search?

💬 maflcko commented on pull request "Make (Read/Write)BinaryFile work with char vector, use AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29229#discussion_r1463592487)
> For vector it ends up calling AutoFile::detail_fread(). It does not check whether ferror(3) occurred.

It does. If the return value of `detail_fread` is not `output.size()`, `operator>>` will fail.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1463600444)
i took the [text from the BIP](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki#cite_note-handshake_progress_19) here. On the python side, [`data_received()` asyncio function](https://docs.python.org/3/library/asyncio-protocol.html#asyncio.Protocol.data_received) only gets called when a bytes object is received.

(thinking about the c++ side)

💬 fanquake commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1463611946)
What is "kill harder"? Containers are already being `--all --force` removed below:
```bash
-a, --all Remove all containers
-f, --force Force removal of a running or unusable container
```
so they shouldn't need to be "stopped" beforehand?

Also, shouldn't this be `podman`?

💬 fanquake commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1463613779)
Also, why is this `sleep` here?

🚀 fanquake merged a pull request: "Add test for negative transaction version w/ CSV to tx_valid.json"
(https://github.com/bitcoin/bitcoin/pull/29291)

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1463620264)
`respond_v2_handshake()` is supposed keep reading bytes from `bitcoind` and shoot out ellswift bytes when first mismatch from 16 bytes `V1_PREFIX` happens. Let's say the mismatch happens after reading 3 bytes - we still need to store the 3 bytes read somewhere (currently stored in `self.received_prefix`) since it's actually the first 3 bytes of ellswift which `bitcoind` was sending us.

we need this 3 bytes later in `complete_handshake()` to read the remaining 64-3 bytes and to compute ellswif
...

💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1463636403)
An alternative (with podman 4.8+) would be to use `run --replace`, see https://github.com/containers/podman/commit/f21c1d238da393cb11c75e408a736336b979e6d5

👍 fanquake approved a pull request: "depends: Update libmultiprocess library to fix C++20 macos build error"
(https://github.com/bitcoin/bitcoin/pull/29276#pullrequestreview-1839389410)
ACK b8105b3ed7c97cd6545dba99d0e13c33f183e450.

✅ fanquake closed an issue: "build: multiprocess compile failure on macOS"
(https://github.com/bitcoin/bitcoin/issues/29248)

🚀 fanquake merged a pull request: "depends: Update libmultiprocess library to fix C++20 macos build error"
(https://github.com/bitcoin/bitcoin/pull/29276)

💬 luke-jr commented on pull request "init: handle empty settings file gracefully":
(https://github.com/bitcoin/bitcoin/pull/29144#issuecomment-1906571657)
Should we perhaps add (and recommend, for non-GUI users), a `-resetguisettings` for non-GUI?

⚠️ helpau opened an issue: "Huge disk fragmentation"
(https://github.com/bitcoin/bitcoin/issues/29296)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

very many fragmented files

### Expected behaviour

files are not fragmented

### Steps to reproduce

An extra description of the behaviour from #26063
Prerequisite: disc defragmented (I used Full optimization from UltraDefrag), Windows Security(including real-time protection) are disabled.
1. Start Bitcoin Core
2. Wait for half an hour
3. Stop Bitcoin Core
4. Run UltraDefrag report

...

✅ achow101 closed an issue: "Huge disk fragmentation"
(https://github.com/bitcoin/bitcoin/issues/29296)