Bitcoin Core Github
44 subscribers
120K links
Download Telegram
👋 waketraindev's pull request is ready for review: "rpc,net: Add getpeerbyid and listpeersbyids RPCs"
(https://github.com/bitcoin/bitcoin/pull/32972)
glozow closed a pull request: "chore: rename misspelling variable"
(https://github.com/bitcoin/bitcoin/pull/32971)
💬 glozow commented on pull request "chore: rename misspelling variable":
(https://github.com/bitcoin/bitcoin/pull/32971#issuecomment-3070566405)
Thanks for your interest in contributing! However, as we have hundreds of pull requests, I am closing this to focus review on the others. (See [contributing guidelines on refactoring](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring)).
🤔 glozow reviewed a pull request: "rpc,net: Add getpeerbyid and listpeersbyids RPCs"
(https://github.com/bitcoin/bitcoin/pull/32972#pullrequestreview-3017372888)
Is anything here not already provided by getpeerinfo?
💬 waketraindev commented on pull request "rpc,net: Add getpeerbyid and listpeersbyids RPCs":
(https://github.com/bitcoin/bitcoin/pull/32972#issuecomment-3070587105)
> Is anything here not already provided by getpeerinfo?

yes, the option to retrieve result by peer id.
🤔 theStack reviewed a pull request: "p2p: improve TxOrphanage denial of service bounds"
(https://github.com/bitcoin/bitcoin/pull/31829#pullrequestreview-3017348305)
Left some nits I found over the last days, mostly about naming. I think it would be worth it to update the commit message of main overhaul commit d5097d6eef85eab971352cf78fb94a30c6e6f127 to use the new "latency score" terminology. Planning to look more at functional and fuzz tests soon.
💬 theStack commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205553042)
:detective:
```suggestion
// Delete this tx, clearing the orphanage.
```
💬 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.