💬 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.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1840190668)
Separate from this PR, I'm looking into whether we can avoid passing `script_pub_key` around. Afaik it's only used by tests and for solo CPU mining.

💬 Sjors commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#discussion_r1840192430)
#31283 returns a block template

💬 Sjors commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#discussion_r1840192936)
#31283 returns a block template

💬 fanquake commented on pull request "guix: scope pkg-config to Linux only":
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2473494323)
Guix build (aarch64)
```bash
a9e1f628dd64913209e6f55f183453b046c91e44fb959ea61e0c40c0a8457e98 guix-build-2833befb7822/output/aarch64-linux-gnu/SHA256SUMS.part
d082fbc96abb72a4bb0495833b6361d1ab61f8eefc3423571589920ed6c89678 guix-build-2833befb7822/output/aarch64-linux-gnu/bitcoin-2833befb7822-aarch64-linux-gnu-debug.tar.gz
6d94622d87e86ceef908e2b05435ac7f0e4b7f3d224454f619155e44c36128b9 guix-build-2833befb7822/output/aarch64-linux-gnu/bitcoin-2833befb7822-aarch64-linux-gnu.tar.gz
ca28805
...

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1840195105)
I still need to study https://github.com/bitcoin/bitcoin/pull/31003#discussion_r1792221843 in more detail to see if this code isn't making similar mistakes.

🤔 BrandonOdiwuor reviewed a pull request: "build: Switch to Qt 6"
(https://github.com/bitcoin/bitcoin/pull/30997#pullrequestreview-2433034814)
Concept ACK

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1840257211)
Not sure what you mean, seems you are arguing in the reverse direction or talking past?

I'm thinking the main use-case is for a miner or pool to increase priority of a transaction due to them getting paid out-of-band. While that is bad from a centralization aspect, having such a facility in unpatched versions of Bitcoin Core means miners/pools have an easier time keeping their nodes up-to-date (less moving parts), meaning they are more responsive to our releases.

💬 sipa commented on pull request "validation: Remove RECENT_CONSENSUS_CHANGE validation result":
(https://github.com/bitcoin/bitcoin/pull/31269#issuecomment-2473610234)
ACK e80e4c6ff91e27d7d40f099a2d7942c29085234c

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1840280274)
Ah, totally missed that. Cheers!

Not sure what they add, it's not like there other transactions in the way. I guess they test that nothing funky is going on with the prioritization-logic resulting in dusty transactions being accepted when they shouldn't be.

💬 TheCharlatan commented on pull request "refactor: Make node_id a const& in RemoveBlockRequest":
(https://github.com/bitcoin/bitcoin/pull/31282#issuecomment-2473659364)
Looks like iwyu is still complaining about some of the includes:
```
[08:23:24.120] (/ci_container_base/src/net_processing.h has correct #includes/fwd-decls)
[08:23:24.120]
[08:23:24.120] /ci_container_base/src/net_processing.cpp should add these lines:
[08:23:24.120]
[08:23:24.120] /ci_container_base/src/net_processing.cpp should remove these lines:
[08:23:24.120] - #include <deploymentstatus.h> // lines 21-21
[08:23:24.120] - #include <node/warnings.h> // lines 39-39
```

💬 maflcko commented on pull request "refactor: Make node_id a const& in RemoveBlockRequest":
(https://github.com/bitcoin/bitcoin/pull/31282#issuecomment-2473688687)
> Looks like iwyu is still complaining about some of the includes:

That's an upstream bug. Maybe a separate issue can be used to track those?