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

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1617976575)
I will keep it here for now unless other reviewers think it should be moved.

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1617976889)
taken with minor edit

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1617976968)
Sounds good, but keeping this for a follow-up.

💬 luke-jr commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1617978145)
Last I checked, we require more than just a flag?

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2136251685)
I'm pouring one out for all the tACKs we've lost but the rebase was necessary.

I have addressed the comments from @Sjors and I think those were all that are in scope for this PR here. Mostly it's adding comments and a few


I think the chain replay idea from @TheBlueMatt is probably best tracked in a separate issue. Potentially there are already projects out there that can provide the necessary functionality, I am not aware of anything like that though.

👍 theStack approved a pull request: "functional test: ensure confirmed utxo being sourced for 2nd chain"
(https://github.com/bitcoin/bitcoin/pull/29998#pullrequestreview-2083992790)
ACK 07aba8dd215b23b06853b1a9fe04ac8b08f62932

💬 brunoerg commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1617990003)
Just need to set the flag with a valid asmap file.

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2136268587)
The CI failure doesn't seem related, somehow the test-each-commit job was instantly cancelled.