👍 TheCharlatan approved a pull request: "depends: Update libmultiprocess library before converting to subtree"
(https://github.com/bitcoin/bitcoin/pull/31740#pullrequestreview-2580718512)
ACK 4e0aa1835b3e980ceda29ec90e7115d7fef53f51

🤔 rkrux reviewed a pull request: "psbt: add non-default sighash types to PSBTs and unify sighash type match checking"
(https://github.com/bitcoin/bitcoin/pull/31622#pullrequestreview-2580366855)
Concept ACK fefe0636d4ae7c246042276cacd60b22f5fc6bb9

Good PR with enough context to unpack for me. Left few comments, will review again soon.

💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933484246)
`If only the parameter is provided`

💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933559100)
`std::optional<int> sighash_type = 1 /* SIGHASH_ALL */`
Does it not require `std::nullopt` as the default like in other cases?

💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933618280)
Confirming - it's being removed from here because this check now happens inside `SignPSBTInput` that is called down below?

💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933698058)
Can this commit be a separate PR? It would be nice to have a functional test that checks non witness utxos are dropped if sighash type is not `SIGHASH_ANYONECANPAY`. It can be added in this PR as well at the cost of a larger diff.

💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933483132)
`parameter`

💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933604449)
`If only the psbt field is provided, refuse to sign.`

How does it refuse to sign in this case?

💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933734099)
Nit feel free to ignore: I ignored this duplication in the first commit but now that it's being done twice, a refactor could be to just iterate this in a loop of sighash types.

💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933731594)
Atm, there is only 1 input in the PSBT. It would be nice to have another input that is Taproot and check for its sighash.

💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933654773)
Is the downside of adding sighash in PSBT in case of default or all is that it increases PSBT size?

💬 TheCharlatan commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621465199)
As an alternative to reaching into depends or committing generated files, could it be feasible to set native compilers in the toolchain file instead for compiling the code generators?

🚀 fanquake merged a pull request: "depends: Update libmultiprocess library before converting to subtree"
(https://github.com/bitcoin/bitcoin/pull/31740)

🤔 glozow reviewed a pull request: "test: fix intermittent timeout in p2p_1p1c_network.py"
(https://github.com/bitcoin/bitcoin/pull/31751#pullrequestreview-2580850613)
utACK 215d8b0a2ccf3a7275ea483339c58da5e0085e83, the explanation makes sense to me and agree the p2ps don't need to be connected after this point.

💬 hebasto commented on issue "build: `cmake --install` fails if only select targets are built":
(https://github.com/bitcoin/bitcoin/issues/31745#issuecomment-2621498472)
> I think what you're really looking for is a way to only build/install certain binaries. For that, the workaround would be to name each one as a separate component. That would allow for:
>
> cmake -B build
> cmake --build build --target bitcoind bitcoin-cli
> cmake --install build --component bitcoind bitcoin-cli

I agree. We have already [done](https://github.com/bitcoin/bitcoin/pull/30835) it for the `bitcoinkernel` target.

On the other hand, the [`CMAKE_SKIP_INSTALL_ALL_DEPENDENCY`](https:
...

🤔 hebasto reviewed a pull request: "doc: update links in ci.yml"
(https://github.com/bitcoin/bitcoin/pull/31736#pullrequestreview-2580887381)
Post-merge ACK 1681c08d42c302088586ca9b36b278217723f66a.

💬 hebasto commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621519226)
Concept ACK.

💬 Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2621698276)
Rebased after the test change in #31740.

👋 ryanofsky's pull request is ready for review: "multiprocess: Add libmultiprocess git subtree"
(https://github.com/bitcoin/bitcoin/pull/31741)

💬 hodlinator commented on pull request "miniscript: convert non-critical asserts to CHECK_NONFATAL":
(https://github.com/bitcoin/bitcoin/pull/31727#issuecomment-2621725687)
> Same reason as [here](https://github.com/bitcoin/bitcoin/pull/31727#discussion_r1929121645). I kept asserts where breaking the invariant would lead to UB. In the examples you point out it would read out of the `sats` vector bounds.

Okay, and there is no way *user input* from RPCs etc would currently be able to trigger the asserts?

> Actually this should be a strict check, not an inferior or equal to!

Ah, didn't realize, good catch indeed, `=` should be dropped.