🤔 furszy reviewed a 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#pullrequestreview-2547867476)
ACK c0045e6cee06bc0029fb79b5a531aa1f2b817424

💬 laanwj commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1913795001)
Ok, made it non-optional now. Somehow feels more consistent. It's likely that this PR will go in first anyhow. Will look into a different solution for #31014.

💬 hodlinator commented on pull request "optimization: batch XOR operations 12% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2588220255)
Nodes on local LAN, same commits, both on SSD. Syncing node was laptop running 13th Gen Intel i7, 20 logical cores.

#### Full node / source

Made to not have any other connections (verified through running with `-debug=net` for a while).

Deleted *anchors.dat* & *peers.dat*.

```
₿ build/src/bitcoind -dbcache=30000 -nofixedseeds -nodnsseed
```

#### Syncing node

Deleted *~/.bitcoin*.

```
₿ time build/src/bitcoind -dbcache=30000 -stopatheight=878000 -connect=<sourcenode>
```
...

💬 darosior commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#issuecomment-2588243197)
re-ACK 9cd85281c6cc8d3fb01930e53109fae97a108dd8

👍 laanwj approved a pull request: "doc: Archive 28.1 release notes"
(https://github.com/bitcoin/bitcoin/pull/31630#pullrequestreview-2547939213)
ACK bb5f76ee013ee439f70e86319d22f308db8a24ea

💬 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/
...