💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2473228711)
Addressed latest feedback from @maflcko

👍 vasild approved a pull request: "ci: skip Github CI on branch pushes for forks"
(https://github.com/bitcoin/bitcoin/pull/30487#pullrequestreview-2432802018)
ACK 8610bcef9d030013f9e36cffe0c58dd2cfe85d66

👍 vasild approved a pull request: "addrman: cap the `max_pct` to not exceed the maximum number of addresses"
(https://github.com/bitcoin/bitcoin/pull/31235#pullrequestreview-2432819520)
ACK 9c5775c331e02dab06c78ecbb58488542d16dda7

💬 polespinasa commented on pull request "rpc: print P2WSH witScript in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1840096537)
I agree, it needs to be changed in one more place apart from this line but yes I will apply it.
Thanks!

💬 naumenkogs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1840103944)
Expanding on this thought, I sketched a two-step improvement in handling single-package tx wrt code readability. Wanna [take a look](https://github.com/naumenkogs/bitcoin/commits/2024-10-submitpackage-singleton-v1/)?

I certainly might be missing something.

👍 rkrux approved a pull request: "test: Rework wallet_migration.py to use previous releases"
(https://github.com/bitcoin/bitcoin/pull/31248#pullrequestreview-2432399968)
tACK a76ad56a80d9c9a60352bb98b363131e359a383b

Certainly useful because it reflects how the end users would do wallet migration after the upcoming release.

Make is successful but around 15 functional tests are failing in my system that seem unrelated to this change because they are failing in master as well. All of the failures are due to timeouts, probably an issue with my setup.

Asked few questions and left suggestions. Found few really good tests in this file in the end!

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839858333)
Should this not be called `create_legacy_wallet_and_get_rpc` now highlighting that it returns the wallet RPC from the old node?

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839822834)
Not in the diff but still

```diff
- # Now migrate and test that we still see have the same balance/transactions
+ # Now migrate and test that we still have the same balance/transactions
```

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839812117)
Nit
```
# restart master node and verify that everything is still there
```

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839933824)
This `notloaded` wallet migration testing is removed because it is effectively tested in `test_unloaded_by_path` below?

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839954334)
+1 for adding this. Is the reason for the absence of this _unsetting_ here not causing any issue earlier when `wallet.setmocktime(curr_time)` was added that no subsequent test in this file below relies on mock time?

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840121466)
I have noticed this pattern in most of these tests that I found a little difficult to read. Slightly more verbose naming would make it easier to parse like below. I am not suggesting this change to be done in this PR because this pattern existed earlier as well but can be updated in the future if others find this helpful as well.

```diff
- def_wallet.sendtoaddress(wallet.getnewaddress(), 10)
+ def_descriptor_wallet.sendtoaddress(legacy_wallet.getnewaddress(), 10)
```

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840131032)
Would prefer to have the commit message mention this instead of `use previous releases`, which gives an impression multiple previous releases are being used. Couple suggestions:

```diff
- test: Rework migratewallet to use previous releases
+ test: Rework migratewallet to use previous release
+ test: Rework migratewallet to use v28 release
```

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1840003618)
TIL!

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839877158)
Don't we need to check for these in the old node?

✅ fanquake closed an issue: "fuzz: connman target: terminate called after throwing an instance of 'std::bad_alloc'"
(https://github.com/bitcoin/bitcoin/issues/31234)

🚀 fanquake merged a pull request: "addrman: cap the `max_pct` to not exceed the maximum number of addresses"
(https://github.com/bitcoin/bitcoin/pull/31235)

📝 Sjors opened a pull request: "Add waitNext() to BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/31283)
Alternative approach to #31003, suggested in https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2451942362.

This PR introduces `waitNext()`. It waits for either the tip to update or for fees at the top of the mempool to rise sufficiently. It then returns a new template, with which the caller can rinse and repeat.

TODO:
- [ ] add tests or use it in existing RPC methods

💬 Sjors commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#issuecomment-2473480206)
I'll close this PR if #31283 gets enough concept ACKs.

💬 Sjors commented on issue "Mining Interface doesn't allow for Bitcoin Core to create blocks when it wants":
(https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2473482653)
See #31283.