💬 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.

💬 BenWestgate commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1653094080)
It was a "change all occurrences" accident. `version_os` was local to `parse_version_string`, so I logically assumed `version_rc` would be too. So I blame the original variable names for this goof.
I didn't see until after I had force pushed.

Shall I restrict the `version_rc` to `rc` rename to only within the `parse_version_string` function? Or not worth changing back?

📝 fanquake opened a pull request: "depends: update doc in Qt pwd patch"
(https://github.com/bitcoin/bitcoin/pull/30336)
Now that upstream has gotten around to fixing this. We don't need any more of the patch, and it likely wont apply to our version of Qt in any case. See: https://github.com/qt/qtbase/commit/3388de698bfb9bbc456c08f03e83bf3e749df35c.

📝 nnsW3 opened a pull request: "Docs improvements"
(https://github.com/bitcoin/bitcoin/pull/30337)
Rectify typographical inaccuracies

This PR addresses several typographical errors across various files in the project. The changes improve readability and maintain the professional standard of the documentation and code comments.

Justification
Typographical errors, while minor, can detract from the overall quality of the project. Correcting these errors ensures clarity and professionalism, making the project more accessible and understandable for current and future contributors.

💬 TheCharlatan commented on pull request "build: use `-no_exported_symbols` on macOS":
(https://github.com/bitcoin/bitcoin/pull/29072#issuecomment-2189380466)
Guix build (aarch64):
```
544f33855573c49ff642be39c54603c9afbf64076e6575cb19c5a64fea1522e4 guix-build-81d4dc8e8739/output/aarch64-linux-gnu/SHA256SUMS.part
a8a3c29fe1a2a21f677cfbd8fadb1d65f5901b6f133531f6fce736f1e1e163e5 guix-build-81d4dc8e8739/output/aarch64-linux-gnu/bitcoin-81d4dc8e8739-aarch64-linux-gnu-debug.tar.gz
d059d19cb255b3f555eb2be6b89e0d000a9476278aa783efdc1ba0c3faac6236 guix-build-81d4dc8e8739/output/aarch64-linux-gnu/bitcoin-81d4dc8e8739-aarch64-linux-gnu.tar.gz
1ab4dac0ea
...

💬 TheCharlatan commented on pull request "depends: build libevent with CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#discussion_r1653136723)
That's fine too, it is not used anyway.

💬 stickies-v commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1653151031)
I'd prefer limiting it to `parse_version_string` and will quickly re-ack if you force push

💬 jonatack commented on pull request "Docs improvements":
(https://github.com/bitcoin/bitcoin/pull/30337#issuecomment-2189405730)
Hi @nnsW3, several of these proposed changes are incorrect. I suggest reading through https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md and https://jonatack.github.io/articles/how-to-contribute-pull-requests-to-bitcoin-core for an idea of contributions that would be useful to this project and valued. Cheers.

💬 BenWestgate commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1653163359)
Done in https://github.com/bitcoin/bitcoin/compare/4a85b154d0874f7bfa848ae8d7ab341eea754607..3ab25201909bece9066ac6191670bcee09791d54

👍 stickies-v approved a pull request: "contrib: Fixup verify-binaries OS platform parsing"
(https://github.com/bitcoin/bitcoin/pull/30147#pullrequestreview-2139226506)
re-ACK 3ab25201909bece9066ac6191670bcee09791d54

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

I just now went to that URL and set this. There were not any other environment variables set.

I wonder if there is a good place to document this configuration, but probably it's not too important as the main effect is just to avoiding wastefully running CI twice on pushes to master (at least according to the original P
...

💬 Sjors commented on pull request "util/system: Close non-std fds when execing slave processes":
(https://github.com/bitcoin/bitcoin/pull/22417#issuecomment-2189525056)
> What is the status of this now that Boost Process is removed

For historical reference: this patch is no longer relevant, since #28981 replaced boost-process with cpp-subprocess.

@luke-jr does `cpp-subprocess` do anything similar that needs addressing? https://github.com/bitcoin/bitcoin/blob/master/src/util/subprocess.h