💬 juanmigdr commented on pull request "Add rpcestimateconservativefees":
(https://github.com/bitcoin/bitcoin/pull/32329#issuecomment-2824142411)
> > Bitcoin Core v28.0 changed the default fee estimation mode for `estimatesmartfee` from _conservative_ to _economical_ ([[#30275](https://github.com/bitcoin/bitcoin/pull/30275)](#30275)). While this change reduces fee overestimation for most users, it may not be ideal in all environments.
>
> You can easily opt in to the `conservative` mode by passing a parameter to `estimatesmartfee`.
>
> > In my case, I maintain a long-running node (since v24.0) with multiple clients depending on the
...

🚀 hebasto merged a pull request: "ci: switch to LLVM 20 in tidy job"
(https://github.com/bitcoin/bitcoin/pull/32226)

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2055969055)
It is functionality that can be activated by the user/CI. Not having the code would leave that case in more of a broken state.

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2055973404)
Can we cover the paths in the tests as well?

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2055986974)
We could definitely test more parts of the framework functionality in another PR. This PR is focused on keeping the error reporting clear for startup failures, to aid troubleshooting.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2824202867)
`2b34857ad5...1c16944a4a`: address suggestions

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2055989720)
Done

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2055990031)
Done

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2055990265)
Done

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2055990539)
Done

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2055992009)
Done, reworded the comment.

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2055994881)
But this PR introduced the dead code

💬 saikiran57 commented on pull request "wallet: removed duplicate call to GetDescriptorScriptPubKeyMan":
(https://github.com/bitcoin/bitcoin/pull/32023#issuecomment-2824224700)
Hi @furszy could you please finalize this review and merge this.

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2056007024)
https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2055969055

💬 saikiran57 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#issuecomment-2824233359)
Hi All @achow101 @furszy @mprenditore @maflcko @w0xlt @davidrobinsonau request to re-review this PR and provide ACK.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2056022026)
Yes, this loose handling makes it possible to do the job with less code. My reasoning is that the worst outcome from the race is acceptable.

I am looking into keeping track of more stuff with respect to the future RPC stats (see at the bottom of https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2042286270) which would also give us more fine grained control here. If it looks reasonable and is not too much code, I will propose the change in https://github.com/bitcoin/bitcoin/pull/29415#
...

💬 darosior commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2056025288)
Good catch, thanks. I should have re-checked with my follow-up PR which introduces validation before pushing the change for the fuzz target. Will do that this time.

💬 fanquake commented on pull request "refactor: Avoid copies by using const references or by move-construction":
(https://github.com/bitcoin/bitcoin/pull/31650#issuecomment-2824264910)
Re-ran tidy as it looks like 20 will pickup more issues: https://cirrus-ci.com/task/5041587878625280?logs=ci#L1916:
```bash
[09:08:35.465] /ci_container_base/src/rpc/util.h:527:50: error: parameter 'pow_limit' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls,-warnings-as-errors]
[09:08:35.465] 527 | uint256 GetTarget(const CBlockIndex& blockindex, const uint256 pow_limit);
`
...

💬 hodlinator commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2056032514)
> you mean we usually ignore the folder when sorting?

Think it's a case of keeping our headers separate from external dependencies/libraries.

💬 hodlinator commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2056049582)
I think it might make sense to introduce our own version of `clamp` that includes such an `assert` since the standard library has no guarantees if cppreference is to be believed. (Or just start off with an `assert`/`Assume` before the call for now).