👍 brunoerg approved a pull request: "fuzz: Limit wallet_notifications iterations"
(https://github.com/bitcoin/bitcoin/pull/31238#pullrequestreview-2419524919)
code review ACK fa461d7a43aa5a321278c8f734e80fb7aa79bfdb

💬 laanwj commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1831772618)
> WDYT about creating a constant tuple for these values? They seem pretty generic enough.

are they needed anywhere else? in #31222 i suggested
```python
MIN_PAGESIZE = 512
MAX_PAGESIZE = 65536
⋮
assert pagesize >= MIN_PAGESIZE and pagesize <= MAX_PAGESIZE and (pagesize & (pagesize-1) == 0)
```
but honestly, enumerating all 8 possible values here seems fine

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2460946238)
> Using the same public key that can be used to verify the final signature (therefore, the tweaked taproot pubkey for keypath spends, or the pubkey as it appears in tapleaves for script spends) seems the most natural choice to me.

That's a good point, I don't feel too strongly about this, it was just a bit more annoying to figure out how. I've updated the PR to do that.

Also opened https://github.com/bitcoin/bips/pull/1695 to clarify in the BIP.

💬 kanzure commented on pull request "doc: Update the developer mailing list address.":
(https://github.com/bitcoin/bitcoin/pull/29782#issuecomment-2460974096)
> For future reference, in case we decide to update the archive links cited in other places of the codebase, here is the mapping:

Unfortunately, lists.linuxfoundation.org is no longer hosting any mailing list archives, not just for bitcoin-dev but all the other mailing lists too.

Are there any particular preferences on doing that proposed replace-all update now that lists.linuxfoundation.org has pulled the content?

I see a few options here:

1) Wait and see if they put it back up.


...

💬 fjahr commented on issue "Remove libevent as a dependency (HTTP / cli / torcontrol)":
(https://github.com/bitcoin/bitcoin/issues/31194#issuecomment-2460987257)
Concept ACK on removing libevent and replacing the HTTP server with our own code. FWIW, I have been working on the removal of libevent from torcontrol but I still see some errors that I need to debug. I hope I can open a PR soon.

🤔 hodlinator reviewed a pull request: "Ephemeral Dust"
(https://github.com/bitcoin/bitcoin/pull/30239#pullrequestreview-2414994647)
Code reviewed up until most of aa7c90a91adcf8ad9d6b94d5d17797cfd0eb5228.

Disclaimer: I posses far from full context when it comes to validation code, but at least as far as I can tell we are only modifying policy, not consensus.

PR title should probably have "p2p:" or some [other prefix](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#creating-the-pull-request).

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1829045123)
nit: Missing word
```suggestion
# Dust is 1 satoshi since create_self_transfer_multi disallows 0
```

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1829048604)
nit: I like the transaction being called "dusty" as it creates dust but is not itself dust. Dust refers only to outputs, not to transactions. Would prefer `dusty_txid`.

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831063128)
nit: Since we unconditionally insert at the end of the loop, we might as well `insert` up here instead and check if it already existed in the set.
```suggestion
if (!processed_parent_set.insert(parent_txid).second) continue;
```

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831120099)
There's an edge-case where `expected` accidentally contains duplicate `tx` instances (since it's a list, not a set), meaning they might exist in the mempool, and the lengths may be equal, but the mempool may contain some other txs.

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831104732)
While it is good to limit dust in the UTXO-set, I'm not sure adding this limitation is wise.

If we want mining pools to use as un-patched versions of Bitcoin Core as possible, we probably shouldn't try to limit what they can do with this RPC.

IMO this commit d147d929f5093f71c39cb7efbb0dd3888f6c8114 should be split out to its own PR.

(My argument is weakened by the fact that this function already requires the transaction to have made it into the mempool in the first place).

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831655381)
More precise:
```suggestion
// Enforces 0-fee for transactions with dust outputs, no incentive to be mined alone
```

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831720019)
All `test_`-methods except `sponsor_cycle` have implementations in the order they are called here, please reorder the call or where in the file the implementation appears.

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831776816)
Slightly easier to follow comments?
```suggestion
# Setup valid sweep in mempool
sweep_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=dusty_tx["new_utxos"], version=3)
self.nodes[0].submitpackage([dusty_tx["hex"], sweep_tx["hex"]])
assert_mempool_contents(self, self.nodes[0], expected=[dusty_tx["tx"], sweep_tx["tx"]])

# RBF-attempt fails when not spending in-mempool dust output from parent
unspent_sweep_tx = self.wallet.create_
...

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831750845)
nit: Less backwards?
```suggestion
def test_parent_with_fee(self):
```

💬 vasild commented on pull request "fuzz: fix bad alloc in connman target":
(https://github.com/bitcoin/bitcoin/pull/31235#issuecomment-2461406655)
I think that the problem warrants a fix in the real code, not in the test. `AddrManImpl::GetAddr_()` would try to overallocate if the caller passes > 100% which is meaningless. It already handles `max_addresses` properly: `nNodes = std::min(nNodes, max_addresses);`. If there are 1000 addresses and the caller asks for 5000, it would clamp it down to 1000.

<details>
<summary>[patch] clamp down the % of addresses, like already done for `max_addresses`</summary>

```diff
diff --git i/src/addr
...

👍 theStack approved a pull request: "TxDownloadManager followups"
(https://github.com/bitcoin/bitcoin/pull/31190#pullrequestreview-2420051892)
ACK 5dc94d13d419b8d5e543cb50edeb872335c090e7

👍 Joj501 approved a pull request: "TxDownloadManager followups"
(https://github.com/bitcoin/bitcoin/pull/31190#pullrequestreview-2420077863)
Connect with my wallet address

🤔 theStack reviewed a pull request: "net: Use actual memory size in receive buffer accounting"
(https://github.com/bitcoin/bitcoin/pull/31164#pullrequestreview-2420152018)
post-merge ACK d22a234ed270286b483aec2db1e2f716b9756231

💬 vasild commented on issue "Remove libevent as a dependency (HTTP / cli / torcontrol)":
(https://github.com/bitcoin/bitcoin/issues/31194#issuecomment-2461527876)
> I have been working on the removal of libevent from torcontrol but I still see some errors that I need to debug. I hope I can open a PR soon.

:heart:

poke me for review