🤔 w0xlt reviewed a pull request: "test: reduce runtime of p2p_opportunistic_1p1c.py"
(https://github.com/bitcoin/bitcoin/pull/33048#pullrequestreview-3053254178)
ACK https://github.com/bitcoin/bitcoin/pull/33048/commits/eb65f57f319dc4e2ea8c83cf7e283c36f1c0d53b

On my machine, master: 43 s
This PR: 27 s

Even reducing the number of transactions and peer connections used in the DoS portions of the test, it still fills the orphan pool past its limit, ensuring that evictions occur.
The use of mock time also makes the test more deterministic.

💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2229559648)
We can have multiple keys if the key is multipath.

💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2229563232)
External signer always has private keys disabled.

💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2229564365)
This call to `Parse` has `require_checksum` set to `false` so the checksum is not required.

💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2229572215)
Done

💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2229572501)
Done

💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2229572728)
Done

💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2229572849)
Done

💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2229573142)
Done

💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2229573405)
Removed

💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2229573665)
Fixed

💬 maflcko commented on pull request "test: Add and use ElapseTime helper":
(https://github.com/bitcoin/bitcoin/pull/32430#issuecomment-3114952384)
(rebased)

💬 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