🤔 BrandonOdiwuor reviewed a pull request: "wallet: Fix migration of blank wallets"
(https://github.com/bitcoin/bitcoin/pull/28976#pullrequestreview-1759651852)
Concept ACK

Evaluating descriptor wallets using descriptor wallet flag instead of the presence of LegacyScriptPubKeyMan when migrating wallets

👍 TheCharlatan approved a pull request: "build: disable external-signer for Windows"
(https://github.com/bitcoin/bitcoin/pull/28967#pullrequestreview-1759657432)
ACK 308aec3e5655327d98e0428d8205d246f24d6af5

💬 willcl-ark commented on pull request "init: don't delete PID file if it was not generated":
(https://github.com/bitcoin/bitcoin/pull/28946#issuecomment-1835990929)
@stickies-v took most of your suggestions in 82f18e907a2af5a47e805861234de4672ed6d801

🤔 maflcko reviewed a pull request: "POC: Replace Boost.Process with cpp-subprocess"
(https://github.com/bitcoin/bitcoin/pull/28981#pullrequestreview-1759731506)
I guess the compile error is due to subprocess requiring C++17 and not allowing C++20 (or later)?

💬 maflcko commented on pull request "POC: Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#discussion_r1412055174)
nit: Could split this into one-per-line to avoid overly long lines, diffs and reduce merge conflicts?

💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1836094254)
`0858b0c084...8b10990aa0`: rebase due to conflicts

💬 kashifs commented on pull request "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block":
(https://github.com/bitcoin/bitcoin/pull/26415#issuecomment-1836097643)
@andrewtoth, that worked! It was an issue with RPC credentials. Now all my testing leads to results similar to yours. Thanks again, for all of your help with this.

💬 vasild commented on pull request "ci: run test_bitcoin with DEBUG_LOG_OUT in RUN_UNIT_TESTS_SEQUENTIAL":
(https://github.com/bitcoin/bitcoin/pull/28736#issuecomment-1836105479)
`e3ade1fbc1...2b7888ec3e`: rebase due to conflicts and add the `ed` package to `CI_BASE_PACKAGES` (see https://github.com/bitcoin/bitcoin/pull/28736#discussion_r1409360652).

💬 vasild commented on pull request "ci: run test_bitcoin with DEBUG_LOG_OUT in RUN_UNIT_TESTS_SEQUENTIAL":
(https://github.com/bitcoin/bitcoin/pull/28736#discussion_r1412093722)
Dropped `TEST_RUNNER_ENV` now that https://github.com/bitcoin/bitcoin/pull/28954 removed it from everywhere.

💬 vasild commented on pull request "ci: run test_bitcoin with DEBUG_LOG_OUT in RUN_UNIT_TESTS_SEQUENTIAL":
(https://github.com/bitcoin/bitcoin/pull/28736#discussion_r1412094023)
Added to `CI_BASE_PACKAGES` in latest push.

💬 fanquake commented on pull request "POC: Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#discussion_r1412108239)
Probably easier to just fix the typos, than add more things for linters to exclude? No new ones should be introduced.

💬 fanquake commented on pull request "POC: Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1836129909)
Concept ACK on removing Boost Process. Is the plan to also delete any unused code from this header? I don't think copy-pasting dead/untested code into this repository is a good idea, and only serves to complicate review, and potentially cause issues in future (it maybe already doesn't work with c++20?)

🤔 glozow reviewed a pull request: "bugfix, Change up submitpackage results to return results for all transactions"
(https://github.com/bitcoin/bitcoin/pull/28848#pullrequestreview-1759894708)
utACK f23ba24aa079d68697d475789cd21bd7b5075550, thanks for taking the suggestions

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1412155613)
Will change 👍

💬 theStack commented on pull request "POC: Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1836203421)
Concept ACK, thanks for picking this up.

💬 maflcko commented on pull request "test: Add and use option for tx-version in MiniWallet methods":
(https://github.com/bitcoin/bitcoin/pull/28972#issuecomment-1836208727)
It is also an alternative to ab2c3eafb7ccadeb3588a89e6383fc751e6578bd (cc @glozow )

💬 Sjors commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#issuecomment-1836284078)
Light re-utACK f23ba24aa079d68697d475789cd21bd7b5075550

💬 ryanofsky commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1836293866)
re: https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1823568385 and earlier discussion about libcrypto starting https://github.com/bitcoin/bitcoin/pull/28690#pullrequestreview-1734365185:

> I agree that crypto isn't the most ideal home for hex/string functions, but it _does_ actually make it easier to use the lib as a standalone and not have to write your own versions.

Just want to note that `libbitcoin_crypto` and `src/crypto/` are not currently mentioned in the libraries docume
...

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1412243707)
Just adding a comment with some context, saying that for chances lower than 1% it may not be worth the computation may be fine. It just struck me as weird where that value came from.

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1412244443)
Fair