💬 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

💬 hebasto commented on pull request "POC: Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1836332387)
> Is the plan to also delete any unused code from this header?

I haven't consider it yet.

> it maybe already doesn't work with c++20?

CI jobs that uses C++20 are passing so far.

💬 sipa commented on pull request "POC: Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1836337134)
> Is the plan to also delete any unused code from this header?

If at all possible, I'd prefer for it to be subtreed, so that updating to newer upstream versions is easy.

💬 fanquake commented on pull request "POC: Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1836341684)
I was actually thinking the opposite. I think a subtree is overkill here, and, upstream isn't exactly active (some random merges but no development), so I'm not sure if there will be many new versions.

I'd rather take the small(er) portion of any code we need, and have some better integrated/reviewed. Hopefully that will help avoid randomly pulling code from upstream, that isn't well developed or reviewed.