💬 theStack commented on issue "Use muhash for assumeUTXO snapshot ":
(https://github.com/bitcoin/bitcoin/issues/27669#issuecomment-1550522541)
Had similar thoughts that using muhash instead would be nice in order to avoid the chain rollback for AssumeUTXO hash reviewers. Not sure if this is a huge problem, but one funny side-effect of this would be that for a chainstate with _n_ coins, there are in theory _n!_ different serializations possible that all would pass as valid (compared to just one for `hash_serialized_2`), since the order of coins doesn't matter for muhash.

> For good measure we should also modify `gettxoutsetinfo` so
...

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1550595711)
> ACK [6b605b9](https://github.com/bitcoin/bitcoin/commit/6b605b91c1faf2c7f7cc0c9d39b4fcfd66dc2965) modulo `miniminer_overlap` test.
>
> Not really blocking, I'm planning to go deeper later. And probably add some explanatory comments and code simplifications. I think that has a readability barrier that will be a maintenance issue moving forward.

Perhaps we can address that in #26152 as well

💬 Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#issuecomment-1550602703)
Rebased on #27021, need to rebase on master next, then will incorporate the follow-up nits from #27021

🤔 ishaanam reviewed a pull request: "wallet: improve IBD sync time by skipping block scanning prior birth time"
(https://github.com/bitcoin/bitcoin/pull/27469#pullrequestreview-1429618571)
Approach ACK, this looks good so far but I haven't reviewed the first commit yet.

💬 ishaanam commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1195808323)
nit:
```suggestion
/** Updates wallet birth time if 'new_birth_time' is lower than m_birth_time*/
```

💬 ishaanam commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1195809999)
Why is this check also done here if it is already done in `FirstKeyTimeChanged`?

💬 ajtowns commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1195879972)
I mostly renamed it to make sure there'd be compiler errors anywhere that it didn't get updated, just in case some type conversion happened automatically.

💬 ajtowns commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1195881028)
Yeah, this is broken as-is. My original idea was to add a timestamp to the `mapRelay` entries too, then it was to drop `mapRelay` and add `vExtraTxnForCompact`-relay, then I compromised by just doing it wrong...

💬 MarcoFalke commented on pull request "Descriptor unit tests and simplifications":
(https://github.com/bitcoin/bitcoin/pull/24361#issuecomment-1550798295)
> Drafted for now given it's (maybe) waiting on https://github.com/bitcoin/bitcoin/pull/26076.

That one was merged last week.

💬 MarcoFalke commented on issue "Mac osx 12.6.5 ":
(https://github.com/bitcoin/bitcoin/issues/27681#issuecomment-1550799971)
Please check your debug.log for possible causes; Alternatively you can upload it here.

You can find the debug.log in your [data dir](https://github.com/bitcoin/bitcoin/blob/master/doc/files.md#data-directory-location).

Please be aware that the debug log might contain personally identifying information.

💬 MarcoFalke commented on issue "Can't compile v24.0.1":
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1550801604)
Closing for now. Let us know if you have any other questions.

✅ MarcoFalke closed an issue: "Can't compile v24.0.1"
(https://github.com/bitcoin/bitcoin/issues/27680)

💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1196058973)
nit: for the next ~~node~~test

💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1550888614)
Code review ACK c79539fc20d902b5f9e098cc996c4e27c8e5b8c5

I plan to test this manually a bit later

💬 hebasto commented on issue "Mac osx 12.6.5 ":
(https://github.com/bitcoin/bitcoin/issues/27681#issuecomment-1550921209)
FWIW, Bitcoin Core v22.0 has reached its EOL (see https://bitcoincore.org/en/lifecycle/).

Suggesting to [download](https://bitcoincore.org/en/download/) a new supported version.

📝 MarcoFalke opened a pull request: " build: Bump minimum Clang to clang-10 "
(https://github.com/bitcoin/bitcoin/pull/27682)
It doesn't make sense to support a minimum clang version that is difficult to install on all supported operating systems, which generally ship a later version:

* Ubuntu Focal 20.04: https://packages.ubuntu.com/focal/clang-10 and https://packages.ubuntu.com/focal/clang-12
* Debian Bullseye: https://packages.debian.org/bullseye/clang-11
* CentOS 8 Stream: All Clang versions from 11.0 to 15.0

Also, it allows to drop build code, which means it won't waste review when rolling over into cmake
...

💬 hebasto commented on pull request "[WIP] rebase: Call ProcessNewBlock() asynchronously":
(https://github.com/bitcoin/bitcoin/pull/18963#issuecomment-1550945493)
From my research it follows that the "[Move BlockChecked to a background thread](https://github.com/bitcoin/bitcoin/pull/18963/commits/2ed0b647435b80946f7f000d573ff4b4e274ca70)" commit fixes a lock-order-inversion in the MessageHandler thread.

See https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1514928912.

💬 hebasto commented on issue "Replace all of the RecursiveMutex instances with the Mutex ones":
(https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1550947349)
> Steps to reproduce on a fresh install of Fedora 38:

[Can](https://github.com/bitcoin/bitcoin/pull/18963#issuecomment-1550945493) be fixed by https://github.com/bitcoin/bitcoin/pull/18963.

💬 Sjors commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#issuecomment-1550948362)
I guess a simpler way to phrase it is: what can go wrong if c14ae132c5a5204a9a755c84c6de05fb30459221 picks the incorrect chainstate for a block? What are all the different ways in which we can receive a block? What do we expect to happen in each of these cases? Tests would be a really nice way to illustrate that, but are apparently a pain to write for net_processing.

💬 hebasto commented on pull request "build: Bump minimum supported Clang to clang-10":
(https://github.com/bitcoin/bitcoin/pull/27682#issuecomment-1550949496)
> Also, it allows to drop build code, which means it won't waste review when rolling over into cmake (`cmake/module/CheckStdFilesystem.cmake`).

Concept ACK.