💬 ismaelsadeeq commented on pull request "Mempool: Do not enforce TRUC checks on reorg":
(https://github.com/bitcoin/bitcoin/pull/33504#discussion_r2398322992)
In "test: add more TRUC reorg coverge" https://github.com/bitcoin/bitcoin/commit/06df14ba75be5f48cf9c417424900ace17d1cf4d

This is empty mempool check in between the block generation is redundant, because we have not send any transaction to the mempool.

💬 ismaelsadeeq commented on pull request "Mempool: Do not enforce TRUC checks on reorg":
(https://github.com/bitcoin/bitcoin/pull/33504#discussion_r2398320236)
In "test: add more TRUC reorg coverge" 06df14ba75be5f48cf9c417424900ace17d1cf4d

What does "Testing overly-large child size" mean here? the tx being created has a normal size
maybe use the pointer as used above 3<-2 (large size)?

💬 maflcko commented on pull request "ci: fix buildx gha cache authentication on forks":
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3360615324)
> (You can see an outline here if you're interested [master...willcl-ark:bitcoin:docker-ci-containers](https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:docker-ci-containers) from march. This attempt tried to preserve the ability to run on bare metal too, but if we didn't want that then most of the scripts could be moved into dockerfiles too, further simplifying/locking us in to docker 😋 .)

bare metal is required for the macOS CI, so I don't think we can go pure docker.
...

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2398415091)
Ah yes, nice catch. I was wondering why the PoW check was still failing so many times in the coverage.

💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3360747581)
> did you consider saving `proxy` in `ThreadPrivateBroadcast()` in `m_private_broadcast` object and then retrieving it later in `ConnectNode` instead of passing the `proxy` as a parameter through `OpenNetworkConnection`/`ConnectNode`? It would eliminate some parameter passing that's only used for private broadcast connections. but might make `proxy` lifetime unclear (not sure about this), just curious if you considered this.

In general, saving the proxy somewhere and later retrieving it from
...

💬 ryanofsky commented on pull request "Update libmultiprocess subtree to support reduced logging":
(https://github.com/bitcoin/bitcoin/pull/33518#issuecomment-3360771493)
> Looks like the tsan CI failed?

Also happens in the 30.x PR (https://github.com/bitcoin/bitcoin/actions/runs/18190672725/job/51784809732?pr=33519) so interesting that it doesn't happen with -fsanitize=thread job in libmultiprocess repository (https://github.com/bitcoin-core/libmultiprocess/actions/runs/18190339842/job/51783802159). Will debug.

🤔 stickies-v reviewed a pull request: "rpc: refactor: use string_view in Arg/MaybeArg"
(https://github.com/bitcoin/bitcoin/pull/32983#pullrequestreview-3294226533)
Force-pushed to address merge conflict from #32326, and addressed 2 review nits from @pablomartin4btc :
- replaced `strprintf` usage with `tfm::format`
- replaced `descriptor.size() == 0` with `descriptor.empty()`

💬 stickies-v commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2398528866)
Keeping as `value_or`, no strong opinion either way.

💬 stickies-v commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2398527599)
Done (but without negating the emptiness which seems wrong)

💬 stickies-v commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2398531222)
Done.

💬 maflcko commented on pull request "Update libmultiprocess subtree to support reduced logging":
(https://github.com/bitcoin/bitcoin/pull/33518#issuecomment-3360857978)
I think the nix config is not using an instrumented libcpp, which could be a reason for the difference? C.f. `USE_INSTRUMENTED_LIBCPP="Thread"`.

🚀 fanquake merged a pull request: "Mempool: Do not enforce TRUC checks on reorg"
(https://github.com/bitcoin/bitcoin/pull/33504)

💬 luke-jr commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2398673387)
At the time, you missed maxfeerate, but I see that has since been added

💬 luke-jr commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#issuecomment-3361035802)
>We don't want to send invalid txs or txs that we would not broadcast,. We want to validates the txs before sending it to a peer.

Maybe it should just do validation then, and can be followed up by the raw send?

>Additionally, peer_id argument could take an array of ids and send the tx to multiple peers. (this is not implemented, but shouldn't be difficult to)

Sounds like a job for batching.

(But maybe it's enough complexity to be worth a simplified RPC like this)

🤔 Eunovo reviewed a pull request: "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py"
(https://github.com/bitcoin/bitcoin/pull/33184#pullrequestreview-3294444435)
Concept ACK https://github.com/bitcoin/bitcoin/pull/33184/commits/1ffdf5253388af3cf274efd723b7129d9639da8c

The test is much simpler and the untested path has been eliminated, but I left the comment about the `OP_RETURN` test below.

💬 Eunovo commented on pull request "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2398710391)
https://github.com/bitcoin/bitcoin/pull/33184/commits/1ffdf5253388af3cf274efd723b7129d9639da8c

This is basically testing that two results of `getblockstats` match each other. It doesn't actually determine the `expected_tip_stats` in the test and then compare against the actual result of `getblockstats`.

🤔 ismaelsadeeq reviewed a pull request: "contrib: fix macOS deployment with no translations"
(https://github.com/bitcoin/bitcoin/pull/33482#pullrequestreview-3294494300)
Thanks for attempting to contribute, welcome 🪴

The config comments also mentioned that adding translation file is optional.

```
abubakarismail@Abubakars-MacBook-Pro ~/D/W/b/b/c/macdeploy (master) [2]> ./macdeployqtplus --help
usage: macdeployqtplus [-h] [-verbose [VERBOSE]] [-no-plugins] [-no-strip] [-translations-dir path] [-zip zip] app-bundle

Improved version of macdeployqt. Outputs a ready-to-deploy app in a folder "dist" and optionally wraps it in a .zip file. Note, that the "dist" fol
...

🤔 ismaelsadeeq reviewed a pull request: "contrib: fix macOS deployment with no translations"
(https://github.com/bitcoin/bitcoin/pull/33482#pullrequestreview-3294495650)
Concept ACK

💬 fanquake commented on pull request "Mempool: Do not enforce TRUC checks on reorg":
(https://github.com/bitcoin/bitcoin/pull/33504#issuecomment-3361147273)
Backported to 30.x in #33473.

💬 fanquake commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3361148614)
Backported to 30.x in #33473.