💬 brunoerg commented on pull request "descriptor: check whitespace in pubkeys within fragments":
(https://github.com/bitcoin/bitcoin/pull/31603#issuecomment-2593855664)
Force-pushed simplifying the approach. Now it just checks the whitespace in the `ParsePubkeyInner` function.

💬 pablomartin4btc commented on pull request "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection":
(https://github.com/bitcoin-core/gui/pull/815#issuecomment-2593858698)
Updates:
- Rebased (fixed previous CI failure - _ASan + LSan + UBSan..._ ).

💬 ryanofsky commented on pull request "wallet: Translate [default wallet] string in progress messages":
(https://github.com/bitcoin/bitcoin/pull/31296#issuecomment-2593868257)
Rebased ebb77ab77df4081f27ce55e8b28b0d35990caac8 -> a46ec1dece8d8a1b16ff1fc3d2932bca130bf67f ([`pr/dtran.2`](https://github.com/ryanofsky/bitcoin/commits/pr/dtran.2) -> [`pr/dtran.3`](https://github.com/ryanofsky/bitcoin/commits/pr/dtran.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/dtran.2-rebase..pr/dtran.3)) due to conflict with #31061

💬 pablomartin4btc commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-2593886226)
Updates:
- Rebased (fixed previous CI failure - _previous releases, depends DEBUG..._ ).

👍 ryanofsky approved a pull request: "kernel: Move kernel-related cache constants to kernel cache"
(https://github.com/bitcoin/bitcoin/pull/31483#pullrequestreview-2553888604)
Code review ACK 2a92702bafca5c78b270a9502a22cb9deac02cfc. Changes since last review are fixing size options to use size_t instead of int64_t again, simplifying CheckedLeftShift more, and making other minor suggested cleanups

💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#issuecomment-2593891189)
@theuni

Thank you for your review! Your comments have been addressed.

💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1917307456)
Thanks! [Done](https://github.com/bitcoin/bitcoin/pull/31662#issuecomment-2593891189).

💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1917309058)
The `LDFLAGS` option has been [added](https://github.com/bitcoin/bitcoin/pull/31662#issuecomment-2593891189).

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2593925274)
Thanks for the suggestions @laanwj,

Rebased f157b0cbc7d90075858a6522d13a7bc4f0b25a5f -> f25616bec485ee6a70e4b797758d4987d25a7c25 ([kernelApi_14](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_14) -> [kernelApi_15](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_15), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_14..kernelApi_15))

* Fixed conflict with #31061

Updated f25616bec485ee6a70e4b797758d4987d25a7c25 -> 4dde75858a3b08f84d71176c7be14bae62020b1f
...

🤔 jonatack reviewed a pull request: "test: add mocked Sock that can read/write custom data and/or CNetMessages"
(https://github.com/bitcoin/bitcoin/pull/30205#pullrequestreview-2554013096)
re-ACK b448b014947093cd217dbde47c8fb9e6c2bc8ba3

🤔 darosior reviewed a pull request: "test: Add mockable steady clock, tests for PCP and NATPMP implementations"
(https://github.com/bitcoin/bitcoin/pull/31022#pullrequestreview-2553493851)
Some more nits i noticed when reusing some of this code for the fuzz target. I don't think they warrant retouching, but figured i'd post them anyways.

💬 darosior commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1917096066)
nit in the context of this test but i think you meant:
```suggestion
if (service.SetSockAddr(sa, sa_len)) {
// Can only bind to one of our local ips
if (!service.IsBindAny() && service != m_local_ip) {
return -1;
}
m_bound = service;
return 0;
}
return -1;
```

💬 darosior commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1917096949)
nit
```suggestion
assert(false && "Move of Sock into PCPTestSock not allowed.");
```

💬 darosior commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1917105856)
nit: it's never used, any reason for defining it?

💬 darosior commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1917273199)
Isn't this redundant with `~Sock`?

💬 darosior commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1917172067)
nit
```suggestion
0x02, 0x81, 0x00, 0x42, // version, opcode, result error
```

👍 ryanofsky approved a pull request: "rpc: add gettarget , target getmininginfo field and show next block info"
(https://github.com/bitcoin/bitcoin/pull/31583#pullrequestreview-2553957182)
Code review ACK 712e3ffde5fa4fd26c0f217942b4a289cf37f361. Main change since last review is mining_testnet.py test being replaced by mining_mainnet.py test with a much smaller data file. It was also rebased and there were small documentation changes and string format tweak in mining_mainnet.py

💬 ryanofsky commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1917334042)
In commit "Use OP_0 for BIP34 padding in signet and tests" (75342f7e19cbbae083c2a3561f2c2cefd4e2968a)

Do we know any reason why this code was using OP_1 instead of OP_0 previously? Any possible advantages to using OP_1? It looks like this code was introduced in #16363

💬 ryanofsky commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1917359516)
In commit "test: check difficulty adjustment using alternate mainnet" (a70f967bda5a0998a2b5a851cd32d969b021df15)

This seems ok, but I think I don't understand why it is useful to test that coins are spendable. It seems like something besides the point of the test (at least as stated in the description), which is to test difficulty retargeting functions on the main chain that can't be tested on the regtest chain. I wonder if this is spending test is checking something that existing tests are n
...

📝 ismaelsadeeq opened a pull request: "Fees: add Fee rate Forecaster Manager"
(https://github.com/bitcoin/bitcoin/pull/31664)
- This PR implements the core component of #30392, introducing a new fee rate forecasting module.
The primary addition is a `ForecasterManager` that coordinates multiple forecasting strategies to be able to provide better transaction fee rate predictions.


### Key Changes

1. **Addition of Fee Rate Forecasting Utility Structures**
- `Forecaster` abstract class, serving as the base class for all fee rate forecasters.
- `ForecastResult` (the response from a `Forecaster`) with a
...