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.

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: unify hex encoding with data-encoding #3047

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
3 changes: 1 addition & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion iroh-relay/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ futures-lite = "2.5"
futures-sink = "0.3"
futures-util = "0.3"
governor = "0.7.0"
hex = "0.4.3"
hickory-proto = { version = "=0.25.0-alpha.4" }
hickory-resolver = "=0.25.0-alpha.4"
hostname = "0.4"
Expand Down Expand Up @@ -96,6 +95,7 @@ tracing-subscriber = { version = "0.3", features = [
url = { version = "2.5", features = ["serde"] }
webpki = { package = "rustls-webpki", version = "0.102" }
webpki-roots = "0.26"
data-encoding = "2.6.0"

[dev-dependencies]
clap = { version = "4", features = ["derive"] }
Expand Down
8 changes: 6 additions & 2 deletions iroh-relay/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,11 @@ impl PingTracker {
///
/// If there is no [`oneshot::Sender`] in the tracker, return `None`.
fn unregister(&mut self, data: [u8; 8], why: &'static str) -> Option<oneshot::Sender<()>> {
trace!("removing ping {}: {}", hex::encode(data), why);
trace!(
"removing ping {}: {}",
data_encoding::HEXLOWER.encode(&data),
why
);
self.0.remove(&data)
}
}
Expand Down Expand Up @@ -759,7 +763,7 @@ impl Actor {
async fn ping(&mut self, s: oneshot::Sender<Result<Duration, ClientError>>) {
let connect_res = self.connect("ping").await.map(|(c, _)| c);
let (ping, recv) = self.pings.register();
trace!("ping: {}", hex::encode(ping));
trace!("ping: {}", data_encoding::HEXLOWER.encode(&ping));

self.ping_tasks.spawn(async move {
let res = match connect_res {
Expand Down
2 changes: 1 addition & 1 deletion iroh-relay/src/protos/stun.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ mod tests {
},
ResponseTestCase {
name: "no-4in6",
data: hex::decode("010100182112a4424fd5d202dcb37d31fc773306002000140002cd3d2112a4424fd5d202dcb382ce2dc3fcc7").unwrap(),
data: data_encoding::HEXLOWER.decode(b"010100182112a4424fd5d202dcb37d31fc773306002000140002cd3d2112a4424fd5d202dcb382ce2dc3fcc7").unwrap(),
want_tid: vec![79, 213, 210, 2, 220, 179, 125, 49, 252, 119, 51, 6],
want_addr: IpAddr::V4(Ipv4Addr::from([209, 180, 207, 193])),
want_port: 60463,
Expand Down
5 changes: 2 additions & 3 deletions iroh/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ axum = { version = "0.7", optional = true }
backoff = "0.4.0"
base64 = "0.22.1"
bytes = "1.7"
data-encoding = "2.2"
der = { version = "0.7", features = ["alloc", "derive"] }
derive_more = { version = "1.0.0", features = [
"debug",
Expand All @@ -36,7 +37,6 @@ futures-lite = "2.5"
futures-sink = "0.3"
futures-util = "0.3"
governor = "0.7.0"
hex = "0.4.3"
hickory-resolver = { version = "=0.25.0-alpha.4" }
hostname = "0.4"
http = "1"
Expand Down Expand Up @@ -112,7 +112,6 @@ net-report = { package = "iroh-net-report", path = "../iroh-net-report", version
iroh-metrics = { version = "0.29", default-features = false }

# local-swarm-discovery
data-encoding = { version = "2.2", optional = true }
swarm-discovery = { version = "0.3.0-alpha.1", optional = true }

# dht_discovery
Expand Down Expand Up @@ -178,7 +177,7 @@ harness = false
default = ["metrics", "discovery-pkarr-dht"]
metrics = ["iroh-metrics/metrics", "iroh-relay/metrics", "net-report/metrics", "portmapper/metrics"]
test-utils = ["iroh-relay/test-utils", "iroh-relay/server", "dep:axum"]
discovery-local-network = ["dep:data-encoding", "dep:swarm-discovery"]
discovery-local-network = ["dep:swarm-discovery"]
discovery-pkarr-dht = ["pkarr/dht", "dep:genawaiter"]
examples = [
"dep:clap",
Expand Down
9 changes: 6 additions & 3 deletions iroh/src/disco.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use std::{
};

use anyhow::{anyhow, bail, ensure, Context, Result};
use data_encoding::HEXLOWER;
use iroh_base::{PublicKey, RelayUrl};
use serde::{Deserialize, Serialize};
use url::Url;
Expand Down Expand Up @@ -383,10 +384,10 @@ impl Display for Message {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Message::Ping(ping) => {
write!(f, "Ping(tx={})", hex::encode(ping.tx_id))
write!(f, "Ping(tx={})", HEXLOWER.encode(&ping.tx_id))
}
Message::Pong(pong) => {
write!(f, "Pong(tx={})", hex::encode(pong.tx_id))
write!(f, "Pong(tx={})", HEXLOWER.encode(&pong.tx_id))
}
Message::CallMeMaybe(_) => {
write!(f, "CallMeMaybe")
Expand Down Expand Up @@ -460,7 +461,9 @@ mod tests {
let got = test.m.as_bytes();
assert_eq!(
got,
hex::decode(test.want.replace(' ', "")).unwrap(),
data_encoding::HEXLOWER
.decode(test.want.replace(' ', "").as_bytes())
.unwrap(),
"wrong as_bytes"
);

Expand Down
27 changes: 14 additions & 13 deletions iroh/src/magicsock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use std::{
use anyhow::{anyhow, Context as _, Result};
use bytes::Bytes;
use concurrent_queue::ConcurrentQueue;
use data_encoding::HEXLOWER;
use futures_lite::{FutureExt, StreamExt};
use futures_util::{stream::BoxStream, task::AtomicWaker};
use iroh_base::{NodeAddr, NodeId, PublicKey, RelayUrl, SecretKey, SharedSecret};
Expand Down Expand Up @@ -1070,20 +1071,20 @@ impl MagicSock {
let handled = self.node_map.handle_ping(sender, addr.clone(), dm.tx_id);
match handled.role {
PingRole::Duplicate => {
debug!(%src, tx = %hex::encode(dm.tx_id), "received ping: path already confirmed, skip");
debug!(%src, tx = %HEXLOWER.encode(&dm.tx_id), "received ping: path already confirmed, skip");
return;
}
PingRole::LikelyHeartbeat => {}
PingRole::NewPath => {
debug!(%src, tx = %hex::encode(dm.tx_id), "received ping: new path");
debug!(%src, tx = %HEXLOWER.encode(&dm.tx_id), "received ping: new path");
}
PingRole::Activate => {
debug!(%src, tx = %hex::encode(dm.tx_id), "received ping: path active");
debug!(%src, tx = %HEXLOWER.encode(&dm.tx_id), "received ping: path active");
}
}

// Send a pong.
debug!(tx = %hex::encode(dm.tx_id), %addr, dstkey = %sender.fmt_short(),
debug!(tx = %HEXLOWER.encode(&dm.tx_id), %addr, dstkey = %sender.fmt_short(),
"sending pong");
let pong = disco::Message::Pong(disco::Pong {
tx_id: dm.tx_id,
Expand Down Expand Up @@ -1137,11 +1138,11 @@ impl MagicSock {
};
if sent {
let msg_sender = self.actor_sender.clone();
trace!(%dst, tx = %hex::encode(tx_id), ?purpose, "ping sent (queued)");
trace!(%dst, tx = %HEXLOWER.encode(&tx_id), ?purpose, "ping sent (queued)");
self.node_map
.notify_ping_sent(id, dst, tx_id, purpose, msg_sender);
} else {
warn!(dst = ?dst, tx = %hex::encode(tx_id), ?purpose, "failed to send ping: queues full");
warn!(dst = ?dst, tx = %HEXLOWER.encode(&tx_id), ?purpose, "failed to send ping: queues full");
}
}

Expand Down Expand Up @@ -1321,7 +1322,7 @@ impl MagicSock {
node_key: self.public_key(),
});
self.try_send_disco_message(dst.clone(), dst_node, msg)?;
debug!(%dst, tx = %hex::encode(tx_id), ?purpose, "ping sent (polled)");
debug!(%dst, tx = %HEXLOWER.encode(&tx_id), ?purpose, "ping sent (polled)");
let msg_sender = self.actor_sender.clone();
self.node_map
.notify_ping_sent(id, dst.clone(), tx_id, purpose, msg_sender);
Expand Down Expand Up @@ -3059,8 +3060,8 @@ mod tests {
val,
msg,
"[sender] expected {}, got {}",
hex::encode(msg),
hex::encode(&val)
HEXLOWER.encode(msg),
HEXLOWER.encode(&val)
);

let stats = conn.stats();
Expand Down Expand Up @@ -3465,8 +3466,8 @@ mod tests {
anyhow::ensure!(
val == $msg,
"expected {}, got {}",
hex::encode($msg),
hex::encode(val)
HEXLOWER.encode(&$msg[..]),
HEXLOWER.encode(&val)
);
};
}
Expand Down Expand Up @@ -3618,8 +3619,8 @@ mod tests {
anyhow::ensure!(
val == $msg,
"expected {}, got {}",
hex::encode($msg),
hex::encode(val)
HEXLOWER.encode(&$msg[..]),
HEXLOWER.encode(&val)
);
};
}
Expand Down
11 changes: 6 additions & 5 deletions iroh/src/magicsock/node_map/node_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::{
time::{Duration, Instant},
};

use data_encoding::HEXLOWER;
use iroh_base::{NodeAddr, NodeId, PublicKey, RelayUrl};
use iroh_metrics::inc;
use iroh_relay::protos::stun;
Expand Down Expand Up @@ -410,7 +411,7 @@ impl NodeState {
#[instrument("disco", skip_all, fields(node = %self.node_id.fmt_short()))]
pub(super) fn ping_timeout(&mut self, txid: stun::TransactionId) {
if let Some(sp) = self.sent_pings.remove(&txid) {
debug!(tx = %hex::encode(txid), addr = %sp.to, "pong not received in timeout");
debug!(tx = %HEXLOWER.encode(&txid), addr = %sp.to, "pong not received in timeout");
match sp.to {
SendAddr::Udp(addr) => {
if let Some(path_state) = self.udp_paths.paths.get_mut(&addr.into()) {
Expand Down Expand Up @@ -461,7 +462,7 @@ impl NodeState {
return None;
}
let tx_id = stun::TransactionId::default();
trace!(tx = %hex::encode(tx_id), %dst, ?purpose,
trace!(tx = %HEXLOWER.encode(&tx_id), %dst, ?purpose,
dst = %self.node_id.fmt_short(), "start ping");
event!(
target: "iroh::_events::ping::sent",
Expand All @@ -488,7 +489,7 @@ impl NodeState {
purpose: DiscoPingPurpose,
sender: mpsc::Sender<ActorMessage>,
) {
trace!(%to, tx = %hex::encode(tx_id), ?purpose, "record ping sent");
trace!(%to, tx = %HEXLOWER.encode(&tx_id), ?purpose, "record ping sent");

let now = Instant::now();
let mut path_found = false;
Expand Down Expand Up @@ -872,7 +873,7 @@ impl NodeState {
match self.sent_pings.remove(&m.tx_id) {
None => {
// This is not a pong for a ping we sent.
warn!(tx = %hex::encode(m.tx_id), "received pong with unknown transaction id");
warn!(tx = %HEXLOWER.encode(&m.tx_id), "received pong with unknown transaction id");
None
}
Some(sp) => {
Expand All @@ -884,7 +885,7 @@ impl NodeState {
let latency = now - sp.at;

debug!(
tx = %hex::encode(m.tx_id),
tx = %HEXLOWER.encode(&m.tx_id),
src = %src,
reported_ping_src = %m.ping_observed_addr,
ping_dst = %sp.to,
Expand Down
Loading