👍 hebasto approved a pull request: "Modify command line help to show support for BIP21 URIs"
(https://github.com/bitcoin-core/gui/pull/752#pullrequestreview-1874446277)
ACK ede5014c445dcb40ddcfdede2c51236bbfe85f5e.

💬 epiccurious commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#issuecomment-1937894386)
Concept ACK 91c90d759503aa3fa15b330bba2148698cdc271d.

🚀 hebasto merged a pull request: "Modify command line help to show support for BIP21 URIs"
(https://github.com/bitcoin-core/gui/pull/752)

👍 hebasto approved a pull request: "Fix: Ensure 'Transaction View' remains disabled if no wallet is selected"
(https://github.com/bitcoin-core/gui/pull/780#pullrequestreview-1874449645)
ACK b2e531e70a88f5c9e1c055ae7341520a3128e15d, tested on Ubuntu 22.04.

🚀 hebasto merged a pull request: "Fix: Ensure 'Transaction View' remains disabled if no wallet is selected"
(https://github.com/bitcoin-core/gui/pull/780)

💬 epiccurious commented on pull request "net: make the list of known message types a compile time constant":
(https://github.com/bitcoin/bitcoin/pull/29421#issuecomment-1937914503)
Concept ACK 2e312ea909ae6c466007acf791ea0e3356e954cb.

✅ ariard closed an issue: "Update security.md with all PGP fingerprints"
(https://github.com/bitcoin/bitcoin/issues/29366)

💬 ariard commented on issue "Update security.md with all PGP fingerprints":
(https://github.com/bitcoin/bitcoin/issues/29366#issuecomment-1937917655)
Actually both. Concerned about particular individuals on the core sec list who have lost my trust (i.e some chaincode folks - their intentions are still troubling to me - no private answer from their side to clarify) and vulnerabilities leaking because there are too many people.

One more concern, indelicate individuals using vulnerabilities knowledge to get benefit if any disclosure induce market-moving activities, or even quick change in network mempools congestion. I did voice those issues
...

💬 ariard commented on pull request "Nuke adjusted time from validation (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1937931750)
> Yes, this is exactly what this pull request did. See also the description, which said: "while the warning to users if their clock is out of sync with the rest of the network remains.”

That I know. It was more related to using an external daemon and naively using the same NTP source than your Bitcoin Core instance. There is no such thing as Bitcoin network time, just a sampling of peers nversion time. Yes it works if this external daemon consumes Core warning messages and takes corrective ac
...

💬 maflcko commented on pull request "test: Fix utxo set hash serialisation signedness":
(https://github.com/bitcoin/bitcoin/pull/29399#issuecomment-1938193535)
rfm? :)

✅ willcl-ark closed a pull request: "rpc: add 'getnetmsgstats' RPC"
(https://github.com/bitcoin/bitcoin/pull/28926)

💬 willcl-ark commented on pull request "rpc: add 'getnetmsgstats' RPC":
(https://github.com/bitcoin/bitcoin/pull/28926#issuecomment-1938228531)
> * But what when a peer disconnects? We don't want to bloat `net_stats` with individual info for disconnected peers.

Yes, and in fact if a peer repeatedly connected and disconnected we would end up with unbounded growth of net_stats size, which would not be good... Let's shelve this thought for now.

> To produce the output in https://github.com/bitcoin/bitcoin/pull/28926#issuecomment-1908444454 it is better to use bitcoin-cli getnetmsgstats '["message_type"]'. How to name the RPC paramete
...

👍 maflcko approved a pull request: "mempool: Log added for dumping mempool transactions to disk"
(https://github.com/bitcoin/bitcoin/pull/29402#pullrequestreview-1874894338)
lgtm

💬 maflcko commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#discussion_r1485967010)
nit: While touching this, could switch to LogInfo in this file, to avoid having to touch it yet again?

💬 maflcko commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#discussion_r1485966299)
Not sure about the argument dependent lookup here. Why not simply fix it the way the CI has instructed? (Add the include and use the fs namespace)?

If the CI error was unclear, suggestions to make it clearer are welcome.

💬 maflcko commented on pull request "redeclare nChainTx to use uint64_t":
(https://github.com/bitcoin/bitcoin/pull/29331#issuecomment-1938380704)
Are you still working on this? If not, I am happy to pick up.

💬 glozow commented on issue "Update security.md with all PGP fingerprints":
(https://github.com/bitcoin/bitcoin/issues/29366#issuecomment-1938397734)
> in the past I’ve been asked by LN maintainers to segregate some security information from the usual set of Core people handling issues. There is no need to distribute the information to a wider set of people than strictly necessary (see _need to know policy_).

One potential solution to this problem is to stop CCing unrelated people on your disclosure emails.

✅ maflcko closed a pull request: "test: Add and use option for tx-version in MiniWallet methods"
(https://github.com/bitcoin/bitcoin/pull/28972)

💬 maflcko commented on pull request "test: Add and use option for tx-version in MiniWallet methods":
(https://github.com/bitcoin/bitcoin/pull/28972#issuecomment-1938431311)
Merged as 27c8786ba918a42c860e6a50eaee9fdf56d7c646

💬 dergoegge commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1486013425)
I'm not sure if that is simpler though. Yes it's less code but harder to read/understand imo (e.g. you forgot the `!` in your suggestion).

I'm actually not a fan of my `std::optional<bool>` choice, so I might try to change that.