👍 dergoegge approved a pull request: "fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read"
(https://github.com/bitcoin/bitcoin/pull/30273#pullrequestreview-2130830802)
Code review ACK 4d81b4de339efbbb68c9785203b699e6e12ecd83

💬 dergoegge commented on pull request "fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read":
(https://github.com/bitcoin/bitcoin/pull/30273#discussion_r1647812026)
Unrelated to this PR but manual timeouts in a fuzz test are a bad idea. If a target requires the use of `GetTime` (or similar) then the harness needs to set mock time (as the test is otherwise non-deterministic).

We should get rid of this and replace it with calls to `SetMockTime` (we might need to make mock time more granular, i.e. currently it only supports seconds iirc).

💬 Sjors commented on pull request "Stratum v2 Transport":
(https://github.com/bitcoin/bitcoin/pull/30315#issuecomment-2181051477)
Would it make sense to add a `libbitcoin_net` library? Either at the same level as `libbitcoin_common` or on top of it. I could see how other node implementation could benefit from that, but it's also just a way to limit `libbitcoin_common` in size. Afaik only `libbitcoin_node` needs networking (and `zmq`?), and a future `libbitcoin_sv2`, and maybe `libbitcoin_rpc`.

💬 Sjors commented on pull request "Stratum v2 Transport":
(https://github.com/bitcoin/bitcoin/pull/30315#discussion_r1647820313)
d787c61afefccd73fe9c3d86f84b18b45d331bf2: @sipa any thoughts on how to best do this?

💬 Sjors commented on pull request "Stratum v2 Transport":
(https://github.com/bitcoin/bitcoin/pull/30315#discussion_r1647822168)
d787c61afefccd73fe9c3d86f84b18b45d331bf2: TODO: shoehorn our `Sv2NetMsg` into a dummy `CNetMessage`.

💬 Sjors commented on pull request "Stratum v2 Transport":
(https://github.com/bitcoin/bitcoin/pull/30315#discussion_r1647825951)
d787c61afefccd73fe9c3d86f84b18b45d331bf2: I don't remember why I decided on a custom implementation here.

💬 Sjors commented on issue "Make Transport independent of CNetMessage and CSerializedNetMsg":
(https://github.com/bitcoin/bitcoin/issues/30209#issuecomment-2181070090)
WIP in #30315

📝 theStack opened a pull request: "Show maximum mempool size in information window"
(https://github.com/bitcoin-core/gui/pull/825)
This PR adds the maximum mempool size to the information window (Menu "Window" -> "Information" -> section "Memory Pool" -> line "Memory usage").

master:


PR:


💬 Sjors commented on pull request "doc: clarify Cirrus self-hosted workers setup":
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1647847594)
I don't know if it works out of the box for every distro out there. It does for Ubuntu 24.04 when used with podman, at least after a reboot. I haven't tested with docker nor on any other distro. The documentation for https://github.com/multiarch/qemu-user-static says you need to run `sudo podman run --rm --privileged multiarch/qemu-user-static --reset -p yes` first, but this is no longer true.

What I'm trying to convey with "may work" is: it might work with just `sudo apt install qemu-user-st
...

💬 Sjors commented on pull request "doc: clarify Cirrus self-hosted workers setup":
(https://github.com/bitcoin/bitcoin/pull/30314#issuecomment-2181104066)
Fixed @maflcko's nits.

💬 Sjors commented on pull request "doc: clarify Cirrus self-hosted workers setup":
(https://github.com/bitcoin/bitcoin/pull/30314#discussion_r1647851021)
This uncertainty is also part of the reason #29274 adds `NO_ARM` as an option.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647853364)
What I remember from a while ago, but I haven't retested, is that under a pull request all the checks initially are marked as skipped. But then they show as either finished or failed as the results come in. It was rather flaky.

I could try again and make some screenshots...

💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2181120807)
Rebased

💬 m3dwards commented on pull request "ci: add option for running tests without volume":
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2181122781)
> Oh, I guess only master creates the cache?

Branches on forks work, see cache restore here: https://github.com/m3dwards/bitcoin/actions/runs/9601151359/job/26479048839

👋 m3dwards's pull request is ready for review: "ci: add option for running tests without volume"
(https://github.com/bitcoin/bitcoin/pull/30310)

💬 m3dwards commented on pull request "ci: add option for running tests without volume":
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2181126299)
Nits should be addresses @maflcko

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647869054)
Glad I checked this, because the glitched described here is no longer happening:
https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-1967030916

💬 m3dwards commented on pull request "ci: add option for running tests without volume":
(https://github.com/bitcoin/bitcoin/pull/30310#issuecomment-2181148450)
This job has a ccache size limit of 100mb, the MacOS job has a ccache limit of 400m. @hebasto is there something specific to MacOS for why the ccache limit is higher there?

💬 ryanofsky commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647880221)
> I could try again and make some screenshots...

Thanks, I wouldn't make an extra effort to understand the broken behavior you were trying to avoid. But maybe consider dropping the word "initially" because I think the comment would make more sense without it. The idea of not avoiding "skip" directive to avoid tests showing up as "skipped" is fairly clear. It's just not clear why they tests might initially show up as skipped but then later show up not skipped, if that's what happens.

It mi
...

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2181152033)
Simplified c02d96f8f5829d433f4a061480aa37ee0c6ff076 because the UI glitch I noticed by in February no longer happens. See https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647869054

Rebased on #30314, so will temporarily mark this PR as draft.