Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use NodeId instead of String for discv5 json types #1488

Merged
merged 1 commit into from
Sep 25, 2024
Merged

refactor: use NodeId instead of String for discv5 json types #1488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 4 additions & 8 deletions ethportal-api/src/types/discv5.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use super::enr::Enr;
use crate::utils::bytes::hex_encode;
use serde::{Deserialize, Serialize};

use discv5::enr::NodeId;
Expand All @@ -9,7 +8,7 @@ use discv5::enr::NodeId;
#[serde(transparent)]
pub struct Bucket {
#[serde(skip_serializing_if = "Vec::is_empty")]
pub node_ids: Vec<String>,
pub node_ids: Vec<NodeId>,
}

/// Represents a discv5 kbuckets table
Expand All @@ -24,15 +23,15 @@ pub struct KBucketsTable {
#[serde(rename_all = "camelCase")]
pub struct NodeInfo {
pub enr: Enr,
pub node_id: String,
pub node_id: NodeId,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be worth adding a comment here that NodeId gets properly serialized as a 0x-prefixed hexstring

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think that's necessary. Since Serialize/Deserialize is derived, one should assume that serialization is done correctly.

But if you feel strong about it, I can add it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

nope, just a nitpick

pub ip: Option<String>,
}

/// Information about a discv5/overlay network's routing table.
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct RoutingTableInfo {
pub local_node_id: String,
pub local_node_id: NodeId,
pub buckets: KBucketsTable,
}

Expand All @@ -41,10 +40,7 @@ impl<TVal: Eq> From<discv5::kbucket::KBucketsTable<NodeId, TVal>> for KBucketsTa
let buckets = table
.buckets_iter()
.map(|bucket| Bucket {
node_ids: bucket
.iter()
.map(|node| hex_encode(*node.key.preimage()))
.collect(),
node_ids: bucket.iter().map(|node| *node.key.preimage()).collect(),
})
.collect();

Expand Down
6 changes: 4 additions & 2 deletions ethportal-peertest/src/scenarios/basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ pub async fn test_discv5_node_info(peertest: &Peertest) {

pub async fn test_discv5_routing_table_info(target: &Client) {
info!("Testing discv5_routingTableInfo");
let node_info = target.node_info().await.unwrap();
let result = Discv5ApiClient::routing_table_info(target).await.unwrap();
assert!(result.local_node_id.starts_with("0x"));
assert_eq!(result.local_node_id, node_info.node_id);
}

pub async fn test_routing_table_info(subnetwork: Subnetwork, target: &Client) {
info!("Testing routing_table_info for {subnetwork}");
let node_info = target.node_info().await.unwrap();
let result = match subnetwork {
Subnetwork::Beacon => BeaconNetworkApiClient::routing_table_info(target),
Subnetwork::History => HistoryNetworkApiClient::routing_table_info(target),
Expand All @@ -39,7 +41,7 @@ pub async fn test_routing_table_info(subnetwork: Subnetwork, target: &Client) {
}
.await
.unwrap();
assert!(result.local_node_id.starts_with("0x"));
assert_eq!(result.local_node_id, node_info.node_id);
}

pub async fn test_radius(subnetwork: Subnetwork, target: &Client) {
Expand Down
6 changes: 3 additions & 3 deletions portalnet/src/discovery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use super::config::PortalnetConfig;
use crate::socket;
use ethportal_api::{
types::{discv5::RoutingTableInfo, enr::Enr, network::Subnetwork, portal_wire::NetworkSpec},
utils::bytes::{hex_decode, hex_encode},
utils::bytes::hex_decode,
NodeInfo,
};
use trin_utils::version::get_trin_version;
Expand Down Expand Up @@ -279,7 +279,7 @@ impl Discovery {
Ok(NodeInfo {
enr: Enr::from_str(&self.discv5.local_enr().to_base64())
.map_err(|err| anyhow!("{err}"))?,
node_id: hex_encode(self.discv5.local_enr().node_id().raw()),
node_id: self.discv5.local_enr().node_id(),
ip: self
.discv5
.local_enr()
Expand All @@ -292,7 +292,7 @@ impl Discovery {
/// k-buckets.
pub fn routing_table_info(&self) -> RoutingTableInfo {
RoutingTableInfo {
local_node_id: hex_encode(self.discv5.local_enr().node_id().raw()),
local_node_id: self.discv5.local_enr().node_id(),
buckets: self.discv5.kbuckets().into(),
}
}
Expand Down
2 changes: 1 addition & 1 deletion portalnet/src/overlay/protocol.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ where
/// Returns the node-id and a nested array of node-ids to represent this node's k-buckets table.
pub fn routing_table_info(&self) -> RoutingTableInfo {
RoutingTableInfo {
local_node_id: hex_encode(self.local_enr().node_id().raw()),
local_node_id: self.local_enr().node_id(),
buckets: self.kbuckets.read().clone().into(),
}
}
Expand Down
Loading