📝 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
...
(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...
(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
...
(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
(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
(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.
(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
(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.
(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
(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
(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 🔫
(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
(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.
(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?
(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?
💬 waketraindev commented on pull request "rpc,net: Add getpeerbyid and listpeersbyids RPCs":
(https://github.com/bitcoin/bitcoin/pull/32972#issuecomment-3071047468)
> > 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?
> > 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?
Yes, this is essentially a more complete version of that, incorporating feedback and suggestions from other contributors. It serves as an alternative approach to PR #32741.
(https://github.com/bitcoin/bitcoin/pull/32972#issuecomment-3071047468)
> > 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?
> > 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?
Yes, this is essentially a more complete version of that, incorporating feedback and suggestions from other contributors. It serves as an alternative approach to PR #32741.
🤔 furszy reviewed a pull request: "index: initial sync speedup, parallelize process"
(https://github.com/bitcoin/bitcoin/pull/26966#pullrequestreview-3017897441)
> Sorry, #32948 probably caused some conflicts.
Auch. Rebased.
(https://github.com/bitcoin/bitcoin/pull/26966#pullrequestreview-3017897441)
> Sorry, #32948 probably caused some conflicts.
Auch. Rebased.
💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3071149725)
> It seems that since m_block_index is a std::unordered_map, the iteration order in RecalculateBestHeader is not guaranteed.
Oh yes, this bit me in an earlier iteration of this PR where I made the mistake to do `PickValue(fuzzed_data_provider, blockman.m_block_index)` directly to pick a random block index (so that no `blocks` vector was needed).
It's also related to the fact that we don't use `CBlockIndexWorkComparator` tiebreak rules for `m_best_header` (documented in #32843), so I think
...
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3071149725)
> It seems that since m_block_index is a std::unordered_map, the iteration order in RecalculateBestHeader is not guaranteed.
Oh yes, this bit me in an earlier iteration of this PR where I made the mistake to do `PickValue(fuzzed_data_provider, blockman.m_block_index)` directly to pick a random block index (so that no `blocks` vector was needed).
It's also related to the fact that we don't use `CBlockIndexWorkComparator` tiebreak rules for `m_best_header` (documented in #32843), so I think
...
👋 waketraindev's pull request is ready for review: "rpc,net: Add getpeerbyid and listpeersbyids RPCs"
(https://github.com/bitcoin/bitcoin/pull/32972)
(https://github.com/bitcoin/bitcoin/pull/32972)
💬 tnndbtc commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-3071393448)
I assume the general code logic should be correct, but likely some exception/run-time error occurred then caused the intermittent issue.
So, I studied `Popen::execute_process()` in src/util/subprocess.h, one interesting statement caught my attention:
```
subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below
stream_.close_child_fds();
```
I notice that we didn't check the return code of the calling of `subprocess_close()/close()`, in many cases. Indeed,
...
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-3071393448)
I assume the general code logic should be correct, but likely some exception/run-time error occurred then caused the intermittent issue.
So, I studied `Popen::execute_process()` in src/util/subprocess.h, one interesting statement caught my attention:
```
subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below
stream_.close_child_fds();
```
I notice that we didn't check the return code of the calling of `subprocess_close()/close()`, in many cases. Indeed,
...
💬 andrewtoth commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3071549529)
> DUST_RELAY_TX_FEE should be decreased along with this as well...
That value is used to calculate the minimum output amount, not the fee rate of a tx. That seems orthogonal to the goal of this PR.
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3071549529)
> DUST_RELAY_TX_FEE should be decreased along with this as well...
That value is used to calculate the minimum output amount, not the fee rate of a tx. That seems orthogonal to the goal of this PR.