💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2310705314)
Alright then, dropping `g_best_block` as well!

Some tests failed with `EnsureAnyNodeContext(context).notifications`, so I fixed that (relative to the above patches). In `init.cpp` the RPC server starts at step 4a, but the notifications are added at step 7. This case is hit by several functional tests that shut the node down early.

📝 Prabhat1308 opened a pull request: "rpc: Add test-only RPCs under `-test=<option>` flag"
(https://github.com/bitcoin/bitcoin/pull/30717)
followup of #29007 , implements adding of test-only RPC options in `-test` as discussed [here](https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1417082950)

Currently a general user has access to `test-only` RPCs , where user might accidentally use it in production. This PR splits off test-only RPCs to be included in `-test` flag with additional restrictions of use in `regtest` segregating the user facing and testing RPCs . These test-only RPCs are split off from args and have their o
...

🤔 hebasto reviewed a pull request: "bench: [refactor] iwyu"
(https://github.com/bitcoin/bitcoin/pull/30716#pullrequestreview-2261245718)
I still see IWYU warnings in https://cirrus-ci.com/task/5307830952001536. Here are some of them:

```
bench/addrman.cpp should add these lines:
#include "netaddress.h" // for CService, CNetAddr, Network
#include "protocol.h" // for CAddress, ServiceFlags
```
```
bench/bech32.cpp should remove these lines:
- #include <cstdint> // lines 9-9
```
```
bench/bench.h should add these lines:
#include "nanobench.h" // for Bench (ptr only)
```
```
bench/data.cpp should add th
...

💬 murchandamus commented on pull request "rpc: Add test-only RPCs under `-test=<option>` flag":
(https://github.com/bitcoin/bitcoin/pull/30717#discussion_r1731615240)
From what I understand, the startup options `deprecatedrpc`, `stopatheight`, `limitancestorcount`, `limitancestorsize`, `limitdescendantcount`, and `limitdescendantsize` are definitely used in production by some users.

🤔 murchandamus reviewed a pull request: "rpc: Add test-only RPCs under `-test=<option>` flag"
(https://github.com/bitcoin/bitcoin/pull/30717#pullrequestreview-2261289148)
Looking at the discussion in #29007, I had the impression that it was suggested to move some RPC options to a "test-only" grouping, so I’m a bit surprised that this PR also moves a number of startup options to "test-only".

I’m wondering whether there was a misunderstanding regarding "RPC options" and "startup options" here.

💬 martinsaposnic commented on pull request "Use MiniWallet in functional test rpc_signrawtransactionwithkey.":
(https://github.com/bitcoin/bitcoin/pull/30701#issuecomment-2310776625)
@ismaelsadeeq rebased and pushed. thanks for the review!

📝 theStack opened a pull request: "test: switch MiniWallet padding unit from weight to vsize "
(https://github.com/bitcoin/bitcoin/pull/30718)
This PR is a late follow-up for #30162, where I retrospectively consider the padding unit of choice as a mistake. The weight unit is merely a consensus rule detail and is largely irrelevant from a user's perspective w.r.t. fee-rate calculations and mempool policy rules (e.g. for package relay and TRUC limits), so there doesn't seem to be any value of using a granularity that we can't even guarantee to reach exactly anyway.

Switch to the more natural unit of vsize instead, which simplifies bot
...

💬 Prabhat1308 commented on pull request "rpc: Add test-only RPCs under `-test=<option>` flag":
(https://github.com/bitcoin/bitcoin/pull/30717#discussion_r1731625165)
In the arguments description in `init.cpp` they are categorised as `DEBUG_ONLY` AND `DEBUG_TEST` which is where I picked them up to be "test-only". There was not much documentation elsewhere to decide otherwise.

💬 murchandamus commented on pull request "rpc: Add test-only RPCs under `-test=<option>` flag":
(https://github.com/bitcoin/bitcoin/pull/30717#discussion_r1731628845)
I see. What’s the overarching intent here: is the purpose just to label them more explicitly as being for testing, or is the intent to restrict their use to test networks?

👍 jaonoctus approved a pull request: "seeds: Pull additional nodes from my seeder and update fixed seeds"
(https://github.com/bitcoin/bitcoin/pull/30008#pullrequestreview-2261317336)
utACK

💬 Prabhat1308 commented on pull request "rpc: Add test-only RPCs under `-test=<option>` flag":
(https://github.com/bitcoin/bitcoin/pull/30717#discussion_r1731633356)
We are restricting its use in test networks only specifically to `regtest` only . the `-test` flag has a check to assert use of `regtest`.

💬 achow101 commented on pull request "test: XORed blocks test follow up":
(https://github.com/bitcoin/bitcoin/pull/30669#issuecomment-2310802609)
ACK e1d5dd732d5dc641faf1dde316275c84b6bb224b

💬 achow101 commented on pull request "test: Add time-timewarp-attack boundary cases":
(https://github.com/bitcoin/bitcoin/pull/30698#issuecomment-2310813586)
ACK 31378d44f44cabc576bf92ddd61afde4b528de77

Going to merge this now in the interest of having test coverage of the boundary before the 28.x branch off.

🚀 achow101 merged a pull request: "test: XORed blocks test follow up"
(https://github.com/bitcoin/bitcoin/pull/30669)

🤔 hodlinator requested changes to a pull request: "test: Improve clarity of subsidy limit test"
(https://github.com/bitcoin/bitcoin/pull/30699#pullrequestreview-2261348316)
Think a possible way forward would be to undo most changes to `subsidy_limit_test`.
* The change from `2099999997690000` to using `'`-separators is nice. Have you considered the slightly more BTC-native `20'999'999'9769'0000` or `20'999'999'97'690'000`? (Only found some vague precedence for the former in `util_ParseMoney` test case and none for the latter).
* As I already [mentioned](https://github.com/bitcoin/bitcoin/pull/30699#pullrequestreview-2255459494), I think it would be an improvement
...

🚀 achow101 merged a pull request: "test: Add time-timewarp-attack boundary cases"
(https://github.com/bitcoin/bitcoin/pull/30698)

💬 murchandamus commented on pull request "rpc: Add test-only RPCs under `-test=<option>` flag":
(https://github.com/bitcoin/bitcoin/pull/30717#discussion_r1731663858)
Sorry, I have to withdraw `limitancestorcount`, `limitancestorsize`, `limitdescendantcount`, and `limitdescendantsize`. Both parties that I suspected to use these startup options, in fact, do not use them. We definitely used `stopatheight` at a prior employer while indexing the blockchain, and my understanding was that `deprecatedrpc` was meant to give users a heads-up that an RPC would be going away with the next release, but not completely requiring an immediate transitioning away from it.

💬 achow101 commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#issuecomment-2310853293)
ACK 1b41d45d462d856a9d0b44ae0039bbb2cd78407c

💬 achow101 commented on pull request "devtools, utxo-snapshot: Fix block height out of range in script":
(https://github.com/bitcoin/bitcoin/pull/30690#issuecomment-2310859905)
ACK 5b4f34006dbd76223b55b156421f87d2241ac296

🚀 achow101 merged a pull request: "devtools, utxo-snapshot: Fix block height out of range in script"
(https://github.com/bitcoin/bitcoin/pull/30690)