💬 ryanofsky 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-2450648747)
Maybe a flexible way to support this would be to add wait options to the `createNewBlock()` method so instead of creating a new block template right away, it could optionally wait for conditions to be reached. Those conditions could be set by the client or determined by the node.

Something like:

<details><summary>diff</summary>
<p>

```diff
--- a/src/interfaces/mining.h
+++ b/src/interfaces/mining.h
@@ -90,9 +90,13 @@ public:
*
* @param[in] script_pub_key the coinbase o
...

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1825058258)
ok so I think I was a bit wrong, these values were being used and would "blind" us potentially from catching the "BUG! Mempool ancestors or descendants were underestimated" error.

💬 laanwj commented on issue "Gracefully handle dropped UPnP support ":
(https://github.com/bitcoin-core/gui/issues/843#issuecomment-2450652369)
> An easy first fix would be to downgrade InitError to InitWarning, as the alert shouldn't be fatal, as it is possible to continue without it. (This applies to bitcoind as well)

Do we also need to explicitly remote the entry from settings.json as well, to make sure it doesn't appear at every start after that. Or does this happen automatically in re-saving it somehow?

⚠️ m3dwards opened an issue: "CI: Improve documentation around replicating CI locally"
(https://github.com/bitcoin/bitcoin/issues/31199)
I've received some feedback that replicating CI jobs locally isn't straightforward. Perhaps the documentation can be improved in this area?

CC @maflcko

⚠️ m3dwards opened an issue: "CI: Make failure message easier to spot"
(https://github.com/bitcoin/bitcoin/issues/31200)
Sometimes during a CI failure the actual error message can be buried in a wall of log output and not immediately clear. Ideally it should be obvious why a CI job has failed.

CC @maflcko

💬 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-2450797714)
@ryanofsky that approach makes sense to me. In that case perhaps `waitFeesChanged()` won't be needed as its own method #31003? That has a nice side-effect in the current implementation of letting us return the last generated block template (I hope eventually we can get the next-block-fees without generating templates at all).

💬 darosior commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825202486)
Yes, as far as i can tell it's an artifact of switching between protocols. I should remove it. In fact i think `g_mapport_enabled` can be removed as well as we can just use `g_mapport_interrupt` to stop it.

📝 l0rinc converted_to_draft a pull request: "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`"
(https://github.com/bitcoin/bitcoin/pull/31144)
The recently merged [XOR obfuscation](https://github.com/bitcoin/bitcoin/pull/28207) is a non-trivial part of serialization, encountered during IBD and `reindex`.

In this PR, I've added a new test (comparing randomized inputs against a trivial implementation), updated the benchmark to validate more realistic scenarios (collected every call of `util::Xor` for the first 400k blocks and modelled the benchmark to have the same distribution of data) and optimized the Xor function to do the work in
...

👍 tdb3 approved a pull request: "init: warn, don't error, when '-upnp' is set"
(https://github.com/bitcoin/bitcoin/pull/31198#pullrequestreview-2409213226)
ACK a1b3ccae4be82297fd20f5be15a03eeb477507d0

Sanity check below.

On master (f07a533dfcb172321972e5afb3b38a4bd24edb87), prevents start:


PR branch (a1b3ccae4be82297fd20f5be15a03eeb477507d0), doesn't:

💬 darosior commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825257255)
Done.

💬 davidgumberg commented on pull request "build: Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to fuzz":
(https://github.com/bitcoin/bitcoin/pull/31191#issuecomment-2450980584)
Tested ACK

This solves https://github.com/bitcoin/bitcoin/issues/31178

`./build/src/bench/bench_bitcoin -filter=LinearizeOptimallyExample11 -min-time=30000`

|branch| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|------------|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
|master (f0
...

💬 darosior commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1825258139)
Done for both `current` and `enabled`.

💬 kevkevinpal commented on pull request "init: warn, don't error, when '-upnp' is set":
(https://github.com/bitcoin/bitcoin/pull/31198#issuecomment-2451022302)
Concept ACK [a1b3cca](https://github.com/bitcoin/bitcoin/pull/31198/commits/a1b3ccae4be82297fd20f5be15a03eeb477507d0)

makes sense to give a warning instead of giving an error which is bad ux for a user who has been using `-upnp`

🤔 theStack reviewed a pull request: "Ephemeral Dust"
(https://github.com/bitcoin/bitcoin/pull/30239#pullrequestreview-2328771065)
Concept ACK

Mostly looked and reasoned about the core commit 92d7ad2b135668cb7f6b0fd32be15fd6dd2f915c so far. From what I understand the "only one dust output" limit would not be strictly necessary and is far less important than the other two rules (must be 0-fee, child must spend all dust from the parent) in order to disisincentivize creating dust UTXOs, or am I missing something?

What I left below are largely nits and follow-up ideas, feel free to ignore. Still want to look deeper into u
...

💬 theStack commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825270642)
nit: calling the RPC via the `.rpc` member shouldn't be needed? (here and below)
```suggestion
dust_txid = self.nodes[0].sendrawtransaction(hexstring=dusty_tx["hex"], maxfeerate=0)
```

💬 theStack commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825269811)
```suggestion
the dust is both created and spent simultaneously.
```

💬 theStack commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825272593)
whitespace nit
```suggestion
#include <policy/ephemeral_policy.h>
#include <policy/policy.h>
```

💬 theStack commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825267562)
could avoid the duplicate map lookup:
```suggestion
if (auto it = map_txid_ref.find(parent_txid); it != map_txid_ref.end()) {
parent_ref = it->second;
```
(though it probably doesn't really matter that much as `map_txid_ref` won't have more than two entries currently; also in light of https://github.com/bitcoin/bitcoin/pull/30239/commits/92d7ad2b135668cb7f6b0fd32be15fd6dd2f915c#r1816952259)

💬 theStack commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825276484)
```suggestion
// Non-0 fee dust transactions are not allowed for entry, and modification not allowed afterwards
```

💬 theStack commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825273227)
potential follow-up nit: could create a helper like `HasDustOutputs` (or `IsDusty`) and use that here and in the `prioritisetransaction` RPC