👍 TheCharlatan approved a pull request: "Encapsulate warnings in generalized node::Warnings and remove globals"
(https://github.com/bitcoin/bitcoin/pull/30058#pullrequestreview-2083683221)
Re-ACK 1a5ab1c27bfce12c46c1dcbb0922c8602798b20c

💬 maflcko commented on pull request "depends: Remove Qt build-time dependencies":
(https://github.com/bitcoin/bitcoin/pull/29923#issuecomment-2135959767)
> This PR doesn't change the libraries required to be installed on the user's system

Correct. I tested this in three different boxes and it worked.

![Screenshot from 2024-05-28 21-25-14](https://github.com/bitcoin/bitcoin/assets/6399679/d
...

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1617796685)
Well being able to ad-hoc override individual functions, while still being able to have non-overridden functions be able to refer to it, is sort of the point of CRTP (virtual functions have the same behavior, but CRTP gives it without runtime overhead). If we didn't want this ability, it'd be possible to just have a `template<typename R> class RNGUtility { R m_int; ...}`, where there are low-level `ChaCha20RNG` and `XoRoShiRo128PlusPlusRNG` classes, and high-level `using FastRandomContext = RNG
...

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1617802306)
That would probably work, but that'd be a really invasive workaround build system wise.

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1617811698)
It appeared to be necessary, IIRC.

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1617833166)
Done.

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1617833753)
I think I decided that the compiler should be smart enough to optimize the final "else" case with `Bits == 0` to effectively a `return 0;`, but it's more obvious to just make it explicit. Done.

⚠️ tanzilahmedbd opened an issue: "BDtaka"
(https://github.com/bitcoin/bitcoin/issues/30187)
### Motivation

Received signature iQJGBAABCAAwFiEE0dvyxLlvLev0wWZUQQEIES5+qB8FAmYeYCwSHGhlYmFzdG9A
Z21haWwuY29tAAoJEEEBCBEufqgfOiQP/3xn9Aa/xZq1KaAjMEpLmxYfELmClBIL
/8iPMhyAeEhPnj2JFwpaOf29JPR94KDGz1MCKX7D4Ah1FvdvZuE4kmvnlvb2zqD/
wyk0fDBtDRCUep2MQZ0nHu4pjFuM74IyLEW2X5rAMUPonSmobWjJLWKpgJPVhNyu
WSl9aqUqcFGv4bvvqA4IrySzJno/OfM9qv0XBXlAvNrtPDTxMxzcIiwD4waX0VaV
IZFrYyaM3gaxu6Eh6cKISHe6SeF4rypUfPwY+SJRnoPibgw1JcvhoGMDj2a23mE3
NiPnJgCdUNrDCY8PNc13JdYbJqG280Ux4u7w7cvKA9Ra9h4v8A1VIkanIHPzIXOb
nt9
...

✅ fanquake closed an issue: "BDtaka"
(https://github.com/bitcoin/bitcoin/issues/30187)

:lock: fanquake locked an issue: "BDtaka"
(https://github.com/bitcoin/bitcoin/issues/30187)

💬 Kordestan1993 commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2136041245)
hi thank you very much

در تاریخ سه‌شنبه ۲۸ مهٔ ۲۰۲۴،‏ ۲۳:۰۵ laanwj ***@***.***> نوشت:

> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/util/netif.cpp
> <https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1617802306>:
>
> > +
> +#include <config/bitcoin-config.h> // IWYU pragma: keep
> +
> +#include <util/netif.h>
> +
> +#include <logging.h>
> +#include <netbase.h>
> +#include <util/check.h>
> +#include <util/sock.h>
> +#includ
...

💬 stickies-v commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1617850698)
Oh damn it, I misread your comment. No, it isn't. This is a leftover from an earlier approach. Removed now, thanks.

💬 stickies-v commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#issuecomment-2136046394)
Force pushed to remove [an unnecessary forward declaration](https://github.com/bitcoin/bitcoin/pull/30058#discussion_r1617787010), and also cleaned up some weird `node::Warnings` initializations.

💬 Kordestan1993 commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1617852831)
ok

💬 theuni commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1617859650)
Yes, please don't do that :)

💬 jamesob commented on issue "Pause IBD during AssumeUTXO snapshot load":
(https://github.com/bitcoin/bitcoin/issues/29993#issuecomment-2136081597)
In an earlier version of the code, we would acquire `cs_main` during the duration of snapshot load/activation. Maybe worth going back to that?

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1617913125)
My expectation is that a `#define typeof __typeof__` around just the use of that macro on FreeBSD could fix it. Just haven't been able to try it out.

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1617976395)
> Well, remember that GetNextWorkRequired ignores the nBit value if enough time went by.

That last part isn't true. The first block of the difficulty period does not allow the usage of the 20-min rule. That code is wrapped in the following if statemnt so `GetNextWorkRequired` always passed off to `CalculateNextWorkRequired` for the first block in a new difficulty period.

```
// Only change once per difficulty adjustment interval
if ((pindexLast->nHeight+1) % params.DifficultyAdjustmentIn
...

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1617976451)
done

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1617976516)
right, I removed the redundant check, also because I already rely only on `hashGenesisBlock` in the timewarp code as well.