💬 fjahr commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#issuecomment-1954281008)
tACK bf5662c678455fb47c402f8520215726ddfe7a94

💬 maflcko commented on pull request "lint: Check for missing bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1954281330)
Added the check to error on redundant (harmless) includes as well, since it was trivial. Should be trivial to re-ACK.

💬 maflcko commented on pull request "lint: Check for missing bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1954286731)
Rebased, for silent merge conflict in `src/bench/wallet_ismine.cpp`. I guess this immediately shows that the check is useful :)

💬 fanquake commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1954289542)
@theuni want to rebase this now that #29404 is in?

💬 sipa commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1495884531)
Since this "in case of non-coinbase first transaction, treat existence of 64-byte transaction as malleation" is new behavior, would it make sense to move it to a separate commit at the end of the PR? (A clearer comment would also be helpful, because I briefly thought this was a consensus change before it was pointed out to be it only triggers in case the block would already be invalid).

💬 Sjors commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1495894984)
Moving to `bip352.h` sounds good too.

✅ so7ow closed an issue: "When selecting a custom data directory on first launch of Bitcoin Core GUI on Mac, where is that setting stored?"
(https://github.com/bitcoin-core/gui/issues/798)

💬 so7ow commented on issue "When selecting a custom data directory on first launch of Bitcoin Core GUI on Mac, where is that setting stored?":
(https://github.com/bitcoin-core/gui/issues/798#issuecomment-1954310379)
Thank you! Confirmed this works:

```
rm ~/Library/Preferences/org.bitcoin.Bitcoin-Qt.plist
killall -u YourUserName cfprefsd
```

💬 instagibbs commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1495954173)
I also had the same misconception the first time I reviewed.

💬 fjahr commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1495983754)
CI failed on this line: https://cirrus-ci.com/task/5549867297144832?logs=ci#L2880

📝 paplorinc opened a pull request: "benchmark/speedup: optimize TryParseHex calls considerably"
(https://github.com/bitcoin/bitcoin/pull/29458)

💬 fjahr commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1954438224)
Concept ACK, looks good at first sight, will do a deeper review when it's clear if the CI failure is an issue. I couldn't reproduce it locally so far...

💬 maflcko commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1954448961)
To reproduce locally, you'll have to compile with the `integer` sanitizer (not to be confused by the `undefined` sanitizer)

💬 ryanofsky commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1954455499)
CI failure is just a preexisting bug in a line of code that doesn't affect anything:

https://github.com/bitcoin/bitcoin/blob/45b2a91897ca8f4a3e0c1adcfb30cf570971da4e/src/rpc/blockchain.cpp#L1658

Two `nChainTx` values in the `getchaintxstats` are subtracted and this triggers the undefined behavior sanitizer because they are unsigned and the result is negative. `getchaintxstats` has a number of edge case problems like this that should be cleaned up but don't really affect anything. I'll prob
...

💬 ryanofsky commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1954457409)
> To reproduce locally, you'll have to compile with the `integer` sanitizer (not to be confused by the `undefined` sanitizer)

Thanks! I was just trying to reproduce this locally using the undefined sanitizer.

💬 fanquake commented on pull request "build: replace custom `MAC_OSX` macro with standard `__APPLE__` for consistent macOS detection":
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-1954461873)
This is `MAC_OSX` because that is what we actually mean, and using our own define gives us more control. We produce release binaries for macOS running on darwin hardware, not just any "`__APPLE__` target", which, for example, would include iOS running on darwin hardware (i.e `arm64e-apple-ios`).

Swapping to just use `__APPLE__` would mean it's no-longer as easy to differentiate the two. At this point, that may no-longer matter, but there is code in this codebase that is relevant for macOS, bu
...

✅ fanquake closed a pull request: "refactor: improve readability of numeric literals in consensus parameters and network settings"
(https://github.com/bitcoin/bitcoin/pull/29444)

💬 fanquake commented on pull request "refactor: improve readability of numeric literals in consensus parameters and network settings":
(https://github.com/bitcoin/bitcoin/pull/29444#issuecomment-1954471769)
Thanks, however we don't generally refactor consensus code for the sake of readability, so we'll leave this as-is for now.

✅ fanquake closed a pull request: "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes-maybe-malware.us"
(https://github.com/bitcoin/bitcoin/pull/29145)

💬 fanquake commented on pull request "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes-maybe-malware.us":
(https://github.com/bitcoin/bitcoin/pull/29145#issuecomment-1954477676)
Given it's been more than a month, and there's no more followup here, I'm going to close this for now. Feel free to ping for a re-open, if/when you've picked a more suitable domain name.