💬 theStack commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205543956)
nitty nit: for consistency, this should probably be done after every successful `AddTx` call (e.g. also in the next loop below), though it doesn't seem to be necessary for now

💬 theStack commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205568179)
in d5097d6eef85eab971352cf78fb94a30c6e6f127: nit: could use a structured binding for getting rid of many `.first/.second` accesses (that I often find confusing) and enhancing the readability, e.g.:
```suggestion
auto [new_announcement_it, inserted] = m_orphans.get<ByWtxid>().emplace(tx, peer, m_current_sequence);
```
(here and in many other return values of `.emplace` calls)

💬 theStack commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205557613)
in d5097d6eef85eab971352cf78fb94a30c6e6f127: nit: the naming seems outdated, considering that (in contrast to master) the map's value type is not an iterator anymore?
```suggestion
std::unordered_map<COutPoint, std::set<Wtxid>, SaltedOutpointHasher> m_outpoint_to_orphan_wtxids;
```

💬 theStack commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205574325)
in d5097d6eef85eab971352cf78fb94a30c6e6f127: nit: should this also called "latency score" here for consistency, or is the different naming intentional?

💬 theStack commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205570142)
in d5097d6eef85eab971352cf78fb94a30c6e6f127: nit: maybe also call it `..._usage` for consistency?

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3070627590)
Ping :)

📝 glozow opened a pull request: "validation: docs and cleanups for MemPoolAccept coins views"
(https://github.com/bitcoin/bitcoin/pull/32973)
Deletes `test_mid_package_evaluation` that is brittle and already covered in other places. It was introduced in #28251 addressing 2 issues: (1) calling `LimitMempoolSize()` in the middle of package validation and (2) not updating coins view cache when the mempool contents change, leading to "disappearing coins."

(1) If you let `AcceptSingleTransaction` call `LimitMempoolSize` in the middle of package validation, you should get a failure in `test_mid_package_eviction_success` (the package is r
...

📝 waketraindev converted_to_draft a pull request: "rpc,net: Add getpeerbyid and listpeersbyids RPCs"
(https://github.com/bitcoin/bitcoin/pull/32972)
Add fast peer lookup by NodeId using `m_node_id_map` in CConnman.

Adds `GetNodeStatsById` and `GetNodeStatsByIds` in `net.{cpp,h}` for efficient internal access.

Introduces two new RPCs:
- `getpeerbyid(id)`: returns info for a single connected peer
- `listpeersbyids([ids], strict=false)`: batch query with optional strict mode

Includes functional tests covering expected behavior and edge cases.

Useful for tools that track peers by ID without scanning the full list, or for quickly lo
...

💬 LaurentMT commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3070834011)
DUST_RELAY_TX_FEE should be decreased along with this as well...

💬 Sjors commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3070838431)
Another thing I noticed while testing using the `send` RPC, in a wallet with _only_ an `sp()` descriptor, is that it insists on having a bech32 descriptor for change:

> Transaction needs a change address, but we can't generate it. Error: No bech32 addresses available.

It seems safer, for backups, to send change to the same silent payment address.

Wallet created by:

```sh
bitcoin rpc createwallet Silent blank=true silent_payments=true
bitcoin rpc importdescriptors '[{"desc": "sp(xpr
...

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205691366)
Right, moved

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205691859)
oh weird! added

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205723195)
Thanks! Added, I should have just done this.

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205692104)
done, thanks

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205723116)
left it in for now.

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205691617)
added

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205722748)
added

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205719816)
CAUGHT 🔫

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205691462)
added

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205735550)
Yeah it was originally implemented that way, but I think this way likely requires much fewer element lookups. Under "normal" conditions, we probably have multiple announcers for each transaction and there are very children from this peer.

🤔 mzumsande reviewed a pull request: "rpc,net: Add getpeerbyid and listpeersbyids RPCs"
(https://github.com/bitcoin/bitcoin/pull/32972#pullrequestreview-3017872764)
> yes, the option to retrieve result by peer id.

Isn't this what your other PR #32741 suggests? Is this meant to be an alternative to that?