👍 hebasto approved a pull request: "ci: Fix lint runner selection (and docker cache)"
(https://github.com/bitcoin/bitcoin/pull/33744#pullrequestreview-3412060705)
ACK 7632e0ba312a372259897c68fd7c7eb723df3738.

💬 hebasto commented on pull request "ci: Fix lint runner selection (and docker cache)":
(https://github.com/bitcoin/bitcoin/pull/33744#discussion_r2487242811)
The GHA [docs](https://docs.github.com/en/actions/reference/workflows-and-actions/metadata-syntax) do not mention `inputs.<input_id>.options` at all.

👍 hebasto approved a pull request: "random: scope environ extern to macOS, BSDs and Illumos"
(https://github.com/bitcoin/bitcoin/pull/33714#pullrequestreview-3412082427)
re-ACK 79d6f458e2300e1f47b94467cda233e1c761f8be.

💬 willcl-ark commented on pull request "ci: Fix lint runner selection (and docker cache)":
(https://github.com/bitcoin/bitcoin/pull/33744#discussion_r2487261226)
Yep, I think the original implementation got wires crossed with workflow dispatch inputs https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#example-of-onworkflow_dispatchinputs

💬 yancyribbens commented on pull request "docs: add doc comment for SRD selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/33622#discussion_r2487266295)
This edit is a tautology. No matter what the change budgeting is, the change amount will be at least `CHANGE_LOWER` since this value is added to `CHANGE_LOWER`. Maybe it's not the place to explain to users of the API _why_ it matters that `CHANGE_LOWER` may not be enough of a change budget, although I would have no idea why without reading the commit that added this parameter.

💬 hebasto commented on issue "Async Payjoin":
(https://github.com/bitcoin/bitcoin/issues/33684#issuecomment-3481668679)
> Given that this is adding new features to the GUI. Is the plan to add them to the "legacy" GUI, the new QML GUI, or implement Payjoin into both? cc [@hebasto](https://github.com/hebasto)

cc @GBKS @johnny9

💬 fanquake commented on pull request "feat: shell completions for bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/33747#issuecomment-3481668738)
Thanks. However aside from the fish addition, this duplicates works already being done in #33385 and #33402.

💬 purpleKarrot commented on pull request "refactor: C++20 operators":
(https://github.com/bitcoin/bitcoin/pull/33771#discussion_r2487296852)
Thanks @stickies-v.

I am not convinced about `std::strong_ordering`.
`constexpr` will definitely be added as a follow up, together with `= default` where appropriate.

💬 fanquake commented on pull request "depends: disable variables, rules and suffixes.":
(https://github.com/bitcoin/bitcoin/pull/33045#issuecomment-3481696397)
Guix Build (aarch64):
```bash
e0edc03339b5b32705b8eacbc213a6fd6e5b787caba574671bb3c18340a141ec guix-build-52b1595850f6/output/aarch64-linux-gnu/SHA256SUMS.part
55778a17e64cabdb200679240fa004b5c5c9a684ecae773e8497997ab7c968d6 guix-build-52b1595850f6/output/aarch64-linux-gnu/bitcoin-52b1595850f6-aarch64-linux-gnu-debug.tar.gz
6a2f49dbc8238438d40d46c3ce699814ae1d9eb6d74dd0b4ff273db6648b181c guix-build-52b1595850f6/output/aarch64-linux-gnu/bitcoin-52b1595850f6-aarch64-linux-gnu.tar.gz
2fce92
...

💬 johnny9 commented on issue "Async Payjoin":
(https://github.com/bitcoin/bitcoin/issues/33684#issuecomment-3481702959)
> > Given that this is adding new features to the GUI. Is the plan to add them to the "legacy" GUI, the new QML GUI, or implement Payjoin into both? cc [@hebasto](https://github.com/hebasto)
>
> cc [@GBKS](https://github.com/GBKS) [@johnny9](https://github.com/johnny9)

My vote would be to continue to add features to the Qt Widgets gui. There might be differing opinions but I see the two as potentially serving different target platforms as well as different target users.

💬 waketraindev commented on pull request "qt: add console commands for clearing output and history":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3481744961)
Added release notes file.

🤔 hebasto reviewed a pull request: "Increase tooltip wrap threshold from 80 to 100 characters"
(https://github.com/bitcoin-core/gui/pull/905#pullrequestreview-3412212034)
This code was introduced in https://github.com/bitcoin/bitcoin/pull/1090. During that discussion, there was a [concern](https://github.com/bitcoin/bitcoin/pull/1090#issuecomment-5452893) that "tooltip windows [were] too wide sometimes".

This isn’t about fitting on the screen, but about improving readability.

cc @laanwj @GBKS

💬 hulxv commented on pull request "feat: shell completions for bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/33747#issuecomment-3481770602)
> Thanks. However aside from the fish addition, this duplicates works already being done in #33385 and #33402.

Actually, bash is the only is implemented in #33385 but zsh isn't implemented in #33402

Also in #33385, he covers only `rpc`, `node`, and `tx`, but mine covers all subcommands

🤔 sipa reviewed a pull request: "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/31713#pullrequestreview-3412258624)
ACK 6bf4f4a87889b0679c744b0a171c98a9386fe8c0

Just one test-only comment.

💬 sipa commented on pull request "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31713#discussion_r2487390409)
In commit "refactor(miniscript): Remove superfluous unique_ptr-indirection"

Should this be `const std::optional<Node>& node` (by reference)?

💬 sipa commented on pull request "miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()`":
(https://github.com/bitcoin/bitcoin/pull/31734#issuecomment-3481865789)
Code review ACK 28a4fcb03c0fb1cd5112eca1eb36dcb13e0b4ff2

Conceptually seems fine too, but I'd want to see @achow101's opinion on that.

🤔 sipa reviewed a pull request: "crypto: Use secure_allocator for `AES256_ctx`"
(https://github.com/bitcoin/bitcoin/pull/31774#pullrequestreview-3412340683)
utACK 8bdcd12d3bcbeaf922fa10dc2a261848e3900cfd

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3482056374)
`a9d697c0e1...c6f46943d1`: rebase due to conflicts, https://github.com/bitcoin/bitcoin/pull/33567 was merged so removed that change from here. Thanks!

💬 b-l-u-e commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3482056998)
> Are you still working on this?

Yes am still working on this
Caught up on something else

💬 hodlinator commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3482164060)
Tested running the Linux->Windows CI locally and this patch seems to fix the failure:
```diff
--- a/src/test/headers_sync_chainwork_tests.cpp
+++ b/src/test/headers_sync_chainwork_tests.cpp
@@ -52,7 +52,7 @@ constexpr size_t COMMITMENT_PERIOD{600}; // Somewhat close to mainnet.

struct HeadersGeneratorSetup : public RegTestingSetup {
const CBlock& genesis{Params().GenesisBlock()};
- const CBlockIndex& chain_start{WITH_LOCK(::cs_main, return *Assert(m_node.chainman->m_blockman.L
...