📝 maflcko opened a pull request: "ci: Add mising -Wno-error=maybe-uninitialized to armhf task"
(https://github.com/bitcoin/bitcoin/pull/30144)

💬 maflcko commented on pull request "cli: Detect port errors in rpcconnect and rpcport":
(https://github.com/bitcoin/bitcoin/pull/29521#discussion_r1607698133)
Fixed in https://github.com/bitcoin/bitcoin/pull/30144

💬 maflcko commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#issuecomment-2121826810)
The CI failure can be ignored during review. It is fixed in https://github.com/bitcoin/bitcoin/pull/30144

💬 maflcko commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#discussion_r1607712689)
style-wise, I wonder if it makes sense to call wait_until and find_conn 6 times, when all logic could be put into a single function, which will be passed as a `lambda` to wait_until once.

I guess this would mean slightly cleaner code, but would make it harder to see which part of the single function returned `False` when a failure happens. Though, debug logs could be added to help with that, if needed.

What do you think?

💬 maflcko commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#discussion_r1607713755)
nit: Could add a comment to where the ID is set, to clarify that it is now required for `connect_nodes` to work?

💬 maflcko commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#issuecomment-2121847793)
> This approach is fragile, as any of the peers involved in the process can drop, lose, or
> create a connection at any step, causing subsequent `wait_until` checks to stall indefinitely
> even when the peers in question were connected successfully.

I'd say the tests should avoid racy code and deterministically execute the test code line-by-line, but enforcing this in `connect_nodes` is the wrong approach. So Concept ACK.

💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1607767150)
Updated to remove the `extra` argument.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1607790271)
Hmm, maybe we can keep track of whether we have have successfully managed to connect to at least one `.onion` address via the `-proxy`, then we can assume that is a Tor proxy? How does that sound?

💬 virtu commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2121965779)
LGTM! ACK [8109319](https://github.com/bitcoin/bitcoin/commit/8109319e102c41d3aeed0ecfbc3a0e23b7fea807)

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2121977231)
> Trying this with -pcp=1 on my home internet connection:

Thanks for testing!
The received packet is not a valid PCP response (too short). But interpreting it as one anyway, the version byte is 0x00, result code is 0x01 (UNSUPP_VERSION). So you might have one of those routers that supports NAT-PMP only.

💬 paplorinc commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#issuecomment-2122051145)
re-ACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd

💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1607890616)
I like this, no need to introduce all this error handling infrastructure for a debug-only option. Applied.

💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2122100379)
Updated 79c9c55b27bd113885da2c36ad76e5ed145027b3 -> 4a1df97a016935348e50f384ed52a6671990835f ([noGlobalScriptCache_0](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_0) -> [noGlobalScriptCache_1](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_0..noGlobalScriptCache_1))

* Addressed @theuni's [comment](https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1607175446), made `C
...

🚀 fanquake merged a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606)

💬 fjahr commented on pull request "ci: Add mising -Wno-error=maybe-uninitialized to armhf task":
(https://github.com/bitcoin/bitcoin/pull/30144#issuecomment-2122157154)
utACK fa73431dd4709754c34a4d5ad1c940ff9e628cf3

💬 fjahr commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#issuecomment-2122161034)
re-ACK ef692e52b4121ba3d8de91e45b2d33aff946e8d2

Also reviewed https://github.com/bitcoin/bitcoin/pull/30144 to confirm the failure is ok.

💬 fanquake commented on pull request "util: avoid using thread_local variable that has a destructor":
(https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1607953823)
I'm going to do this in #30137.

🚀 fanquake merged a pull request: "util: avoid using thread_local variable that has a destructor"
(https://github.com/bitcoin/bitcoin/pull/30095)

🚀 fanquake merged a pull request: "doc: Update NetBSD Build Guide"
(https://github.com/bitcoin/bitcoin/pull/30143)

💬 maflcko commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2122188355)
I wonder if it makes sense to collect the fuzz inputs to qa-assets, from testers that still have them. However, the fuzz CI does not compile with BDB, so that'll probably need to be adjusted first.