🤔 Prabhat1308 reviewed a pull request: "wallet: Set minversion to FEATURE_LATEST during migration"
(https://github.com/bitcoin/bitcoin/pull/33041#pullrequestreview-3048953524)
Agree with the comment by @pablomartin4btc here that if we want to go the route of keeping the wallet Version , its better to set it as a constant since `FEATURE_LATEST` is the only version being maintained in the practical sense and rename the function along the lines of `SetVersion` . Then we can even go ahead with the modified https://github.com/bitcoin/bitcoin/pull/32977 cleanup .

💬 l0rinc commented on pull request "refactor,test: follow-ups to multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/33039#discussion_r2226638379)
Thanks, I'll consider it next time I'm pushing

🤔 w0xlt reviewed a pull request: "wallet: Set minversion to FEATURE_LATEST during migration"
(https://github.com/bitcoin/bitcoin/pull/33041#pullrequestreview-3049035879)
ACK https://github.com/bitcoin/bitcoin/pull/33041/commits/5aa758861cf1399842fd0bea3a42d5b0cafcb0f6

💬 achow101 commented on pull request "test: reduce runtime of p2p_opportunistic_1p1c.py":
(https://github.com/bitcoin/bitcoin/pull/33048#issuecomment-3110151117)
The runtime is now much better, thanks!

ACK eb65f57f319dc4e2ea8c83cf7e283c36f1c0d53b

I checked that the changes looked sane, but did not exhaustively walkthrough the scenarios to determine whether evictions were occurring.

💬 w0xlt commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2226679396)
Is there any specific reason not to use `unsigned char` directly, like `std::span<const unsigned char > sp)` ?

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2226689907)
Will change if I need to retouch.

💬 furszy commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3110184342)
@HowHsu, are you going to include the test or a patch to reproduce the issue? I’m happy to ACK it either way.

💬 w0xlt commented on pull request "wallet: Store transactions in a separate sqlite table":
(https://github.com/bitcoin/bitcoin/pull/33034#issuecomment-3110194893)
Concept ACK

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2226706401)
In theory we could change `uint256`'s `value_type`, and we wouldn't need to make any changes here in that case.

🤔 w0xlt reviewed a pull request: "wallet: Prepare for future upgrades by recording versions of last client to open and decrypt"
(https://github.com/bitcoin/bitcoin/pull/32895#pullrequestreview-3049112073)
ACK https://github.com/bitcoin/bitcoin/pull/32895/commits/b0470f573735a04ebc87ca5d3e00a73e3f10fee1

nit: Commit https://github.com/bitcoin/bitcoin/pull/32895/commits/3662960470d74850008b7b48b6820d02002f7a5d appears to have CI failures.

🤔 w0xlt reviewed a pull request: "wallet: Set migrated wallet name only on success"
(https://github.com/bitcoin/bitcoin/pull/32984#pullrequestreview-3049114357)
reACK https://github.com/bitcoin/bitcoin/pull/32984/commits/060695c22ae7b2b0f2a1dd1417ed1b9d5a5ab542

💬 achow101 commented on pull request "wallet: Prepare for future upgrades by recording versions of last client to open and decrypt":
(https://github.com/bitcoin/bitcoin/pull/32895#issuecomment-3110226273)
> nit: Commit [3662960](https://github.com/bitcoin/bitcoin/commit/3662960470d74850008b7b48b6820d02002f7a5d) appears to have CI failures.

I believe that's only because the tasks were canceled by a new push.

💬 Karim8090 commented on issue "guix: Zip file non-determinism when building in WSL":
(https://github.com/bitcoin/bitcoin/issues/32931#issuecomment-3110253591)
Babu8090

🤔 w0xlt reviewed a pull request: "wallet: Set descriptor cache upgraded flag for migrated wallets"
(https://github.com/bitcoin/bitcoin/pull/33031#pullrequestreview-3049159589)
ACK https://github.com/bitcoin/bitcoin/pull/33031/commits/4e5faaba82b4e1f42edb3a52270ea9086c1edfa9

🤔 mzumsande reviewed a pull request: "net: Fix Discover() not running when using -bind=0.0.0.0:port"
(https://github.com/bitcoin/bitcoin/pull/32757#pullrequestreview-3049008730)
Does the added test succeed for others? I tried to follow the instructions and get

```
test 2025-07-23T21:50:01.787971Z TestFramework (ERROR): Address 1.1.1.1 not found in node0's local addresses: []
test 2025-07-23T21:50:01.788044Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "XXX/bitcoin/32757_bind0000/test/functional/test_framework/test_framework.py", line 195, in main

...

💬 mzumsande commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#discussion_r2226651425)
unrelated to the changes here, but `ifconfig` is deprecated, so this could be updated.

💬 pinheadmz commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3110456945)
Test worked for me on mac and failed as expected without 1.1.1.1 and 2.2.2.2

On my system the command was
`ifconfig en0 alias 1.1.1.1`

💬 murchandamus commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3110475043)
Maybe the intent of my comment wasn’t clear. We are seeing that blocks including lower feerate transactions take significantly longer to get propagated, and the way I’m reading the sentiment here, a lot of people do not share my concern over a potential DoS protection reduction. In so far, it seems that there is at least interest in this PR being further developed. My suggestion is that if someone were to address the review feedback here, they should probably lower all three settings, not just t
...

💬 achow101 commented on pull request "doc: update headers and TOC in `developer-notes.md`":
(https://github.com/bitcoin/bitcoin/pull/33040#issuecomment-3110481836)
I would prefer to drop the table of contents entirely.

I pretty much only ever read `.md` files on GitHub or with `mdcat`. On GitHub, the table of contents will be automatically generated, and with `mdcat`, the links don't work anyways.

🤔 stickies-v reviewed a pull request: "log: rate limiting followups"
(https://github.com/bitcoin/bitcoin/pull/33011#pullrequestreview-3046739749)
code lgtm 8aa2726f1cfbc1e4267a796cb000c4eeae5910f5 - but would like to see if we can make improvements on the `m_limiter` lifetime guarantees, and on the `logging_filesize_rate_limit` tests. Both are not essential, but I think it could make sense to include them here.