💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1956206594)
Too bad :(

Try the smaller batches and see how we do? Lowering maxorphantxs is better than no test at all but clearly not optimal.

💬 instagibbs commented on pull request "test: Rename send_message to send_without_ping":
(https://github.com/bitcoin/bitcoin/pull/31859#issuecomment-2659437755)
concept ACK, many intermittent test failures end up being this

🤔 murchandamus reviewed a pull request: "psbt: MuSig2 Fields"
(https://github.com/bitcoin/bitcoin/pull/31247#pullrequestreview-2617937191)
LGTM, ACK 3adf43df82a7659c1734ce552917daf8b429bb24

🤔 rkrux reviewed a pull request: "wallet: fix crash on double block disconnection"
(https://github.com/bitcoin/bitcoin/pull/31757#pullrequestreview-2617954596)
Concept ACK

Initial pass, will review in detail soon.

💬 rkrux commented on pull request "wallet: fix crash on double block disconnection":
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1956236842)
```diff
- for (const CTransactionRef& ptx : Assert(block.data)->vtx) {
- SyncTransaction(ptx, TxStateInactive{});
+ for (const CTransactionRef& ptx : Assert(block.data)->vtx) {
+ // Coinbase transactions are not only inactive but also abandoned,
+ // meaning they should never be relayed standalone via the p2p protocol.
+ SyncTransaction(ptx, TxStateInactive{ /*abandoned=*/ ptx->IsCoinBase()});
```

Build and tests all good with this.

I considered this with
...

💬 Sjors commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1956245111)
It seems neither of these are available on Apple Silicon (`HWCAP2_RNG` isn't defined), so not much to test here.

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2659582312)
Rebased 5aeaa3f49d10562b8936ef36b1c25a6466dbe03e -> a604321c3e4bd50b52fa28e8567f6b068b2d2fb3 ([kernelApi_23](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_23) -> [kernelApi_24](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_24), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_23..kernelApi_24))

* Fixed conflict with #31844

💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-2659604837)
Rebased d2ceb2e0735a2c8343f8316b55fac55323aba62c -> 45bfd97ec7c9991f41673d79b01277bfd940e64e ([`pr/libexec.3`](https://github.com/ryanofsky/bitcoin/commits/pr/libexec.3) -> [`pr/libexec.4`](https://github.com/ryanofsky/bitcoin/commits/pr/libexec.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/libexec.3-rebase..pr/libexec.4)) due to conflict with #31844

💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2659615530)
Rebased 9194e461518e6b843c1a707ba1b10ab7a000e096 -> 59e1d4abb34453805a09a62c51ead7347787b2e3 ([`pr/mine.16`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.16) -> [`pr/mine.17`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.17), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.16-rebase..pr/mine.17)) due to conflict with #31844

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-2659617238)
Will review again soon, but a quick question: would it make sense to provide the ability for a user (who doesn't necessarily have easy access to the source code) to extract the asmap data from a running node? For example, to be able to run `asmap.py diff` on it. It could be an RPC command that just spits out the data in textual format, or a REST endpoint that directly provides it in binary.

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2659622448)
Rebased edb8a72226b362ee60e4184fd8f56d1e588ce5b6 -> 27bb348388a6708e8748d7073e352c047e8fb95c ([`pr/wrap.19`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.19) -> [`pr/wrap.20`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.20), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/wrap.19-rebase..pr/wrap.20)) due to silent conflict with #31844

📝 fanquake opened a pull request: "doc: add missing copyright headers"
(https://github.com/bitcoin/bitcoin/pull/31864)
Add & amends a number of copyright headers.
Remove some now obselete doc/code.

📝 fanquake opened a pull request: "build: move `rpc/external_signer` to node library"
(https://github.com/bitcoin/bitcoin/pull/31865)
Move `rpc/external_signer` from `bitcoin_common` to `bitcoin_node`.
Remove the check-deps suppression.

📝 ryanofsky opened a pull request: "test, refactor: Add TestNode.binaries to hold binary paths"
(https://github.com/bitcoin/bitcoin/pull/31866)
Add new `TestNode.binaries` object to manage paths to bitcoin binaries.

The `binaries` object makes it possible for the test framework to exercise the bitcoin wrapper executable introduced in https://github.com/bitcoin/bitcoin/pull/31375 and also makes it easier in general to add new binaries, and new options and environment variables controlling how they are invoked, because logic for invoking them that was previously spread out is now consolidated in one place.

These changes were origina
...

💬 maflcko commented on pull request "refactor: Remove redundant definitions":
(https://github.com/bitcoin/bitcoin/pull/29492#issuecomment-2659695808)
tend towards NACK, without a motivation (example of a real bug that this prevents or prevented):

* Given a motivation, it should be enforced with clang-tidy (or otherwise)
* Without motivation, it shouldn't be enforced, nor should the changes be made in the first place

💬 luisfg30 commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1956383771)
Unused variable `res`

```suggestion
_, wallet = self.migrate_and_get_rpc("taproot")
```

💬 maflcko commented on pull request "build: move `rpc/external_signer` to node library":
(https://github.com/bitcoin/bitcoin/pull/31865#issuecomment-2659736131)
lgtm ACK e501246e77cc36cff222fb07aed2fd1316e11e19

💬 s373nZ commented on pull request "cmake: add optional source files to bitcoin_crypto and crc32c directly":
(https://github.com/bitcoin/bitcoin/pull/31268#issuecomment-2659736519)
ACK 9cf746d6631739df9c9f80accd5812b319efcfec

Tested compilation on Ubuntu 24.04 x86_64. Learned a lot looking over this PR. Nice work!

💬 hebasto commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2659741940)
Tested a Windows-specific part:
```
$ env HOSTS=x86_64-w64-mingw32
$ ./contrib/guix/guix-build
$ ./contrib/guix/guix-codesign
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
ac50a82cb146e016d5d643460dc4ff7452a70497f2d95f76cee2bcfd82724ab6 guix-build-096525e92cc2/output/dist-archive/bitcoin-096525e92cc2-codesignatures-e252afe1296a.tar.gz
504608f78dc2be04bda41dc212d3cbb09afd270485884b03b426ad596b4b3611 guix-build-0965
...

💬 fanquake commented on pull request "cmake: add optional source files to bitcoin_crypto and crc32c directly":
(https://github.com/bitcoin/bitcoin/pull/31268#issuecomment-2659742890)
@theuni could you circle back here?