✅ 1440000bytes closed an issue: "Add wallet RPC to get new taproot address from a silent payment address"
(https://github.com/bitcoin/bitcoin/issues/31258)

👍 danielabrozzoni approved a pull request: "test: enhance p2p_orphan_handling"
(https://github.com/bitcoin/bitcoin/pull/31037#pullrequestreview-2424652363)
ACK 9de9c858d5aa412e1c6086e564fbe85984a4f2d5

🚀 achow101 merged a pull request: "Remove mempoolfullrbf"
(https://github.com/bitcoin/bitcoin/pull/30592)

👋 polespinasa's pull request is ready for review: "rpc: print P2WSH witScript in getrawtransaction"
(https://github.com/bitcoin/bitcoin/pull/31252)

💬 laanwj commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2465676202)
re-ACK 0de3e96e333090548a43e5e870c4cb8941d6baf1

💬 ismaelsadeeq commented on pull request "test: enable running independent functional test sub-tests":
(https://github.com/bitcoin/bitcoin/pull/30991#discussion_r1835024661)
The approach here was added to address [this comment](https://github.com/bitcoin/bitcoin/pull/30991#discussion_r1827274569) cc @rkrux .

Which is an overkill for this simple feature honestly.

Is the simplified approach below preferable?

```python3
def run_custom_tests(self):
for method_name in self.options.test_methods:
self.log.info(f"Attempting to execute method: {method_name}")
method = getattr(self, method_name)
method()
self.log.info(f"Metho
...

💬 ismaelsadeeq commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1835031219)
I've updated the code to use `FeeFrac`, and it simplified things – no need for tuples from the initial approach, just a vector of `FeeFrac` values which achieved same aim.

Smaller diff and no precision loss 💯
I will update all the dependent commits to follow suite.

💬 achow101 commented on pull request "Wallet: (Refactor) GetBalance to calculate used balance":
(https://github.com/bitcoin/bitcoin/pull/29062#discussion_r1835032532)
I don't think it makes sense to return `m_mine_used` without respecting `min_depth`. The original code passes 0 for `min_depth` because that is the default value but it needs to pass the `avoid_reuse` flag. It is not because used balance should always calculated with a min depth of 0. Ignoring it is a layer violation here.

💬 achow101 commented on pull request "Wallet: (Refactor) GetBalance to calculate used balance":
(https://github.com/bitcoin/bitcoin/pull/29062#discussion_r1835037852)
When the `avoid_reuse` flag is passed, `tx_credit_mine` and `tx_credit_mine_used` will be the same so the returned used_balance will be 0. That seems incorrect.

💬 jb55 commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2465720609)
utACK 0de3e96e333090548a43e5e870c4cb8941d6baf1

💬 mzumsande commented on issue "net: Tor service target port collides when running multiple nodes, making bitcoind error out":
(https://github.com/bitcoin/bitcoin/issues/31133#issuecomment-2465734085)
> (yes, I think it is a subset of bind=, somebody correct me if I am wrong)

As far as I understand it, one difference is that `-bind` disables self-discovery of addresses because `connOptions.bind_on_any` will not be set - whereas `-port` will work together with bind-on-any. So if you want to use `Discover()` together with a non-default port, `-port` is the only option.

👍 darosior approved a pull request: "descriptor: Add proper Clone function to miniscript::Node"
(https://github.com/bitcoin/bitcoin/pull/30866#pullrequestreview-2424940909)
ACK 6a00ea17c136e67a66f3558dd2d02a07860b0afe

💬 darosior commented on issue "Listen on random port by default (not 8333)":
(https://github.com/bitcoin/bitcoin/issues/31036#issuecomment-2465795279)
> What about having P2P-seeds, an alternative to DNS-seeds

I guess the biggest drawback with this is how bootstrapping nodes will actually connect to this "P2P-seeds" instead of leveraging the DNS cache of their ISP.

💬 darosior commented on issue "Listen on random port by default (not 8333)":
(https://github.com/bitcoin/bitcoin/issues/31036#issuecomment-2465821538)
Besides the load, which can be dealt with by having a large number of seeds, i wonder how much getting rid of the DNS caching impacts "privacy", loosely defined. Of course from the perspective of a node, if you want to hide your IP you use an anonymizing network period. But it's also the case that being a P2P seed gives you a privileged observation position on all the nodes joining the network, which is less true for DNS seeds since most of the time the bootstrapping nodes would not actually con
...

🤔 hodlinator reviewed a pull request: "Ephemeral Dust"
(https://github.com/bitcoin/bitcoin/pull/30239#pullrequestreview-2420454069)
Concept ACK 131bed19bdfc922328cad9781fa9122b6810a8fd

Thanks for working on this! Seems like an obvious win for L2s.

Haven't yet absorbed the full context sufficiently to fully A-C-K it myself. Hopefully my comments here and above provide value even though most are surface level. My personal priority with this review is to get it in better shape for others so they can A-C-K more easily if they agree.

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832718055)
```suggestion
// so the last transaction to be generated (in a >1 package) must spend all package-made outputs
```

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832702717)
Only exercising v2 - maybe you could describe the reasoning behind the test at the top of fuzz target?

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832453654)
(Just curious - this is a very common pattern, why do we leave it up to the fuzz engine instead of just doing:
```suggestion
LIMITED_WHILE(true, 300)
```
Does it help define discrete sections of the fuzz data, which ends up being useful somehow?

These kind of uses make more sense to me: `LIMITED_WHILE(provider.remaining_bytes() > 0, iter_limit)`, `LIMITED_WHILE(!available_coins.empty(), 500)`.)

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832376920)
"Also"? But in this case the sweep-transaction survives the reorg, unlike the case above.

💬 hodlinator commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832400954)
Why 25 and not 1 block?