💬 fanquake commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2189167818)
> Maintainer note: NO_BRANCH=true must be set in Cirrus for bitcoin-core/gui before merging this. See https://cirrus-ci.com/github/bitcoin-core/gui -> Settings.

Can you rename this to something that actually explains what it's doing; `NO_BRANCH` is incredibly vauge. Why not `DONT_RUN_CI_ON_BRANCH_PUSH_ON_THIS_FORK` etc? Then someone looking at the settings in Cirrus will actually know what this is doing.

💬 fanquake commented on pull request "depends: build libevent with CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#discussion_r1653002407)
Probably. Happy to just drop this, and it could always be added later.

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2189207922)
> a highly specialized process (I think we're only expecting a small minority of nodes to be running SV2?)

I'd like to see a lot more users mine, and do so with their own block template. In order to achieve that we have to lower the barrier to entry.

There's a generic case to be made for reducing the scope of Bitcoin Core, and not further increasing it. I think we still lack the tooling for this. Something like a plugin system, or a convenient way to ship a release tag plus custom function
...

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2189223926)
Renamed `NO_BRANCH` to `SKIP_BRANCH_PUSH`.

👍 stickies-v approved a pull request: "contrib: Fixup verify-binaries OS platform parsing"
(https://github.com/bitcoin/bitcoin/pull/30147#pullrequestreview-2138611736)
ACK 4a85b154d0874f7bfa848ae8d7ab341eea754607 - nice work

💬 stickies-v commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1652784235)
meta nit: although consistency is good, changing otherwise unaffected lines/functions just to do an unnecessary rename can be counterproductive, so this would have been fine to keep as is

👍 vasild approved a pull request: "net: Allow -proxy=[::1] on nodes with IPV6 lo only"
(https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2139044652)
ACK 60a753b7b38fbe89494f8df66f13a84f28af244b

Thank you!

✅ fanquake closed a pull request: "util/system: Close non-std fds when execing slave processes"
(https://github.com/bitcoin/bitcoin/pull/22417)

💬 fanquake commented on pull request "util/system: Close non-std fds when execing slave processes":
(https://github.com/bitcoin/bitcoin/pull/22417#issuecomment-2189291497)
Closing, see above, and this is adding Boost Process code, which no-longer exists in this codebase.

💬 vasild commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-2189293850)
`6d9083b249...45f4dbe484`: rebase due to conflicts

📝 fanquake converted_to_draft a pull request: "fuzz: wallet, add target for Spend"
(https://github.com/bitcoin/bitcoin/pull/28236)
This PR adds fuzz coverage for `wallet/spend`.

Motivation: Issue [27272](https://github.com/bitcoin/bitcoin/issues/27272#issue-1628327906)

This PR adds Fuzz coverage to the whole concept of Creating a New Transaction as well as other sections of the Spend file. Because `CreateTransaction` is one of the most frequently used functions in the wallet codebase, merging this PR will significantly improve the wallet codebase's Fuzz testing!

I also used the `Singleton Class` concept for creati
...

💬 fanquake commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-2189297229)
> I've the approach in mind and will force push the update by the end of this month.

Moved to draft for now.

📝 fanquake converted_to_draft a pull request: "rpc: show P2(W)SH redeemScript in getrawtransaction #27637"
(https://github.com/bitcoin/bitcoin/pull/27638)
This a work in progress PR. I'll squash all my commits at the end to make commits atomic (as mentioned [here](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#committing-patches))
It's an effort to solve this issue https://github.com/bitcoin/bitcoin/issues/27391.



<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
h
...

💬 fanquake commented on pull request "Bugfix: Correct first-run free space checks":
(https://github.com/bitcoin/bitcoin/pull/29678#issuecomment-2189303456)
@hebasto can you follow up given the gui / translation Qs here. This also needs a rebase.

📝 fanquake converted_to_draft a pull request: "lint/contrib/doc updates"
(https://github.com/bitcoin/bitcoin/pull/30084)

💬 fanquake commented on pull request "lint/contrib/doc updates":
(https://github.com/bitcoin/bitcoin/pull/30084#issuecomment-2189305171)
Moved to draft for now, given it's not clear what the status of this is.

📝 fanquake converted_to_draft a pull request: "Enhanced error messages for invalid network prefix during address parsing."
(https://github.com/bitcoin/bitcoin/pull/27260)
### **Issue:**

Simply `DecodeDestination` does not handle errors where the user inputs a valid address for the wrong network. _e.x. testnet while running client on mainnet_

The current error `not a valid Bech32 or Base58 encoding` for a valid address on a different network is entirely incorrect. This is because of the is_bech32 variable used at the core of `DecodeDestination` logic only checks that the prefix matches. If it doesn't it just starts running everything as `DecodeBase58Check
...

💬 fanquake commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#issuecomment-2189309433)
Moved to draft for now, given at least one outstanding review comment and needs a rebase.

✅ fanquake closed a pull request: "doc: Add link to ccache docs for more description"
(https://github.com/bitcoin/bitcoin/pull/29604)

💬 fanquake commented on pull request "doc: Add link to ccache docs for more description":
(https://github.com/bitcoin/bitcoin/pull/29604#issuecomment-2189316076)
Thanks, however I think we'll leave this as-is.