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

💬 kilrau commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1938459985)
> Plus, a default Replace-By-Fee will just make BTC harder to use as a transactional currency

That's part of the goal. (Micro-) transactional use cases should use Layer 2.

Anyhow, it doesn't matter. I can only speak for [Boltz](https://boltz.exchange/), but for us this train has left the station. Only one pool accepting fullrbf and it's not safe to accept 0-conf anymore. Game over. It took a while to grow on me, give it some time.

ACK

🤔 hebasto reviewed a pull request: "Intro: Have user choose assumevalid"
(https://github.com/bitcoin-core/gui/pull/149#pullrequestreview-1875020927)
Approach ACK cf940f0e5f5ffb14ec7b6f751879bd70b3c350be.

This branch has to be rebased due to the conflicts with the master branch.

💬 hebasto commented on pull request "QRImageWidget: Sizing and font fixups":
(https://github.com/bitcoin-core/gui/pull/506#issuecomment-1938496294)
Closing due to lack of support from the PR author (https://github.com/bitcoin-core/gui/pull/506#issuecomment-1549832808, https://github.com/bitcoin-core/gui/pull/506#pullrequestreview-1630119899).

✅ hebasto closed a pull request: "QRImageWidget: Sizing and font fixups"
(https://github.com/bitcoin-core/gui/pull/506)

💬 Sjors commented on issue "Add taproot descriptor to existing descriptor wallet":
(https://github.com/bitcoin/bitcoin/issues/24193#issuecomment-1938499238)
Which is superseeded by #29130

✅ Sjors closed an issue: "More coin stats (for dumpcoinstats & gettxoutsetinfo)"
(https://github.com/bitcoin/bitcoin/issues/24581)

💬 Sjors commented on issue "More coin stats (for dumpcoinstats & gettxoutsetinfo)":
(https://github.com/bitcoin/bitcoin/issues/24581#issuecomment-1938500792)
Not much happening here. Can reopen is anyone has a wish list.

👍 hebasto approved a pull request: "Intro: Never change the prune checkbox after the user has touched it"
(https://github.com/bitcoin-core/gui/pull/658#pullrequestreview-1875086606)
ACK bee0ffbecf4f95b65f4084893924a1ab250ca77c, both commits are improvements of the current behaviour. Tested on Ubuntu 23.10.