Skip to content

Commit

Permalink
restricted build version to be a semver
Browse files Browse the repository at this point in the history
  • Loading branch information
pompon0 committed Aug 21, 2024
1 parent 08c80ae commit 631358e
Show file tree
Hide file tree
Showing 13 changed files with 53 additions and 15 deletions.
2 changes: 2 additions & 0 deletions node/Cargo.lock

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

1 change: 1 addition & 0 deletions node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ quote = "1.0.33"
rand = "0.8.0"
rand04 = { package = "rand", version = "0.4" }
rocksdb = "0.21.0"
semver = "1.0.23"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0.95"
serde_yaml = "0.9"
Expand Down
1 change: 1 addition & 0 deletions node/actors/executor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ zksync_protobuf.workspace = true
anyhow.workspace = true
async-trait.workspace = true
rand.workspace = true
semver.workspace = true
tracing.workspace = true
vise.workspace = true

Expand Down
3 changes: 1 addition & 2 deletions node/actors/executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ pub struct Validator {
#[derive(Debug)]
pub struct Config {
/// Label identifying the build version of the binary that this node is running.
/// There is no specific semantics assigned to it.
pub build_version: Option<String>,
pub build_version: Option<semver::Version>,
/// IP:port to listen on, for incoming TCP connections.
/// Use `0.0.0.0:<port>` to listen on all network interfaces (i.e. on all IPs exposed by this VM).
pub server_addr: std::net::SocketAddr,
Expand Down
1 change: 1 addition & 0 deletions node/actors/network/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ tls-listener.workspace = true
base64.workspace = true
build_html.workspace = true
bytesize.workspace = true
semver.workspace = true

[dev-dependencies]
assert_matches.workspace = true
Expand Down
2 changes: 1 addition & 1 deletion node/actors/network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub struct GossipConfig {
pub struct Config {
/// Label identifying the build version of the binary that this node is running.
/// There is no specific semantics assigned to it.
pub build_version: Option<String>,
pub build_version: Option<semver::Version>,
/// TCP socket address to listen for inbound connections at.
pub server_addr: net::tcp::ListenerAddr,
/// Public TCP address that other nodes are expected to connect to.
Expand Down
11 changes: 8 additions & 3 deletions node/actors/network/src/gossip/handshake/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub(crate) struct Handshake {
pub(crate) is_static: bool,
/// Version at which peer's binary has been built.
/// It is declared by peer (i.e. not verified in any way).
pub(crate) build_version: Option<String>,
pub(crate) build_version: Option<semver::Version>,
}

impl ProtoFmt for Handshake {
Expand All @@ -42,15 +42,20 @@ impl ProtoFmt for Handshake {
session_id: read_required(&r.session_id).context("session_id")?,
genesis: read_required(&r.genesis).context("genesis")?,
is_static: *required(&r.is_static).context("is_static")?,
build_version: r.build_version.clone(),
build_version: r
.build_version
.as_ref()
.map(|x| x.parse())
.transpose()
.context("build_version")?,
})
}
fn build(&self) -> Self::Proto {
Self::Proto {
session_id: Some(self.session_id.build()),
genesis: Some(self.genesis.build()),
is_static: Some(self.is_static),
build_version: self.build_version.clone(),
build_version: self.build_version.as_ref().map(|x| x.to_string()),
}
}
}
Expand Down
21 changes: 19 additions & 2 deletions node/actors/network/src/gossip/handshake/testonly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,28 @@
//! if tests require stricter properties of the generated data.
use super::Handshake;
use rand::{
distributions::{Alphanumeric, DistString, Distribution, Standard},
distributions::{Distribution, Standard},
Rng,
};
use zksync_consensus_roles::node;

/// Semver has specific restrictions on how an identifier
/// should look like, so we play it safe and generate
/// a letter-only string of fixed length.
fn gen_semver_identifier<R: Rng + ?Sized>(rng: &mut R) -> String {
(0..10).map(|_| rng.gen_range('a'..='z')).collect()
}

fn gen_semver<R: Rng + ?Sized>(rng: &mut R) -> semver::Version {
semver::Version {
major: rng.gen(),
minor: rng.gen(),
patch: rng.gen(),
pre: semver::Prerelease::new(&gen_semver_identifier(rng)).unwrap(),
build: semver::BuildMetadata::new(&gen_semver_identifier(rng)).unwrap(),
}
}

impl Distribution<Handshake> for Standard {
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> Handshake {
let key: node::SecretKey = rng.gen();
Expand All @@ -17,7 +34,7 @@ impl Distribution<Handshake> for Standard {
session_id: key.sign_msg(session_id),
genesis: rng.gen(),
is_static: rng.gen(),
build_version: Some(Alphanumeric.sample_string(rng, 10)),
build_version: Some(gen_semver(rng)),
}
}
}
2 changes: 1 addition & 1 deletion node/actors/network/src/gossip/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub(crate) struct Connection {
/// Peer's public key.
pub(crate) key: node::PublicKey,
/// Build version of peer's binary (not verified).
pub(crate) build_version: Option<String>,
pub(crate) build_version: Option<semver::Version>,
/// TCP connection stats.
pub(crate) stats: Arc<MeteredStreamStats>,
}
Expand Down
6 changes: 4 additions & 2 deletions node/actors/network/src/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,9 @@ impl DebugPageServer {

fn connections_html<'a>(
&self,
connections: impl Iterator<Item = (String, &'a Arc<MeteredStreamStats>, Option<String>)>,
connections: impl Iterator<
Item = (String, &'a Arc<MeteredStreamStats>, Option<semver::Version>),
>,
) -> String {
let mut table = Table::new()
.with_custom_header_row(
Expand Down Expand Up @@ -238,7 +240,7 @@ impl DebugPageServer {
table.add_body_row(vec![
Self::shorten(key),
stats.peer_addr.to_string(),
build_version.unwrap_or_default(),
build_version.map(|v| v.to_string()).unwrap_or_default(),
bytesize::to_string(received, false),
// TODO: this is not useful - we should display avg from the last ~1min instead.
bytesize::to_string(received / age.as_secs(), false) + "/s",
Expand Down
14 changes: 12 additions & 2 deletions node/actors/network/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,23 @@ impl NetworkGauges {
let inbound = state.gossip.inbound.current();
gauges.gossip_inbound_connections.set(inbound.len());
for conn in inbound.values() {
let v = BuildVersion(conn.build_version.clone().unwrap_or_default());
let v = BuildVersion(
conn.build_version
.as_ref()
.map(|v| v.to_string())
.unwrap_or_default(),
);
gauges.gossip_peers_by_build_version[&v].inc_by(1);
}
let outbound = state.gossip.outbound.current();
gauges.gossip_outbound_connections.set(outbound.len());
for conn in outbound.values() {
let v = BuildVersion(conn.build_version.clone().unwrap_or_default());
let v = BuildVersion(
conn.build_version
.as_ref()
.map(|v| v.to_string())
.unwrap_or_default(),
);
gauges.gossip_peers_by_build_version[&v].inc_by(1);
}
if let Some(consensus_state) = &state.consensus {
Expand Down
2 changes: 1 addition & 1 deletion node/actors/network/src/proto/gossip.proto
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ message Handshake {
optional bool is_static = 2; // required

// Optional information about the node.
optional string build_version = 4; // optional
optional string build_version = 4; // SemVer; optional
}

message PushValidatorAddrs {
Expand Down
2 changes: 1 addition & 1 deletion node/tools/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ impl Configs {

let e = executor::Executor {
config: executor::Config {
build_version: Some(CRATE_VERSION.into()),
build_version: Some(CRATE_VERSION.parse().context("CRATE_VERSION.parse()")?),
server_addr: self.app.server_addr,
public_addr: self.app.public_addr.clone(),
node_key: self.app.node_key.clone(),
Expand Down

0 comments on commit 631358e

Please sign in to comment.