💬 fanquake commented on pull request "fuzz: Limit wallet_notifications iterations (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31467#issuecomment-2672635509)
@dergoegge you likely also have some thoughts here?

✅ fanquake closed a pull request: "test: locking -testdatadir when not specified and then deleting lock and dir at end of test"
(https://github.com/bitcoin/bitcoin/pull/31328)

💬 fanquake commented on pull request "test: locking -testdatadir when not specified and then deleting lock and dir at end of test":
(https://github.com/bitcoin/bitcoin/pull/31328#issuecomment-2672636274)
I think its fine to leave this as-is for now. So going to close.

💬 maflcko commented on issue "build: x86 ASan build broken "error: inline assembly requires more registers than available"":
(https://github.com/bitcoin/bitcoin/issues/31913#issuecomment-2672638759)
> > You can also set -O1 instead of -O0, but I guess you don't want that?
>
> Just re-read your message, isn't `-O2` our default? i.e. I don't think I was using `-O0` when I first saw this.

You explicitly set it to the empty string in `-DCMAKE_CXX_FLAGS_RELWITHDEBINFO=""` and IIRC the default for compilers is `-O0`, so I presumed that this was intentional.

💬 fanquake commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#issuecomment-2672644248)
Moved to draft for now. You'll need to squash your commits, and address the various review feedback.

📝 fanquake converted_to_draft a pull request: "Added rescan option for import descriptors"
(https://github.com/bitcoin/bitcoin/pull/31668)
Fix related to issue: https://github.com/bitcoin/bitcoin/issues/31263

💬 zaidmstrr commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#issuecomment-2672645087)
Concept ACK

💬 zaidmstrr commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r1964317304)
Done

📝 mahdirahimi1999 opened a pull request: "Refactor file handling with context managers in multiple files"
(https://github.com/bitcoin/bitcoin/pull/31919)
**Description:**
This pull request refactors multiple instances of file handling across different files by replacing direct `open()` calls with context managers. The change does not affect the core functionality of the application but improves resource management by ensuring that files are properly closed after use. This results in cleaner code and better resource handling without altering any existing behavior.

✅ maflcko closed a pull request: "fuzz: Limit wallet_notifications iterations (take 2)"
(https://github.com/bitcoin/bitcoin/pull/31467)

💬 maflcko commented on pull request "fuzz: Limit wallet_notifications iterations (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31467#issuecomment-2672649759)
Follow-ups could be:

* Remove knapsack, or disable it for this target
* Temporarily remove the target until knapsack is removed
* Other possible avenues for speed-up of this target, or a cleanup, or a fix for stability issues

Though, I don't have time to work on this right now, so closing for now.

💬 fanquake commented on pull request "refactor: Enforce readability-avoid-const-params-in-decls":
(https://github.com/bitcoin/bitcoin/pull/31650#issuecomment-2672651916)
Concept ACK - has enforcement, few conflicts, added to 30.x.

✅ fanquake closed a pull request: "Refactor file handling with context managers in multiple files"
(https://github.com/bitcoin/bitcoin/pull/31919)

👍 fanquake approved a pull request: "test: Remove non-portable IPv6 test"
(https://github.com/bitcoin/bitcoin/pull/31580#pullrequestreview-2631146511)
ACK d871d778251c35fd55d420ecf5c278f3edfea227

🚀 fanquake merged a pull request: "test: Remove non-portable IPv6 test"
(https://github.com/bitcoin/bitcoin/pull/31580)

💬 glozow commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1964333623)
This `CFeeRate` construction seem wrong to me - you've swapped the size and fee?

💬 glozow commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1964333834)
same

📝 theuni opened a pull request: "build: create Depends build type for depends and use it by default for depends builds"
(https://github.com/bitcoin/bitcoin/pull/31920)
As discussed a good bit with fanquake and hebasto. Would be nice to have for v29, but it's very late, so no worries if it doesn't make it.

Fundamentally, this creates a "Depends" build type which represents the flags that were used to build depends as opposed to colliding with the other types.

This allows the forwarding of optional flags into the CMake build for the "Depends" build type, but the user can now optionally use an existing (Debug/RelWitDebInfo/etc.) type to ignore the optimizat
...

💬 maflcko commented on pull request "build: create Depends build type for depends and use it by default for depends builds":
(https://github.com/bitcoin/bitcoin/pull/31920#issuecomment-2672701261)
How does this interact with the depends DEBUG build? It looks like this is a breaking change for people (and the ci) picking the Debug build type afterwards. It would be good to clarify this and also adjust the CI scripts, if needed.

💬 mzumsande commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1964238336)
If I understand it correctly, the current logic is that we assign additional weight for all peers that have an entry in `m_peer_orphanage_info`, which means we don't assign weight for peers not participating in tx relay or that do but never sent us an orphan before. But once they sent us an orphan, that weight will stay reserved until they disconnect (and can be used by other peers too), even if they never send us another orphan.

This seems like a middle-ground between assigning each peer a f
...