Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions bin/floresta-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ fn do_request(cmd: &Cli, client: Client) -> anyhow::Result<String> {
serde_json::to_string_pretty(&client.get_block(hash, verbosity)?)?
}
Methods::GetPeerInfo => serde_json::to_string_pretty(&client.get_peer_info()?)?,
Methods::GetAddedNodeInfo { node } => {
serde_json::to_string_pretty(&client.get_added_node_info(node)?)?
}
Methods::Stop => serde_json::to_string_pretty(&client.stop()?)?,
Methods::AddNode {
node,
Expand Down Expand Up @@ -293,6 +296,18 @@ pub enum Methods {
#[command(name = "getpeerinfo")]
GetPeerInfo,

#[doc = include_str!("../../../doc/rpc/getaddednodeinfo.md")]
#[command(
name = "getaddednodeinfo",
about = "Returns information about the given added node, or all added nodes",
long_about = Some(include_str!("../../../doc/rpc/getaddednodeinfo.md")),
disable_help_subcommand = true
)]
GetAddedNodeInfo {
/// If provided, return information about this specific node
node: Option<String>,
},

/// Returns the value associated with a UTXO, if it's still not spent.
/// This function only works properly if we have the compact block filters
/// feature enabled
Expand Down
10 changes: 10 additions & 0 deletions crates/floresta-node/src/json_rpc/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,14 @@ impl<Blockchain: RpcChain> RpcImpl<Blockchain> {
.await
.map_err(|_| JsonRpcError::Node("Failed to get peer information".to_string()))
}

pub(crate) async fn get_added_node_info(
&self,
node: Option<String>,
) -> Result<Vec<floresta_wire::node_interface::AddedNodeInfoItem>> {
self.node
.get_added_node_info(node)
.await
.map_err(|e| JsonRpcError::Node(e.to_string()))
}
}
9 changes: 9 additions & 0 deletions crates/floresta-node/src/json_rpc/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,15 @@ async fn handle_json_rpc_request(
.await
.map(|v| serde_json::to_value(v).unwrap()),

"getaddednodeinfo" => {
let node = get_optional_field(&params, 0, "node", get_string)?;

state
.get_added_node_info(node)
.await
.map(|v| serde_json::to_value(v).unwrap())
}

"addnode" => {
let node = get_string(&params, 0, "node")?;
let command = get_string(&params, 1, "command")?;
Expand Down
10 changes: 10 additions & 0 deletions crates/floresta-rpc/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ pub trait FlorestaRPC {
/// the peer's IP address, the peer's version, the peer's user agent, the transport protocol
/// and the peer's current height.
fn get_peer_info(&self) -> Result<Vec<PeerInfo>>;
#[doc = include_str!("../../../doc/rpc/getaddednodeinfo.md")]
/// Returns information about manually added peers.
fn get_added_node_info(&self, node: Option<String>) -> Result<Vec<AddedNodeInfo>>;
/// Returns a block, given a block hash
///
/// This method returns a block, given a block hash. If the verbosity flag is 0, the block
Expand Down Expand Up @@ -293,6 +296,13 @@ impl<T: JsonRPCClient> FlorestaRPC for T {
self.call("getpeerinfo", &[])
}

fn get_added_node_info(&self, node: Option<String>) -> Result<Vec<AddedNodeInfo>> {
match node {
Some(node) => self.call("getaddednodeinfo", &[Value::String(node)]),
None => self.call("getaddednodeinfo", &[]),
}
}

fn get_best_block_hash(&self) -> Result<BlockHash> {
self.call("getbestblockhash", &[])
}
Expand Down
20 changes: 20 additions & 0 deletions crates/floresta-rpc/src/rpc_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,26 @@ pub struct PeerInfo {
pub transport_protocol: String,
}

/// Information about a manually added peer, as returned by `getaddednodeinfo`.
#[derive(Debug, Deserialize, Serialize)]
pub struct AddedNodeInfo {
/// The node address in `ip:port` format, as provided to `addnode`.
pub addednode: String,
/// Whether the node is currently connected.
pub connected: bool,
/// A list of addresses with connection direction info. Empty when not connected.
pub addresses: Vec<AddedNodeAddressInfo>,
}

/// An address entry within [`AddedNodeInfo`].
#[derive(Debug, Deserialize, Serialize)]
pub struct AddedNodeAddressInfo {
/// The address in `ip:port` format.
pub address: String,
/// Connection direction: `"outbound"` when connected.
pub connected: String,
}

#[derive(Debug, Deserialize, Serialize)]
#[serde(untagged)]
pub enum GetBlockRes {
Expand Down
54 changes: 54 additions & 0 deletions crates/floresta-wire/src/p2p_wire/node/user_req.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@ use tracing::warn;
use super::try_and_log;
use super::NodeRequest;
use super::UtreexoNode;
use crate::address_man::LocalAddress;
use crate::block_proof::Bitmap;
use crate::node::running_ctx::RunningNode;
use crate::node_context::NodeContext;
use crate::node_interface::AddedNodeAddress;
use crate::node_interface::AddedNodeInfoItem;
use crate::node_interface::NodeInterface;
use crate::node_interface::NodeResponse;
use crate::node_interface::UserRequest;
Expand Down Expand Up @@ -47,6 +50,52 @@ where
try_and_log!(responder.send(NodeResponse::GetPeerInfo(peers)));
}

/// Handles getaddednodeinfo requests, returning info about manually addedd peers.
fn handle_get_added_node_info(
&self,
node: Option<String>,
responder: oneshot::Sender<NodeResponse>,
) {
let items: Vec<AddedNodeInfoItem> = self
.added_peers
.iter()
.filter_map(|added| {
let added_ip = LocalAddress::from(added.address.clone()).get_net_address();
let addr_str = format!("{}:{}", added_ip, added.port);

// If a specific node was requested, skip non-matching entries
if let Some(ref filter) = node {
if &addr_str != filter && &added_ip.to_string() != filter {
return None;
}
}

// Check if this added peer is currently connected
let is_connected = self
.peers
.values()
.any(|peer| peer.address == added_ip && peer.port == added.port);

let addresses = if is_connected {
vec![AddedNodeAddress {
address: addr_str.clone(),
connected: "outbound".to_string(),
}]
} else {
vec![]
};

Some(AddedNodeInfoItem {
addednode: addr_str,
connected: is_connected,
addresses,
})
})
.collect();

try_and_log!(responder.send(NodeResponse::GetAddedNodeInfo(items)));
}

/// Actually perform the user request
///
/// These are requests made by some consumer of `floresta-wire` using the [`NodeInterface`], and may
Expand Down Expand Up @@ -90,6 +139,11 @@ where
return;
}

UserRequest::GetAddedNodeInfo(node) => {
self.handle_get_added_node_info(node, responder);
return;
}

UserRequest::Add((addr, port, v2transport)) => {
let node_response = match self.handle_addnode_add_peer(addr, port, v2transport) {
Ok(_) => {
Expand Down
44 changes: 44 additions & 0 deletions crates/floresta-wire/src/p2p_wire/node_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ pub enum UserRequest {

/// Adds a transaction to mempool and advertises it
SendTransaction(Transaction),

/// Return information about manually added peers, optionally filtered by address.
GetAddedNodeInfo(Option<String>),
}

#[derive(Debug, Clone, Serialize)]
Expand All @@ -113,6 +116,26 @@ pub struct PeerInfo {
pub transport_protocol: TransportProtocol,
}

#[derive(Debug, Clone, Serialize)]
/// Information about a manually added peer, as returned by `getaddednodeinfo`.
pub struct AddedNodeInfoItem {
/// The address of the added node in `ip:port` format.
pub addednode: String,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

floresta’s RPCs need to be as close as possible to the core standard, so here it’s just the ip from core without the port

https://bitcoincore.org/en/doc/30.0.0/rpc/network/getaddednodeinfo/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks...this will be addressed on PR #916

/// Whether the node is currently connected.
pub connected: bool,
/// A list of addresses associated with this added node, with connection direction info.
pub addresses: Vec<AddedNodeAddress>,
}

#[derive(Debug, Clone, Serialize)]
/// An address entry within `AddedNodeInfoItem`.
pub struct AddedNodeAddress {
/// The address in `ip:port` format.
pub address: String,
/// The connection direction: `"outbound"` if connected, `"false"` if not.
pub connected: String,
}

#[derive(Debug)]
/// A response that can be sent back to the user.
///
Expand Down Expand Up @@ -151,6 +174,9 @@ pub enum NodeResponse {

/// Transaction broadcast
TransactionBroadcastResult(Result<Txid, AcceptToMempoolError>),

/// A response containing a list of manually added peer information.
GetAddedNodeInfo(Vec<AddedNodeInfoItem>),
}

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -308,6 +334,24 @@ impl NodeInterface {
extract_variant!(GetPeerInfo, val);
}

/// Gets information about all manually added peers, optionally filtered by address.
///
/// This function will return a list of `AddedNodeInfoItem` structs, each of which contains
/// information about a manually added peer.
///
/// If an address is provided, the function will only return information about the peer with
/// that address.
pub async fn get_added_node_info(
&self,
node: Option<String>,
) -> Result<Vec<AddedNodeInfoItem>, oneshot::error::RecvError> {
let val = self
.send_request(UserRequest::GetAddedNodeInfo(node))
.await?;

extract_variant!(GetAddedNodeInfo, val);
}

/// Pings all connected peers to check if they are alive.
pub async fn ping(&self) -> Result<bool, oneshot::error::RecvError> {
let val = self.send_request(UserRequest::Ping).await?;
Expand Down
52 changes: 52 additions & 0 deletions doc/rpc/getaddednodeinfo.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# `getaddednodeinfo`

Returns information about the given added node, or all added nodes.

## Usage

### Synopsis

```bash
floresta-cli getaddednodeinfo [node]
```

### Examples

```bash
floresta-cli getaddednodeinfo
floresta-cli getaddednodeinfo 192.168.0.1:8333
```

## Arguments

`node` - (string, optional) If provided, return information about this specific node. Can be an `ip:port` or just an `ip`.

## Returns

### Ok response

```json
[
{
"addednode": "192.168.0.1:8333",
"connected": true,
"addresses": [
{
"address": "192.168.0.1:8333",
"connected": "outbound"
}
]
}
]
```

- `addednode` - (string) The node address in `ip:port` format, as provided to `addnode`.
- `connected` - (boolean) Whether the node is currently connected.
- `addresses` - (json array) A list of addresses with connection direction info. Empty when not connected.
- `address` - (string) The address in `ip:port` format.
- `connected` - (string) Connection direction: `"outbound"` when connected.

## Notes

- Only nodes added via `addnode add` are listed. Nodes connected with `addnode onetry` are not included.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this information is wrong based on the code I saw. It would be useful to demonstrate this in the Python test, since this is a rule for this RPC that must be followed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@moisesPompilio thanks for your feedbacks... the way getaddednodeinfo was implemented here was such that it reads from added_pairs ...thats the reason onetry peers won't appear...cus handle_addnode_onetry_peer never adds to added_peers
image

@jaoleal opened a PR with this implementation there and we decided to use that...your reviews there too will be highly appreciated...

- The `addresses` array is empty when `connected` is `false`.
62 changes: 62 additions & 0 deletions tests/floresta-cli/getaddednodeinfo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# SPDX-License-Identifier: MIT OR Apache-2.0

"""
floresta_cli_getaddednodeinfo.py

This functional test cli utility to interact with a Floresta node with `getaddednodeinfo`
"""

from test_framework import FlorestaTestFramework
from test_framework.node import NodeType


class GetAddedNodeInfoTest(FlorestaTestFramework):
"""
Test `getaddednodeinfo` by adding a bitcoind peer via `addnode add`,
then verifying the result of `getaddednodeinfo` contains the peer.
"""

def set_test_params(self):
"""
Setup florestad and bitcoind nodes
"""
self.florestad = self.add_node_default_args(variant=NodeType.FLORESTAD)
self.bitcoind = self.add_node_extra_args(
variant=NodeType.BITCOIND,
extra_args=["-v2transport=0"],
)

def run_test(self):
"""
Test getaddednodeinfo returns correct info before and after adding a peer
"""
self.run_node(self.florestad)
self.run_node(self.bitcoind)

# Before adding any node, the list should be empty
result = self.florestad.rpc.get_addednodeinfo()
self.assertIsSome(result)
self.assertEqual(len(result), 0)

# Add bitcoind as a persistent peer
self.connect_nodes(self.florestad, self.bitcoind, "add", v2transport=False)
self.wait_for_peers_connections(self.florestad, self.bitcoind, True)

# getaddednodeinfo should now return one entry
result = self.florestad.rpc.get_addednodeinfo()
self.assertIsSome(result)
self.assertEqual(len(result), 1)
self.assertTrue(result[0]["connected"])
self.assertEqual(len(result[0]["addresses"]), 1)
self.assertEqual(result[0]["addresses"][0]["connected"], "outbound")

# Query by specific node address
node_addr = result[0]["addednode"]
filtered = self.florestad.rpc.get_addednodeinfo(node_addr)
self.assertIsSome(filtered)
self.assertEqual(len(filtered), 1)
self.assertEqual(filtered[0]["addednode"], node_addr)


if __name__ == "__main__":
GetAddedNodeInfoTest().main()
8 changes: 8 additions & 0 deletions tests/test_framework/rpc/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,3 +327,11 @@ def disconnectnode(self, node_address: str, node_id: Optional[int] = None):
"disconnectnode", params=[node_address, node_id]
)
return self.perform_request("disconnectnode", params=[node_address])

def get_addednodeinfo(self, node: Optional[str] = None):
"""
Returns information about the given added node, or all added nodes
"""
if node is not None:
return self.perform_request("getaddednodeinfo", params=[node])
return self.perform_request("getaddednodeinfo")
1 change: 1 addition & 0 deletions tests/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
("example", "functional"),
("floresta-cli", "getmemoryinfo"),
("floresta-cli", "getpeerinfo"),
("floresta-cli", "getaddednodeinfo"),
("floresta-cli", "getblockchaininfo"),
("floresta-cli", "getblockheader"),
("example", "bitcoin"),
Expand Down