💬 davidgumberg commented on pull request "descriptors: Try pubkeys of both evenness when retrieving the private keys for an xonly pubkey in a descriptor":
(https://github.com/bitcoin/bitcoin/pull/31590#issuecomment-2588255783)
utACK https://github.com/bitcoin/bitcoin/pull/31590/commits/c0045e6cee06bc0029fb79b5a531aa1f2b817424

I think this fixes #31589, and fixes a bug in miniscript parsing of xonly pubkeys. [^1]

[^1]: I think there might still be an issue with `ConstPubkeyProvider::GetSize()`, I'll try to look into this and open a follow-up if necessary.

👍 hodlinator approved a pull request: "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior"
(https://github.com/bitcoin/bitcoin/pull/30529#pullrequestreview-2546259926)
ACK eb325bc0efd3f999189e155ba836be92e6cc9af7

Nice break up of commits for correcting each `-arg`.

In my case I still reviewed mostly through:
```
₿ git diff 2eccb8bc5e2245e42e05b8df3d9aa264b5f21a9a 80608ba5d282ae8713ea0136ea9a0208b254c082 > old
₿ git diff 2eccb8bc5e2245e42e05b8df3d9aa264b5f21a9a eb325bc0efd3f999189e155ba836be92e6cc9af7 > new
₿ meld old new &
```

*feature_config_args.py* - Nice that we are no longer relying on "leaked" `connect_arg` from prior `for`-loop, instead m
...

💬 hodlinator commented on pull request "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior":
(https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1912983327)
Typo
```suggestion
However, there are exceptions to this general rule. For example, it is an error to negate some options (e.g. `-nodatadir` is disallowed), and some negated strings are treated like `"0"` instead of `""` (e.g. `-noproxy` is treated like `-proxy=0`), and negating some lists can have side effects in addition to clearing the lists (e.g. `-noconnect` disables automatic connections in addition to dropping any manual connections specified previously with `-connect=<host>`). When the
...

📝 maflcko opened a pull request: "ci: Turn CentOS task into native one"
(https://github.com/bitcoin/bitcoin/pull/31651)
Cross-compiling to `i686-pc-linux-gnu` on CentOS in the CI is mostly redundant with the `ci/test/00_setup_env_i686_multiprocess.sh` task (albeit it using clang):

https://github.com/bitcoin/bitcoin/blob/35bf426e02210c1bbb04926f4ca2e0285fbfcd11/ci/test/00_setup_env_i686_multiprocess.sh#L9-L12

One task seems sufficient as a sanity check, given that there seems to be no real demand for this architecture anyway.

Turning the task into a native one makes it easier to run it on aarch64 or any o
...

💬 adamandrews1 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r1913911212)
This is done to align the method with PSBT. It could be removed without breaking the change. But it also is just clearing data which should take minimal time. I think it's worth keeping to align with PSBT format.

🤔 mzumsande reviewed a pull request: "test: avoid internet traffic"
(https://github.com/bitcoin/bitcoin/pull/31646#pullrequestreview-2548206643)
Code Review ACK 66dbfaca22ba55aea3a5e7ab73974b42134ba8f4

💬 mzumsande commented on pull request "test: avoid internet traffic":
(https://github.com/bitcoin/bitcoin/pull/31646#discussion_r1913933802)
could use `UNREACHABLE_PROXY_ARG` here as well

👍 tdb3 approved a pull request: "rpc: add gettarget , target getmininginfo field and show next block info"
(https://github.com/bitcoin/bitcoin/pull/31583#pullrequestreview-2548335954)
code review re ACK b945668216abbc978ce351b8a89910e651c6e595

Changes were to address the latest comments (removing "0x" and adjusting a comment).

```diff
$ git range-diff 228aba2c4d9ac0b2ca3edd3c2cdf0a92e55f669b..58b828f7f7ad216c7d7bf2e2ff431a66591e9d5c e8ea483fec05d824d2f35182b1a3bd68416b2df3~..b945668216abbc978ce351b8a89910e651c6e595
1: 6accee1844 = 1: e8ea483fec consensus: add DeriveTarget() to pow.h
2: 3e976d4171 = 2: 6a72497f50 build: move pow and chain to bitcoin_common
3
...

🤔 tdb3 reviewed a pull request: "test: avoid internet traffic"
(https://github.com/bitcoin/bitcoin/pull/31646#pullrequestreview-2548351524)
Concept ACK

Nice. Better consistency/privacy as well as a test runtime reduction. Will circle back to take a deeper look.

💬 darosior commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#issuecomment-2588847257)
I was playing with a couple routers and figured i'd test this. Could successfully map ports against an OPNSense box and a Spectrum "[WIFI 6E router](https://www.spectrum.com/internet/wifi-service/spectrum-advanced-wifi)".

💬 luke-jr commented on pull request "wallet: allow lable for external descriptor & disallow label for ranged descriptors":
(https://github.com/bitcoin/bitcoin/pull/31514#issuecomment-2588907624)
It would be nice to have a test that checks all 4 success/failure cases.

💬 naiyoma commented on pull request "test: Add test for rpcwhitelistdefault":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1914319868)
I've moved the new user to the strange_user list, renamed user3 to strangedude6 for consistency, and replaced "" with None as suggested here https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1904497634

💬 naiyoma commented on pull request "test: Add test for rpcwhitelistdefault":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1914324487)
https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1914319868

💬 naiyoma commented on pull request "test: Add test for rpcwhitelistdefault":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1914326733)
Done https://github.com/bitcoin/bitcoin/pull/29858/commits/cfce5f5cbacf7d4c209112ac9d35856bed4a3c6b

💬 naiyoma commented on pull request "test: Add test for rpcwhitelistdefault":
(https://github.com/bitcoin/bitcoin/pull/29858#issuecomment-2589165785)
> Code review ACK [fcf0ead](https://github.com/bitcoin/bitcoin/commit/fcf0ead6cd358fd35909e1102bdae2c176b0ace6). I left suggestions below, but none are important and they can be ignored. I think it is better to have this test coverage for this than not to have it, and no need to spend too much time iterating on the PR just to improve test code quality which is already decent.

Thanks for the review, I have pushed an update and resolved all the suggestions, https://github.com/bitcoin/bitcoin/
...

💬 vasild commented on pull request "test: avoid internet traffic":
(https://github.com/bitcoin/bitcoin/pull/31646#issuecomment-2589289729)
`66dbfaca22...2ed161c5ce`: do https://github.com/bitcoin/bitcoin/pull/31646#discussion_r1913933802

💬 vasild commented on pull request "test: avoid internet traffic":
(https://github.com/bitcoin/bitcoin/pull/31646#discussion_r1914418707)
Indeed! Done, thanks!

💬 hodlinator commented on pull request "optimization: batch XOR operations 12% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2589299296)
(I did a second run on 433412fd8478923dfdb20044f74c5d1e19fa8dd8, it took 3h45m53s, 9.5 minutes / 4.4% faster. So there is some variance).

💬 l0rinc commented on pull request "optimization: batch XOR operations 12% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2589302015)
These are the fastest IBDs I've seen so far

💬 Sjors commented on pull request "miner: always treat SegWit as active":
(https://github.com/bitcoin/bitcoin/pull/31625#issuecomment-2589341922)
It seems useful to illustrate the change here by regenerating the alternative mainnet blocks introduced in #31583, switching their coinbase outputs to taproot addresses. Before this PR the last block, which spends a coinbase output, would fail with `unexpected-witness`.

Marking as draft when that lands.