💬 maflcko commented on pull request "util: Abort on failing CHECK_NONFATAL in debug builds":
(https://github.com/bitcoin/bitcoin/pull/32588#discussion_r2229576705)
> make it 101% accurate

thx, done

💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229612292)
The blank flag needs to be unset before `EncryptWallet` so that `EncryptWallet` can generate the new keys after encrypting. Otherwise, the newly created wallet will be encrypted but blank, which is not what we want in this scenario.

💬 w0xlt commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2229614994)
```suggestion
if (nLoadWalletRet != DBErrors::LOAD_OK) {
const auto wallet_file = m_database->Filename();

switch (nLoadWalletRet) {
case DBErrors::CORRUPT:
error = strprintf(_("Error loading %s: Wallet corrupted"), wallet_file);
break;
case DBErrors::NONCRITICAL_ERROR:
warnings.push_back(strprintf(_("Error reading %s! All keys read correctly, but transaction data"

...

💬 yancyribbens commented on pull request "test: add assertions to SRD max weight test":
(https://github.com/bitcoin/bitcoin/pull/33058#issuecomment-3115059948)
Oh I see, SRD generates a different solution each time in the test-suit. That's annoying.

🤔 mzumsande reviewed a pull request: "Cache m_cached_finished_ibd where SetTip is called."
(https://github.com/bitcoin/bitcoin/pull/32885#pullrequestreview-3053316434)
> The function is only interesting when it can latch to the IBD finished state.
>
> That's only possible when all four conditions are met, which can only happen when the tip is updated.
>
> The final time based condition can only change when the tip changes as it gets further away with time, not closer.

I think that @luke-jr is right. If we reindex, we set `m_importing` to true in `ImportBlocks`, so any blocks we connect there can never result in getting out of IBD due to the `m_blockma
...

💬 mzumsande commented on pull request "Cache m_cached_finished_ibd where SetTip is called.":
(https://github.com/bitcoin/bitcoin/pull/32885#discussion_r2229609944)
since the function is annotated with `EXCLUSIVE_LOCKS_REQUIRED(cs_main)` anyway, why not put it to the beginning of the function, as it is done in most other places?

💬 mzumsande commented on pull request "Cache m_cached_finished_ibd where SetTip is called.":
(https://github.com/bitcoin/bitcoin/pull/32885#discussion_r2229577797)
First I thought this was not necessary because disconnecting a block shouldn't usually get you out of IBD, but it guess there are edge cases (starting up, with the old tip having a lower timestamp than it's parent block) where this could lead to get us out of IBD?!

💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646189)
Done

💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646353)
Made `void`

💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646469)
Done

💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646537)
Done

💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646683)
I think leaving them here is fine.

💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646764)
Done

💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646853)
Done

💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#discussion_r2229646929)
Done

💬 achow101 commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#issuecomment-3115094134)
ACK ec0b90e40e21ccb97c57373f7a7665ab9554acec

In a sample size of 1, this seems to provide a significant improvement in IBD time. It's surprising to me that it looks like this would have had benefit around the time that Taproot activated as previous measurements hadn't indicated that.

💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2229667232)
I've pulled in your suggestion here.

💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2229670245)
Not sure if it makes a measurable difference in performance, but I'm happy to use your version. Updated.

💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-3115124995)
Updated per review feedback (thanks!)

💬 achow101 commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3115210492)
Also seeing @mzumsande's observation if I add the addresses to loopback, but not if I add them to a different interface.

I think it's reasonable to filter out all loopback addresses, so the resolution is probably to just update the instructions to have the addresses bound to a non-loopback interface.