💬 darosior commented on pull request "[29.x] Backport #32521":
(https://github.com/bitcoin/bitcoin/pull/33013#issuecomment-3113500474)
Opened a separate backport PR for the flakiness fix: https://github.com/bitcoin/bitcoin/pull/33052

💬 fanquake commented on pull request "test: fix intermittent failure in wallet_reorgsrestore.py":
(https://github.com/bitcoin/bitcoin/pull/32069#issuecomment-3113519822)
Backported to `29.x` in #33052.

👍 ryanofsky approved a pull request: "Add bitcoin-{node,gui} to release binaries for IPC"
(https://github.com/bitcoin/bitcoin/pull/31802#pullrequestreview-3051726249)
Code review ACK 241e9d14879abbc75a17dba764ef2ad58179db57. Just rebased since last review

💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2228555002)
In commit "cmake: enable ENABLE_IPC option by default" (969a9472a43b7e8c0ae1b8d0dd1a71e5d118ea0c)

Maybe sort these while you are splitting them up. libboost-dev libqrencode-dev and systemtap-sdt-dev seem to be the only entries out of order.

💬 purpleKarrot commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3113533739)
I confirm that the change to the CMake code is OK. I am not very fond of the `install_binary_component` function, but that is orthogonal to the PR.

💬 maflcko commented on pull request "rpc: Fix internal bug in descriptorprocesspsbt when encountering invalid signatures":
(https://github.com/bitcoin/bitcoin/pull/33014#issuecomment-3113534833)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

💬 fjahr commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2228569351)
done

💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2228571714)
I did sort them, but missed a few.

💬 glozow commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#discussion_r2228571572)
ah doi, restored

💬 glozow commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#discussion_r2228570611)
🤦 thank you, fixed

💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2228578817)
Fixed the sort

💬 fanquake commented on pull request "[29.x] backport #32069":
(https://github.com/bitcoin/bitcoin/pull/33052#issuecomment-3113551574)
Thanks, this should have been backported when #31757 was done, but likely missed at that happened later. Can you also add the rel note: https://github.com/fanquake/bitcoin/commit/da24253775f9f6cecd8fbcf65e8aa6f40090f72c.

💬 fanquake commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2228510972)
In 28af4d727b3eeff3da0a3dfab0260c57fb52111c: This should be squashed into the commit that actually changes the Guix build. Ideally CMake would warn/error here, at least in the Guix build. We don't want it to continue silently if binaries don't exist/are missing from these checks.

💬 fanquake commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2228508989)
Please don't make random (incomplete codebase-wise) formatting changes, to unrelated parts of the code. I think there's enough happening in this PR.

💬 fanquake commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2228571627)
Yea, these need fleshing out, because they don't give much useful information to users, and will likely leave them with questions: "What's IPC?" (never defined), "What's `bitcoin-node`; should I use that instead of `bitcoind`?" (same for the GUI), "Why is this enabled by default if it's for some experimental feature I (and basically all other users) don't use?".

💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2228590806)
This kept triggering needs-rebase, so seemed useful to fix. But I can revert.

💬 fanquake commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3113566277)
You could probably split out the renaming, and just get them landed, given that should happen regardless of all the other changes here.

🤔 pablomartin4btc reviewed a pull request: "rpc: refactor: use string_view in Arg/MaybeArg"
(https://github.com/bitcoin/bitcoin/pull/32983#pullrequestreview-3051797902)
re-ACK 122b432b71e148cd85edcd74df741f99ac7f187c

Not blocking: I’d personally prefer keeping `Arg<std::string>` in `signmessage.cpp` if we're not planning to update `MessageVerify` to accept `string_view` instead of `const std::string&`. Is updating `MessageVerify` something you’d prefer to avoid at this stage?

💬 darosior commented on pull request "[29.x] backport #32069":
(https://github.com/bitcoin/bitcoin/pull/33052#issuecomment-3113596396)
Sure, done.

💬 stickies-v commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#issuecomment-3113627621)
> Is updating `MessageVerify` something you’d prefer to avoid at this stage?

Yes. It involves a cascade of changes, many touching consensus code, that require significant review and have mostly nothing to do with RPC, so it would blow up the scope of this PR.