💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2109790109)
extra fn doesn't seem worth it
```
diff --git a/src/node/txorphanage_impl.h b/src/node/txorphanage_impl.h
index 89974ec506..b6137845b4 100644
--- a/src/node/txorphanage_impl.h
+++ b/src/node/txorphanage_impl.h
@@ -205,11 +205,11 @@ class TxOrphanageImpl
}

- /** Return number of announcements with the same wtxid as it. */
- unsigned int CountSameWtxid(Iter<ByWtxid> it) const
+ /** Return number of announcements with this wtxid. */
+ unsigned int CountWtxid(const Wtx
...

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2112585455)
PeerMap is unused circa 455b4b817884f860cd4467f0a9be4a459e89891c

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2112167760)
737c5127df841e9c8037b1885284f80b0aba17dd

might be good to note in commit message the getorphantxs is experimental, so breaking is considered ok

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2112547493)
455b4b817884f860cd4467f0a9be4a459e89891c

nit: unsure if "uses" section adds a lot to clarity vs reading the code

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2112572392)
not sure what result_type is doing here or in ByPeerViewExtractor

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2112597837)
nit: s/PeerInfo/PeerDoSInfo/

💬 achow101 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2112813722)
In 0341e021fe6edfa21252f4bd93b9e1a209383891 "Implement rescan stop with timestamp as never for import descriptors"

Please use `snake_case` for new variables. Additionally, when the type is not annoying to type out, please give the full type rather than use `auto`.

Same for `requestTimestamp` below.

💬 achow101 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2112814780)
In 0341e021fe6edfa21252f4bd93b9e1a209383891 "Implement rescan stop with timestamp as never for import descriptors"

nit: space between `now` and `:`

```suggestion
const int64_t timestamp = requestTimestamp < 0 ? now : requestTimestamp;
```

💬 achow101 commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#issuecomment-2917732757)
fe0432b1c4a10b74844c2dedefccfe340c0d3b10

💬 achow101 commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#discussion_r2112856923)
```suggestion
"Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only.\n"
```

💬 achow101 commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#discussion_r2112856236)
Comma should be a period.

```suggestion
"if the transaction has unconfirmed inputs. This is because the wallet will attempt to make the\n"
```

🤔 w0xlt reviewed a pull request: "wallet: init, don't error out when loading legacy wallets"
(https://github.com/bitcoin/bitcoin/pull/32449#pullrequestreview-2876603197)
ACK https://github.com/bitcoin/bitcoin/pull/32449/commits/86e1111239cdb39dd32cfb5178653c608fa30515

nit: Perhaps the comment in `HandleWalletError` could also explain why `DatabaseStatus::FAILED_LEGACY_DISABLED` is also being mapped to `RPC_WALLET_NOT_FOUND`. While it seems clear from looking at this PR, it might be a useful for future reference.

⚠️ harryvik990 opened an issue: "My phone f"
(https://github.com/bitcoin/bitcoin/issues/32635)

✅ fanquake closed an issue: "My phone f"
(https://github.com/bitcoin/bitcoin/issues/32635)

💬 achow101 commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#issuecomment-2917784928)
ACK f6517df210f5e940d87823c86358976743de2641

💬 achow101 commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2917799485)
> OP_RETURN fallback if no address or descriptor set is provided.

Why? I think it would be simpler to just require that at least one output address/descriptor is specified.

💬 achow101 commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2112898425)
Descriptors are accepted too.

💬 achow101 commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2112898168)
Although this RPC is only used by tests, I don't think we should be breaking backwards compatibility here. It's equally easy to determine whether this parameter is a string, so we can accept a string here for backwards compatibility.

The reason to keep compatibility is because this RPC has a high likelihood of being used in scripts and automated tests. Changing the type of the parameter will break all of those.

🤔 1440000bytes reviewed a pull request: "log: Additional compact block logging"
(https://github.com/bitcoin/bitcoin/pull/32582#pullrequestreview-2876724595)
ACK 83df64d7491b8271f7dfa2aea30f055102e3ff39

<details>
<summary>Signature</summary>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 83df64d7491b8271f7dfa2aea30f055102e3ff39
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQSSUwYqT5LWNFkIXKYtIwUgISpZAQUCaDekrAAKCRAtIwUgISpZ
Aa86AP0cOFEVIMWCd7bhNekXwpmdgIVlMV9LdU2YQ40r6TdZJQEA8hVtCHmD2sMN
pkHDM0gALAeez7FyJQplX2HYYqh/tgU=
=ldiS
-----END PGP SIGNATURE-----

</details>

🤔 w0xlt reviewed a pull request: "wallet, rpc: Return normalized descriptor in parent_descs"
(https://github.com/bitcoin/bitcoin/pull/32594#pullrequestreview-2876745493)
ACK https://github.com/bitcoin/bitcoin/pull/32594/commits/fdabfc7b01ec2cff5f2356e1b13b5537d30dbaf8