💬 hodlinator commented on pull request "fees: Remove CLIENT_VERSION serialization":
(https://github.com/bitcoin/bitcoin/pull/29702#discussion_r1802081828)
nit: Could improve adjacent line:
```suggestion
throw std::runtime_error(strprintf("File version (%d) indicates it was generated by a newer version of the software.", nVersionRequired));
```

💬 willcl-ark commented on pull request "fees: document non-monotonic estimation edge case":
(https://github.com/bitcoin/bitcoin/pull/31080#discussion_r1802585830)
OK pushed that in [372ca14](https://github.com/bitcoin/bitcoin/pull/31080/commits/372ca14fbe0e716c789b74b766a8532f0f174268)

✅ Sjors closed a pull request: "ci: skip Github CI on branch pushes for forks"
(https://github.com/bitcoin/bitcoin/pull/30487)

💬 Sjors commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2416092451)
A use case that mentioned to me offline is that some people, before opening a PR to the main repo, like to push their branch and have CI check it. They then wait for CI on the branch to pass before opening a new PR. Their main reason for this is to reduce noise on the main repo.

It seems this PR breaks that use case. I suggest we reconsider it if and when Github adds support for custom ENV variables on a fork, like Cirrus does.

👍 laanwj approved a pull request: "test: Print CompletedProcess object on error"
(https://github.com/bitcoin/bitcoin/pull/31067#pullrequestreview-2371619568)
for reference, a CompletedProcess object is formatted like this:
```python
>>> print(subprocess.run(['/bin/ls'], capture_output=True, text=True))
CompletedProcess(args=['/bin/ls'], returncode=0, stdout='...', stderr='')
```
This contains some useful information, so ACK fa43c4f93ca5b40734ec9b3ff91b74acf3ed7cf2

🤔 laanwj reviewed a pull request: "Add Signet and testnet4 launch shortcuts for Windows"
(https://github.com/bitcoin/bitcoin/pull/26334#pullrequestreview-2371649358)
ACK cfd03de965a081facbd72316c76603dd7aa511bd

👍 laanwj approved a pull request: "Add Signet and testnet4 launch shortcuts for Windows"
(https://github.com/bitcoin/bitcoin/pull/26334#pullrequestreview-2371650372)
ACK cfd03de965a081facbd72316c76603dd7aa511bd

💬 willcl-ark commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2416121958)
From https://github.com/bitcoin/bitcoin/commit/e9bfbb5414ab14ca14d8edcfdf77f28c9ed67c33:

> When a contributor maintains a fork of the repo, any pull request they make
> to their own fork, or to the main repository, will trigger two CI runs:
> one for the branch push and one for the pull request.
> This can be avoided by setting SKIP_BRANCH_PUSH=true as a custom env variable
> in Cirrus repository settings, accessible from

GitHub supports setting custom env vars in each repo like Cirrus
...

✅ Sjors closed a pull request: "Stratum v2 Noise Protocol"
(https://github.com/bitcoin/bitcoin/pull/29346)

💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2416132740)
I moved this to https://github.com/Sjors/bitcoin/pull/66 so we can focus on building an interface for external applications to use, and getting multiprocess in a release. I plan to open a tracking issue for that.

The followup PRs #30315 and #29432 will also be moved shortly.

@stratospher I will apply your feedback on the moved PR later.

✅ Sjors closed a pull request: "Stratum v2 Transport"
(https://github.com/bitcoin/bitcoin/pull/30315)

💬 Sjors commented on pull request "Stratum v2 Transport":
(https://github.com/bitcoin/bitcoin/pull/30315#issuecomment-2416152718)
I moved this to https://github.com/Sjors/bitcoin/pull/67 so we can focus on building an interface for external applications to use, and getting multiprocess in a release.

💬 laanwj commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#discussion_r1802670686)
Maybe factor this shovel-from-socketa-to-socketb loop out to a function in netutil as well.

💬 laanwj commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#discussion_r1802683914)
i don't think `sendall` can be guaranteed with non-blocking sockets, it will fail if it gets EAGAIN
https://stackoverflow.com/questions/6240737/python-socket-sendall-function

💬 instagibbs commented on issue "Why does `submitpackage` require at least two transactions":
(https://github.com/bitcoin/bitcoin/issues/31085#issuecomment-2416189977)
I think it's reasonable to support. I'll take a look.

💬 laanwj commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1802716839)
nit: please put multiple statements on multiple lines, this is pretty hard to read code as-is :sweat_smile:

💬 laanwj commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#issuecomment-2416224594)
ACK on overall change, small nit

📝 instagibbs opened a pull request: "Package validation: accept packages of size 1"
(https://github.com/bitcoin/bitcoin/pull/31096)
There's no particular reason to restrict single transaction submissions with submitpackage. This change relaxes the RPC checks as well as the `IsChildWith*` checks to allow these cases, as well as return false rather than segfault with an empty package.

Resolves #31085

💬 instagibbs commented on issue "Why does `submitpackage` require at least two transactions":
(https://github.com/bitcoin/bitcoin/issues/31085#issuecomment-2416273464)
Opened #31096

💬 TheCharlatan commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2416279768)
Cobcept ACK