💬 waketraindev commented on pull request "net, rpc, qt: add per-peer blocktxn stats to getpeerinfo and qt debug window":
(https://github.com/bitcoin/bitcoin/pull/32732#issuecomment-2965894128)
Thanks for the feedback, makes sense. I use this in my node monitoring tooling. Closed.

💬 maflcko commented on pull request "rpc: Use type-safe exception to pass RPC help":
(https://github.com/bitcoin/bitcoin/pull/32660#discussion_r2142211515)
done style-nit. Should be easy to re-review via `git range-diff bitcoin-core/master fa0cf42380 fa946520d2`.

📝 fanquake opened a pull request: "[28.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/32735)
This backports
* https://github.com/bitcoin/bitcoin/commit/3656b828dc2204418974e94928cc8d915b10ed95 - Which was missed in #32563, see https://github.com/bitcoin/bitcoin/pull/32563#issuecomment-2960237938.

This will also backport #32693, to fix CMake `4.x` compat with depends.

💬 fanquake commented on pull request "[28.x] Backport #31407":
(https://github.com/bitcoin/bitcoin/pull/32563#issuecomment-2965912930)
Thanks, addressed in #32735.

💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2142257380)
Done

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

Reapplied the tests from https://github.com/bitcoin/bitcoin/pull/32660#issuecomment-2939092052 - passes without the changes, fails with the change as expected!

<details>
<summary>the diff compared to previous ack is indeed only the rebase and the exception rename + commit message update</summary>

> git fetch origin fa946520d229ae45b30519bccc9eaa2c47b4a093 fa0cf42380dec17e73d38aa69dd70662a0bc344a
> git range-diff master fa0cf42380dec1
...

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142250207)
Nice Idea, this lead me to think about doing
```cpp
const int32_t p95_weight = 0.95 * total_weight;
int32_t total_package_size = std::accumulate(
package_feerates.begin(),
package_feerates.end(),
0,
[](int32_t sum, const FeeFrac& f) {
return sum + f.size;
}
);
// Return empty percentiles if the total packages weight is less than the the 95th percentile weight.
if (total_package_size < p95_weight) return Percentil
...

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142219150)
I think it it correct we also use it to decide which percentile fee rate to return to user in mempool forecaster.

50th or 75th percentile fee rate.

The main idea is that conservative fee rate is for users who are willing to pay a bit higher to increase likelihood of confirmation; because they don't want to fee bump.

Since all packages can be fee bumped now (due to full rbf).

I rephrased this to
"If true, returns a higher fee rate for greater confirmation probability." let me know h
...

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142268856)
I think [doc guideline](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c) say camel case, so I switched to that.

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142269615)
done

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142269814)
Taken

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142270436)
Nice catch, fixed.

💬 willcl-ark commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142273192)
Ok that makes sense to me

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142274723)
The defaults round down I think, not sure if there is any potential difference by rounding up.
So will leave as is.

👍 rkrux approved a pull request: "wallet: Fix wallet interface detection of encrypted wallets"
(https://github.com/bitcoin/bitcoin/pull/32620#pullrequestreview-2920415710)
utACK 130a922980778b293b22169d5e5649afde3ba33b

In #31250, the check was added for `require_format` to be equal to `BERKELEY_RO`, which occurs only in migration, so for encrypted legacy wallets to be migrated from the GUI this format fallback is required. The migration flow from the RPC accepts the passphrase argument for encrypted wallets, and this `isEncrypted` function is not called from there.

💬 rkrux commented on pull request "wallet: Fix wallet interface detection of encrypted wallets":
(https://github.com/bitcoin/bitcoin/pull/32620#discussion_r2142270773)
While adding this `BERKELEY_RO` database format fallback in this seemingly generic `isEncrypted` function looks unsafe, it is fine because the only usage of this function lies in the wallet migration functionality in the GUI. The `FAILED_LEGACY_DISABLED` error from `MakeDatabase` also enforces the flow towards wallet migration.

Using the `require_format = DatabaseFormat::BERKELEY_RO` checks seems like an indirect way of validating the migration flow though when the `MakeWalletDatabase` funct
...

💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-2966042888)
Rebased df0eb65bd98f300618cccd667720f1ccc6145865 -> 969799291d34da706b43209ba1209038ec349424 ([spendblock_5](https://github.com/TheCharlatan/bitcoin/tree/spendblock_5) -> [spendblock_6](https://github.com/TheCharlatan/bitcoin/tree/spendblock_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_5..spendblock_6))

* Fixed conflict with #32673
* Fixed conflict with #32421

💬 PeterWrighten commented on pull request "wallet: Allow read-only database access for info and dump commands":
(https://github.com/bitcoin/bitcoin/pull/32685#discussion_r2142290203)
You mean we should throw an exception before get a return from `sqlite3_exec` ?

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142292007)


I don't think users should configure this. It should be determined by the forecaster, based on the potential increment of the top block in the mempool.

Previously, I researched how long it takes for a transaction to finish propagating through the network. @sr-gi responded with an estimate of approximately 7 seconds:
https://bitcoin.stackexchange.com/questions/125776/how-long-does-it-take-for-a-transaction-to-propagate-through-the-network.

I thought I had updated this accordingly, but
...

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142292170)
Done

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142297543)
Not sure whether that a good idea for ASAP-mempool Forecaster.
But can good if you want something like https://github.com/FelixWeis/WhatTheFee--legacy
So far I've not experimented with whatTheFee yet to determine its correctness.