💬 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

💬 theStack commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825278134)
follow-up idea: could take use of that helper in `mempool_package_rbf.py` (might be a nice "good first issue")

💬 theStack commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825284831)
is this change needed? seems to also pass with the dust value

💬 theStack commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825279952)
follow-up idea: could put this in a helper for creating an ephemeral package (`dusty_tx` + `sweep_tx`), as that's a repeated pattern in many sub-tests; might make sense to parametrize with tx version, number of dust outputs and optional additional sponsors

💬 kevkevinpal commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2451089685)
Not yet sure on this PR we already have `priority_level` for benchmarks

All the benchmarks being removed seem to be `benchmark::PriorityLevel::HIGH`
not sure if it makes sense to move them down a level?

> This frequently comes up with new contributors and they can't be blamed for thinking that e.g. hex parsing should be made faster if a benchmark test exists.

I understand this is one of the motivations for this change does this happen often for benchmarks that are marked `benchmark::P
...

⚠️ Sandra-Amina-Boss opened an issue: "Cosigner"
(https://github.com/bitcoin/bitcoin/issues/31201)
Zpub6yQaKv5t4M6wxzXVE7GFFLxuA3Zcj6g1baUMr48krvf3VqiZbXzQ6ab6EUT9HwUyCDzU2CJ9u2znpDpbRRQvBfeaF6HHroAUGZryqyZBzuD

✅ achow101 closed an issue: "."
(https://github.com/bitcoin/bitcoin/issues/31201)

:lock: achow101 locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/31201)

👍 tdb3 approved a pull request: "test: added test to assert TX decode rpc error on submitpackage rpc"
(https://github.com/bitcoin/bitcoin/pull/31139#pullrequestreview-2409329609)
ACK d7fd766feb2f579bdba0e778bacdeb13103e8282

Looks good, thanks.
Corecheck.dev shows the added coverage for mempool.cpp.
Ran a few quick sanity checks.

💬 tdb3 commented on pull request "test: added test to assert TX decode rpc error on submitpackage rpc":
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1825331169)
Induced failure with:
```diff
- hex_list = [valid_tx_list[0]["hex"][:-1] + 'X', valid_tx_list[1]["hex"]]
+ hex_list = [valid_tx_list[0]["hex"], valid_tx_list[1]["hex"]]
```
Also verified that the `not in node.getrawmempool()` asserts also fail, e.g.
```diff
self.log.info("Submitpackage only allows valid hex inputs")
valid_tx_list = self.wallet.create_self_transfer_chain(chain_length=2)
- hex_list = [valid_tx_list[0]["hex"][:-1] + 'X', valid_tx_list
...