💬 pstratem commented on pull request "Cache m_cached_finished_ibd where SetTip is called.":
(https://github.com/bitcoin/bitcoin/pull/32885#issuecomment-3046915168)
> Conceptually not a bad idea to cache and lock less, but imo this makes the code more brittle (and harder to understand), e.g. if any tip updates happen without the cache being updated separately.
>
> Do you have any data as to the actual performance improvements from this PR?

I'm (very) open to suggestions on how to make the caching call more robust. (Indeed I expected some.)

There's no performance improvement from this PR, it's the first in a series of proposed changes I'll be making
...

💬 achow101 commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#issuecomment-3046915670)
ACK 4207d9bf823bea9f94b484ebf3c9274ca781b245

🚀 achow101 merged a pull request: "test: fix feature_init.py intermittencies"
(https://github.com/bitcoin/bitcoin/pull/32835)

💬 achow101 commented on pull request "rpc: Use type-safe exception to pass RPC help":
(https://github.com/bitcoin/bitcoin/pull/32660#issuecomment-3046955303)
ACK fa946520d229ae45b30519bccc9eaa2c47b4a093

💬 achow101 commented on pull request "rpc: use CScheduler for relocking wallet and remove RPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32862#issuecomment-3046962563)
ACK fcfd3db563e89fd79820a4cdfa102d624d801de1

🤔 furszy reviewed a pull request: "wallet: remove dead code in legacy wallet migration"
(https://github.com/bitcoin/bitcoin/pull/32758#pullrequestreview-2995635591)
code reviewed, looks good. Just left a comment about the last commit.

💬 furszy commented on pull request "wallet: remove dead code in legacy wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32758#discussion_r2191246827)
In 2b8b658308c8fbfd9d41d8eb7b9491b615c43872:
Based on the commit description, this change seems more like you reasoning about the code flow than something that benefits the reader? I'm not sure it's adding much value.

💬 furszy commented on issue "intermittent issue in feature_init.py (bitcoind should have exited with expected error LevelDB error: Corruption)":
(https://github.com/bitcoin/bitcoin/issues/32600#issuecomment-3046966568)
can close it.

💬 pstratem commented on pull request "Cache m_cached_finished_ibd where SetTip is called.":
(https://github.com/bitcoin/bitcoin/pull/32885#issuecomment-3046981384)
> I think something like this, if properly implemented (I haven't thought much about the code yet), would reduce the GUI freezes during IBD in a noticeable manner.

I hadn't even considered that, but certainly that's a possible direct improvement.

✅ achow101 closed an issue: "intermittent issue in feature_init.py (bitcoind should have exited with expected error LevelDB error: Corruption)"
(https://github.com/bitcoin/bitcoin/issues/32600)

🚀 achow101 merged a pull request: "rpc: Use type-safe exception to pass RPC help"
(https://github.com/bitcoin/bitcoin/pull/32660)

🚀 achow101 merged a pull request: "rpc: use CScheduler for relocking wallet and remove RPCTimer"
(https://github.com/bitcoin/bitcoin/pull/32862)

💬 pablomartin4btc commented on pull request "wallet: remove dead code in legacy wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32758#discussion_r2191271514)
I don’t have a strong preference either way, but I agree, the loading failure is part of the broader migration process. Also, if we (ever) later refactor the migration logic into a separate component, the current comment might no longer reflect the actual flow, making it prone to becoming outdated.

💬 HowHsu commented on pull request "index: remove unnecessary locater cleaning in BaseIndex::Init()":
(https://github.com/bitcoin/bitcoin/pull/32882#discussion_r2191340453)
I tend to favor the principle of adding things only when they're actually needed.

💬 ryanofsky commented on pull request "POC: IPC tracing interface":
(https://github.com/bitcoin/bitcoin/pull/32898#issuecomment-3047160262)
Notes on current implementation of this:

- The third commit f880817d54d9558938c697a37d0ef9acd4f34cf4 would be a lot simpler if it added a `Traces* g_traces` or similar global variable, which would be a reasonable thing to do. Initially, I wanted to avoid globals because they are hard to get rid of, and avoiding them could have benefits like being able to attach different tracers to different `CCoinsViewCache` instances, but this may not be worth extra complexity.
- In the main commit 8e09fbe
...

💬 HowHsu commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3047190022)
> furszy

But like furszy said, the Sync() in test uses mutex and conditional variables to sync with the main thread, while the real Sync() in bitcoind changes as times goes by.

💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2191468854)
Done! Added you as co-author on that commit.

💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#issuecomment-3047392949)
<ins>_Updates_</ins>:
- Addressed feedback from @maflcko (co-authored): extended `MaybeArg` to return a `std::optional<std::string>` for use in the `GetWalletNameFromJSONRPCRequest` overload. Also refactored the `migratewallet` handling of `wallet_name` to align with the existing behavior in `unloadwallet`.
- Incorporated @furszy's suggestion (co-authored): added a new template in RPC utils to check whether all provided parameters are null or empty, now used in the `getdescriptoractivity` RPC.
...

💬 sean-k1 commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#issuecomment-3047454052)
@achow101 @maflcko
PTAL

💬 l0rinc commented on pull request "Cache m_cached_finished_ibd where SetTip is called.":
(https://github.com/bitcoin/bitcoin/pull/32885#issuecomment-3047465141)
@pstratem, not sure if you saw this, but could be helpful: https://github.com/bitcoin/bitcoin/pull/25081