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

💬 marcofleon commented on pull request "fuzz: increase `txorphan` harness stability":
(https://github.com/bitcoin/bitcoin/pull/30186#issuecomment-2136280587)
Seems like CI still doesn't like it.

I tried this (no space after -i)
`sed -i'' 's/nNextSweep/m_next_sweep/g' $(git grep -l 'nNextSweep')`

and it didn't work on Mac. Adding the `-e` without `.bak` creates new `txorphanage` files with `-e` appended to the end of them. Something like `txorphanage.h-e`. Adding `.bak` prevented that but CI isn't accepting it.

💬 davidgumberg commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#issuecomment-2136391365)
reACK https://github.com/bitcoin/bitcoin/commit/4b7d9842691046b01f0c08d69f924ddb62ccc4c6

Looks great!

Tested with the following diff:

```diff
diff --git a/test/lint/README.md b/test/lint/README.md
index 49ed8356c3..47ac472aa6 100644
--- a/test/lint/README.md
+++ b/test/lint/README.md
@@ -36,7 +36,7 @@ Then you can use:
| [`lint-shell.py`](/test/lint/lint-shell.py) | [ShellCheck](https://github.com/koalaman/shellcheck)
-| [`lint-spelling.py`](/test/lint/lint-spelling.py) | [codes
...

🤔 jonatack reviewed a pull request: "rpc: net: follow-ups for #30062"
(https://github.com/bitcoin/bitcoin/pull/30183#pullrequestreview-2084228631)
Concept ACK, perhaps take https://github.com/bitcoin/bitcoin/pull/30062#discussion_r1612276581 too if you agree. These could be one commit, i.e. `rpc: getrawaddrman "mapped_as" follow-ups`

💬 jonatack commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1618155192)
Perhaps use the -netinfo help doc (here and line 1157 below):

```
Mapped AS (Autonomous System) number in the BGP route to the peer, used for diversifying
peer selection (only displayed if the -asmap config option is set)
```

(the getpeerinfo help below is similar but the -netinfo one seems more complete)

```
The AS in the BGP route to the peer used for diversifying
peer selection (only available if the asmap config flag is set)
```

💬 ajtowns commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2136522127)
utACK 0a89179d62a5b2c5ebc20b806122bb50fc199cba -- no deep review, but being able to mess around with the utxo set as an sqlite dump is handy, and adding a script to contrib seems low risk.

As at height 844861, the raw dump is ~12GB and the sqlite version is ~25GB, so having a bunch of free space is helpful if you want to try it out.