Network RPCs: getaddednodeinfo, addpeeraddress, getnodedaddresses e getaddrmaninfo.#916
Network RPCs: getaddednodeinfo, addpeeraddress, getnodedaddresses e getaddrmaninfo.#916jaoleal wants to merge 4 commits intogetfloresta:masterfrom
Conversation
Add getaddednodeinfo RPC that returns information about manually added nodes, including whether each is currently connected. Cross-references added_peers with active peer connections. Includes integration test and Python RPC wrapper.
Add addpeeraddress RPC that manually adds a peer address to the address manager. Accepts address, port, and optional tried parameter. Constructs a LocalAddress with WITNESS|NETWORK_LIMITED services and pushes it to AddressMan. Also adds address_count() helper to AddressMan. Includes integration test and Python RPC wrapper.
Add getnodeaddresses RPC that returns known peer addresses from the address manager. Accepts an optional count parameter and optional network filter (ipv4, ipv6, onion, i2p, cjdns). Includes integration test and Python RPC wrapper.
Add getaddrmaninfo RPC that returns address manager statistics broken down by network type (ipv4, ipv6, cjdns). Each network reports total, new (untried), and tried address counts. Adds get_addrman_info() aggregation method to AddressMan. Includes integration test and Python RPC wrapper.
ea030cf to
9f84eb4
Compare
|
Can you add a changelog to the PR description? |
| pub(crate) async fn get_added_node_info(&self) -> Result<Vec<AddedNodeInfo>> { | ||
| self.node | ||
| .get_added_node_info() | ||
| .await | ||
| .map_err(|e| JsonRpcError::Node(e.to_string())) | ||
| } |
There was a problem hiding this comment.
Hi @jaoleal ..looking at the RPC docs for bitocoin core, https://bitcoincore.org/en/doc/30.0.0/rpc/network/getaddednodeinfo/ the rpc endpoint is meant to take the node address and return info for that address else it returns node information for all nodes...
| "getaddednodeinfo" => state | ||
| .get_added_node_info() | ||
| .await | ||
| .map(|v| serde_json::to_value(v).unwrap()), |
There was a problem hiding this comment.
this should take something like let node = get_optional_field(¶ms, 0, "node", get_string)?;
| #[derive(Debug, Clone, Serialize)] | ||
| /// Information about a manually added node (via `addnode`). | ||
| pub struct AddedNodeInfo { | ||
| /// The address of the added node in "ip:port" format. |
There was a problem hiding this comment.
as @moisesPompilio raised , we need to have the RPCs as close as possible to the core standard which requires only IP without the port
| pub struct AddedNodeInfo { | ||
| /// The address of the added node in "ip:port" format. | ||
| pub addednode: String, | ||
| /// Whether we are currently connected to this node. | ||
| pub connected: bool, | ||
| } |
| self.florestad.rpc.addnode(self.bitcoind.p2p_url, "add", v2transport=False) | ||
| self.wait_for_peers_connections(self.florestad, self.bitcoind) |
There was a problem hiding this comment.
| self.florestad.rpc.addnode(self.bitcoind.p2p_url, "add", v2transport=False) | |
| self.wait_for_peers_connections(self.florestad, self.bitcoind) | |
| self.connect_nodes(self.florestad, self.bitcoind) |
| /// Returns information about all manually added nodes. | ||
| pub async fn get_added_node_info( | ||
| &self, | ||
| ) -> Result<Vec<AddedNodeInfo>, oneshot::error::RecvError> { |
There was a problem hiding this comment.
I think it's better to import RecvError and use it directly instead of using a fully-qualified path
| addresses | ||
| .into_iter() | ||
| .filter(|(addr, _, _, _)| match &network { | ||
| None => true, | ||
| Some(ReachableNetworks::IPv4) => matches!(addr, AddrV2::Ipv4(_)), | ||
| Some(ReachableNetworks::IPv6) => matches!(addr, AddrV2::Ipv6(_)), | ||
| Some(ReachableNetworks::TorV3) => matches!(addr, AddrV2::TorV3(_)), | ||
| Some(ReachableNetworks::I2P) => matches!(addr, AddrV2::I2p(_)), | ||
| Some(ReachableNetworks::Cjdns) => matches!(addr, AddrV2::Cjdns(_)), | ||
| }) | ||
| .take(count) | ||
| .filter_map(|(addr, time, services, port)| { | ||
| let (address, network) = match &addr { | ||
| AddrV2::Ipv4(ip) => (ip.to_string(), "ipv4"), | ||
| AddrV2::Ipv6(ip) => (ip.to_string(), "ipv6"), | ||
| AddrV2::Cjdns(ip) => (ip.to_string(), "cjdns"), | ||
| AddrV2::TorV3(key) => (hex::encode(key), "onion"), | ||
| AddrV2::I2p(key) => (hex::encode(key), "i2p"), | ||
| _ => return None, | ||
| }; | ||
|
|
||
| Some(NodeAddress { | ||
| time, | ||
| services: services.to_u64(), | ||
| address, | ||
| port, | ||
| network: network.to_string(), | ||
| }) | ||
| }) | ||
| .collect() |
There was a problem hiding this comment.
Wouldn't it be better to use a single filter_map instead of a filter followed by a filter_map?
|
|
||
| ### Ok Response | ||
|
|
||
| A JSON array of address objects: |
There was a problem hiding this comment.
lso missing to include the network, because if the user doesn't choose a type, they need to know which network that address is for
https://bitcoincore.org/en/doc/30.0.0/rpc/network/getnodeaddresses/
|
|
||
| def set_test_params(self): | ||
| self.florestad = self.add_node_default_args(variant=NodeType.FLORESTAD) | ||
| self.bitcoind = self.add_node_extra_args( |
There was a problem hiding this comment.
use the add_node_default_args instead add_node_extra_args
| self.connect_nodes(self.florestad, self.bitcoind, "add", v2transport=False) | ||
| self.wait_for_peers_connections(self.florestad, self.bitcoind) |
There was a problem hiding this comment.
| self.connect_nodes(self.florestad, self.bitcoind, "add", v2transport=False) | |
| self.wait_for_peers_connections(self.florestad, self.bitcoind) | |
| self.connect_nodes(self.florestad, self.bitcoind) |
| extra_args=["-v2transport=0"], | ||
| ) | ||
|
|
||
| def run_test(self): |
There was a problem hiding this comment.
Before connecting to the node, can you test what happens if you make this RPC request when the peer has no node addresses?
| pub fn get_addrman_info(&self) -> ConnectionStats { | ||
| let mut stats = ConnectionStats::default(); | ||
|
|
||
| for addr in self.addresses.values() { | ||
| let bucket = match &addr.address { | ||
| AddrV2::Ipv4(_) => &mut stats.ipv4, | ||
| AddrV2::Ipv6(_) => &mut stats.ipv6, | ||
| AddrV2::TorV3(_) => &mut stats.onion, | ||
| AddrV2::I2p(_) => &mut stats.i2p, | ||
| AddrV2::Cjdns(_) => &mut stats.cjdns, | ||
| _ => continue, | ||
| }; | ||
|
|
||
| bucket.total += 1; | ||
| match addr.state { | ||
| AddressState::Tried(_) | AddressState::Connected => bucket.tried += 1, | ||
| _ => bucket.new += 1, | ||
| } | ||
| } | ||
|
|
||
| stats | ||
| } |
There was a problem hiding this comment.
| pub fn get_addrman_info(&self) -> ConnectionStats { | |
| let mut stats = ConnectionStats::default(); | |
| for addr in self.addresses.values() { | |
| let bucket = match &addr.address { | |
| AddrV2::Ipv4(_) => &mut stats.ipv4, | |
| AddrV2::Ipv6(_) => &mut stats.ipv6, | |
| AddrV2::TorV3(_) => &mut stats.onion, | |
| AddrV2::I2p(_) => &mut stats.i2p, | |
| AddrV2::Cjdns(_) => &mut stats.cjdns, | |
| _ => continue, | |
| }; | |
| bucket.total += 1; | |
| match addr.state { | |
| AddressState::Tried(_) | AddressState::Connected => bucket.tried += 1, | |
| _ => bucket.new += 1, | |
| } | |
| } | |
| stats | |
| } | |
| pub fn get_addrman_info(&self) -> AddrManInfo { | |
| let mut stats = AddrManInfo::default(); | |
| for addr in self.addresses.values() { | |
| let bucket = match &addr.address { | |
| AddrV2::Ipv4(_) => &mut stats.ipv4, | |
| AddrV2::Ipv6(_) => &mut stats.ipv6, | |
| AddrV2::TorV3(_) => &mut stats.onion, | |
| AddrV2::I2p(_) => &mut stats.i2p, | |
| AddrV2::Cjdns(_) => &mut stats.cjdns, | |
| _ => continue, | |
| }; | |
| bucket.total += 1; | |
| stats.all_networks.total += 1; | |
| match addr.state { | |
| AddressState::Tried(_) | AddressState::Connected => { | |
| bucket.tried += 1; | |
| stats.all_networks.tried += 1; | |
| } | |
| _ => { | |
| bucket.new += 1; | |
| stats.all_networks.new += 1; | |
| } | |
| } | |
| } | |
| stats | |
| } |
Consider removing ConnectionStats and using AddrManInfo directly here
|
Needs rebase. |

Description and Notes
These already had the needed subsystems and theyre pretty usefull.
How to verify the changes you have done?
Review the tests and propose exercise extensions whether it applies.