refactor(app): De-duplicate and fix version handling code (#6996)

* De-duplicate app_version and user_agent code, rename to build_version

* Make RPC testnet flag forward-compatible with additional testnets

* Fix RPC tests with new argument

* Use "modified" rather than "dirty" for uncommitted changes in build metadata

* Split the vergen version into its own function
This commit is contained in:
teor 2023-06-20 12:42:06 +10:00 committed by GitHub
parent fba9440af0
commit c3f0f53256
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 139 additions and 77 deletions

View File

@ -59,7 +59,7 @@ pub enum Network {
#[default]
Mainnet,
/// The testnet.
/// The oldest public test network.
Testnet,
}
@ -124,6 +124,11 @@ impl Network {
pub fn lowercase_name(&self) -> String {
self.to_string().to_ascii_lowercase()
}
/// Returns `true` if this network is a testing network.
pub fn is_a_test_network(&self) -> bool {
*self != Network::Mainnet
}
}
impl FromStr for Network {

View File

@ -251,8 +251,11 @@ where
{
// Configuration
//
/// Zebra's application version.
app_version: String,
/// Zebra's application version, with build metadata.
build_version: String,
/// Zebra's RPC user agent.
user_agent: String,
/// The configured network for this RPC service.
network: Network,
@ -300,8 +303,13 @@ where
Tip: ChainTip + Clone + Send + Sync + 'static,
{
/// Create a new instance of the RPC handler.
pub fn new<Version>(
app_version: Version,
//
// TODO:
// - put some of the configs or services in their own struct?
#[allow(clippy::too_many_arguments)]
pub fn new<VersionString, UserAgentString>(
build_version: VersionString,
user_agent: UserAgentString,
network: Network,
debug_force_finished_sync: bool,
debug_like_zcashd: bool,
@ -310,21 +318,24 @@ where
latest_chain_tip: Tip,
) -> (Self, JoinHandle<()>)
where
Version: ToString,
VersionString: ToString + Clone + Send + 'static,
UserAgentString: ToString + Clone + Send + 'static,
<Mempool as Service<mempool::Request>>::Future: Send,
<State as Service<zebra_state::ReadRequest>>::Future: Send,
{
let (runner, queue_sender) = Queue::start();
let mut app_version = app_version.to_string();
let mut build_version = build_version.to_string();
let user_agent = user_agent.to_string();
// Match zcashd's version format, if the version string has anything in it
if !app_version.is_empty() && !app_version.starts_with('v') {
app_version.insert(0, 'v');
if !build_version.is_empty() && !build_version.starts_with('v') {
build_version.insert(0, 'v');
}
let rpc_impl = RpcImpl {
app_version,
build_version,
user_agent,
network,
debug_force_finished_sync,
debug_like_zcashd,
@ -364,25 +375,10 @@ where
State::Future: Send,
Tip: ChainTip + Clone + Send + Sync + 'static,
{
#[allow(clippy::unwrap_in_result)]
fn get_info(&self) -> Result<GetInfo> {
// Build a [BIP 14] valid user agent with release info.
//
// [BIP 14]: https://github.com/bitcoin/bips/blob/master/bip-0014.mediawiki
let release_version = self
.app_version
// remove everything after the `+` character if any
.split('+')
.next()
.expect("always at least 1 slice");
// Remove the previously added `v` character at the start since it's not a part of the user agent.
let release_version = release_version.strip_prefix('v').unwrap_or(release_version);
let user_agent = format!("/Zebra:{release_version}/");
let response = GetInfo {
build: self.app_version.clone(),
subversion: user_agent,
build: self.build_version.clone(),
subversion: self.user_agent.clone(),
};
Ok(response)

View File

@ -25,7 +25,7 @@ impl Response {
networksolps,
networkhashps: networksolps,
chain: network.bip70_network_name(),
testnet: network == Network::Testnet,
testnet: network.is_a_test_network(),
}
}
}

View File

@ -40,6 +40,7 @@ proptest! {
let mut mempool = MockService::build().for_prop_tests();
let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -94,6 +95,7 @@ proptest! {
let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -153,6 +155,7 @@ proptest! {
let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -220,6 +223,7 @@ proptest! {
let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -276,6 +280,7 @@ proptest! {
let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -330,6 +335,7 @@ proptest! {
let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -430,6 +436,7 @@ proptest! {
let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -488,6 +495,7 @@ proptest! {
let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -535,6 +543,7 @@ proptest! {
// look for an error with a `NoChainTip`
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
network,
false,
@ -585,6 +594,7 @@ proptest! {
// Start RPC with the mocked `ChainTip`
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
network,
false,
@ -671,6 +681,7 @@ proptest! {
// Start RPC with the mocked `ChainTip`
runtime.block_on(async move {
let (rpc, _rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
network,
false,
@ -734,6 +745,7 @@ proptest! {
// Start RPC with the mocked `ChainTip`
runtime.block_on(async move {
let (rpc, _rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
network,
false,
@ -785,6 +797,7 @@ proptest! {
let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -874,6 +887,7 @@ proptest! {
let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,

View File

@ -72,6 +72,7 @@ async fn test_rpc_response_data_for_network(network: Network) {
// Init RPC
let (rpc, _rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"/Zebra:RPC test/",
network,
false,
true,

View File

@ -29,6 +29,7 @@ async fn rpc_getinfo() {
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"/Zebra:RPC test/",
Mainnet,
false,
true,
@ -72,6 +73,7 @@ async fn rpc_getblock() {
// Init RPC
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -223,6 +225,7 @@ async fn rpc_getblock_parse_error() {
// Init RPC
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -265,6 +268,7 @@ async fn rpc_getblock_missing_error() {
// Init RPC
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -333,6 +337,7 @@ async fn rpc_getbestblockhash() {
// Init RPC
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -378,6 +383,7 @@ async fn rpc_getrawtransaction() {
// Init RPC
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -539,6 +545,7 @@ async fn rpc_getaddresstxids_invalid_arguments() {
zebra_state::populated_state(blocks.clone(), Mainnet).await;
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -682,6 +689,7 @@ async fn rpc_getaddresstxids_response_with(
zebra_state::populated_state(blocks.to_owned(), network).await;
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
"RPC test",
network,
false,
@ -733,6 +741,7 @@ async fn rpc_getaddressutxos_invalid_arguments() {
let mut state: MockService<_, _, _, BoxError> = MockService::build().for_unit_tests();
let rpc = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,
@ -781,6 +790,7 @@ async fn rpc_getaddressutxos_response() {
zebra_state::populated_state(blocks.clone(), Mainnet).await;
let rpc = RpcImpl::new(
"RPC test",
"RPC test",
Mainnet,
false,

View File

@ -43,9 +43,16 @@ mod tests;
/// Zebra RPC Server
#[derive(Clone)]
pub struct RpcServer {
/// The RPC config.
config: Config,
/// The configured network.
network: Network,
app_version: String,
/// Zebra's application version, with build metadata.
build_version: String,
/// A handle that shuts down the RPC server.
close_handle: CloseHandle,
}
@ -54,7 +61,7 @@ impl fmt::Debug for RpcServer {
f.debug_struct("RpcServer")
.field("config", &self.config)
.field("network", &self.network)
.field("app_version", &self.app_version)
.field("build_version", &self.build_version)
.field(
"close_handle",
// TODO: when it stabilises, use std::any::type_name_of_val(&self.close_handle)
@ -66,21 +73,35 @@ impl fmt::Debug for RpcServer {
impl RpcServer {
/// Start a new RPC server endpoint using the supplied configs and services.
/// `app_version` is a version string for the application, which is used in RPC responses.
///
/// `build_version` and `user_agent` are version strings for the application,
/// which are used in RPC responses.
///
/// Returns [`JoinHandle`]s for the RPC server and `sendrawtransaction` queue tasks,
/// and a [`RpcServer`] handle, which can be used to shut down the RPC server task.
//
// TODO: put some of the configs or services in their own struct?
// TODO:
// - put some of the configs or services in their own struct?
// - replace VersionString with semver::Version, and update the tests to provide valid versions
#[allow(clippy::too_many_arguments)]
pub fn spawn<Version, Mempool, State, Tip, BlockVerifierRouter, SyncStatus, AddressBook>(
pub fn spawn<
VersionString,
UserAgentString,
Mempool,
State,
Tip,
BlockVerifierRouter,
SyncStatus,
AddressBook,
>(
config: Config,
#[cfg(feature = "getblocktemplate-rpcs")]
mining_config: get_block_template_rpcs::config::Config,
#[cfg(not(feature = "getblocktemplate-rpcs"))]
#[allow(unused_variables)]
mining_config: (),
app_version: Version,
build_version: VersionString,
user_agent: UserAgentString,
mempool: Buffer<Mempool, mempool::Request>,
state: State,
#[cfg_attr(not(feature = "getblocktemplate-rpcs"), allow(unused_variables))]
@ -93,7 +114,8 @@ impl RpcServer {
network: Network,
) -> (JoinHandle<()>, JoinHandle<()>, Option<Self>)
where
Version: ToString + Clone + Send + 'static,
VersionString: ToString + Clone + Send + 'static,
UserAgentString: ToString + Clone + Send + 'static,
Mempool: tower::Service<
mempool::Request,
Response = mempool::Response,
@ -159,7 +181,8 @@ impl RpcServer {
// Initialize the rpc methods with the zebra version
let (rpc_impl, rpc_tx_queue_task_handle) = RpcImpl::new(
app_version.clone(),
build_version.clone(),
user_agent,
network,
config.debug_force_finished_sync,
#[cfg(feature = "getblocktemplate-rpcs")]
@ -202,7 +225,7 @@ impl RpcServer {
let rpc_server_handle = RpcServer {
config,
network,
app_version: app_version.to_string(),
build_version: build_version.to_string(),
close_handle,
};

View File

@ -61,6 +61,7 @@ fn rpc_server_spawn(parallel_cpu_threads: bool) {
config,
Default::default(),
"RPC server test",
"RPC server test",
Buffer::new(mempool.clone(), 1),
Buffer::new(state.clone(), 1),
Buffer::new(router_verifier.clone(), 1),
@ -147,6 +148,7 @@ fn rpc_server_spawn_unallocated_port(parallel_cpu_threads: bool, do_shutdown: bo
config,
Default::default(),
"RPC server test",
"RPC server test",
Buffer::new(mempool.clone(), 1),
Buffer::new(state.clone(), 1),
Buffer::new(router_verifier.clone(), 1),
@ -227,6 +229,7 @@ fn rpc_server_spawn_port_conflict() {
config.clone(),
Default::default(),
"RPC server 1 test",
"RPC server 1 test",
Buffer::new(mempool.clone(), 1),
Buffer::new(state.clone(), 1),
Buffer::new(router_verifier.clone(), 1),
@ -244,6 +247,7 @@ fn rpc_server_spawn_port_conflict() {
config,
Default::default(),
"RPC server 2 conflict test",
"RPC server 2 conflict test",
Buffer::new(mempool.clone(), 1),
Buffer::new(state.clone(), 1),
Buffer::new(router_verifier.clone(), 1),
@ -335,6 +339,7 @@ fn rpc_server_spawn_port_conflict_parallel_auto() {
config.clone(),
Default::default(),
"RPC server 1 test",
"RPC server 1 test",
Buffer::new(mempool.clone(), 1),
Buffer::new(state.clone(), 1),
Buffer::new(router_verifier.clone(), 1),
@ -352,6 +357,7 @@ fn rpc_server_spawn_port_conflict_parallel_auto() {
config,
Default::default(),
"RPC server 2 conflict test",
"RPC server 2 conflict test",
Buffer::new(mempool.clone(), 1),
Buffer::new(state.clone(), 1),
Buffer::new(router_verifier.clone(), 1),

View File

@ -132,6 +132,7 @@ chrono = { version = "0.4.26", default-features = false, features = ["clock", "s
humantime-serde = "1.1.1"
indexmap = "1.9.3"
lazy_static = "1.4.0"
semver = "1.0.17"
serde = { version = "1.0.164", features = ["serde_derive"] }
toml = "0.7.4"
@ -207,7 +208,6 @@ hex = "0.4.3"
jsonrpc-core = "18.0.0"
once_cell = "1.18.0"
regex = "1.8.4"
semver = "1.0.17"
# zebra-rpc needs the preserve_order feature, it also makes test results more stable
serde_json = { version = "1.0.97", features = ["preserve_order"] }

View File

@ -7,8 +7,9 @@ use abscissa_core::{
config::CfgCell,
status_err,
terminal::{component::Terminal, stderr, stdout, ColorChoice},
Application, Component, Configurable, FrameworkError, Shutdown, StandardPaths, Version,
Application, Component, Configurable, FrameworkError, Shutdown, StandardPaths,
};
use semver::{BuildMetadata, Version};
use zebra_network::constants::PORT_IN_USE_ERROR;
use zebra_state::constants::{DATABASE_FORMAT_VERSION, LOCK_FILE_ERROR};
@ -29,25 +30,17 @@ fn fatal_error(app_name: String, err: &dyn std::error::Error) -> ! {
/// Application state
pub static APPLICATION: AppCell<ZebradApp> = AppCell::new();
/// Returns the zebrad version for this build, in SemVer 2.0 format.
/// Returns the `zebrad` version for this build, in SemVer 2.0 format.
///
/// Includes the git commit and the number of commits since the last version
/// tag, if available.
/// Includes `git describe` build metatata if available:
/// - the number of commits since the last version tag, and
/// - the git commit.
///
/// For details, see <https://semver.org/>
pub fn app_version() -> Version {
pub fn build_version() -> Version {
// CARGO_PKG_VERSION is always a valid SemVer 2.0 version.
const CARGO_PKG_VERSION: &str = env!("CARGO_PKG_VERSION");
// VERGEN_GIT_DESCRIBE should be in the format:
// - v1.0.0-rc.9-6-g319b01bb84
// - v1.0.0-6-g319b01bb84
// but sometimes it is just a short commit hash. See #6879 for details.
//
// Currently it is the output of `git describe --tags --dirty --match='v*.*.*'`,
// or whatever is specified in zebrad/build.rs.
const VERGEN_GIT_DESCRIBE: Option<&str> = option_env!("VERGEN_GIT_DESCRIBE");
// We're using the same library as cargo uses internally, so this is guaranteed.
let fallback_version = CARGO_PKG_VERSION.parse().unwrap_or_else(|error| {
panic!(
@ -56,6 +49,20 @@ pub fn app_version() -> Version {
)
});
vergen_build_version().unwrap_or(fallback_version)
}
/// Returns the `zebrad` version from this build, if available from `vergen`.
fn vergen_build_version() -> Option<Version> {
// VERGEN_GIT_DESCRIBE should be in the format:
// - v1.0.0-rc.9-6-g319b01bb84
// - v1.0.0-6-g319b01bb84
// but sometimes it is just a short commit hash. See #6879 for details.
//
// Currently it is the output of `git describe --tags --dirty --match='v*.*.*'`,
// or whatever is specified in zebrad/build.rs.
const VERGEN_GIT_DESCRIBE: Option<&str> = option_env!("VERGEN_GIT_DESCRIBE");
// The SemVer 2.0 format is:
// - 1.0.0-rc.9+6.g319b01bb84
// - 1.0.0+6.g319b01bb84
@ -66,16 +73,20 @@ pub fn app_version() -> Version {
// - optional build: `+`tag[`.`tag ...]
// change the git describe format to the semver 2.0 format
let Some(vergen_git_describe) = VERGEN_GIT_DESCRIBE else {
return fallback_version;
return None;
};
// `git describe` uses "dirty" for uncommitted changes,
// but users won't understand what that means.
let vergen_git_describe = vergen_git_describe.replace("dirty", "modified");
// Split using "git describe" separators.
let mut vergen_git_describe = vergen_git_describe.split('-').peekable();
// Check the "version core" part.
let version = vergen_git_describe.next();
let Some(mut version) = version else {
return fallback_version;
return None;
};
// strip the leading "v", if present.
@ -83,7 +94,7 @@ pub fn app_version() -> Version {
// If the initial version is empty, just a commit hash, or otherwise invalid.
if Version::parse(version).is_err() {
return fallback_version;
return None;
}
let mut semver = version.to_string();
@ -92,7 +103,7 @@ pub fn app_version() -> Version {
// but only consume it if it is a pre-release tag.
let Some(part) = vergen_git_describe.peek() else {
// No pre-release or build.
return semver.parse().expect("just checked semver is valid");
return semver.parse().ok();
};
if part.starts_with(char::is_alphabetic) {
@ -107,12 +118,12 @@ pub fn app_version() -> Version {
// Check if the next part is a build part.
let Some(build) = vergen_git_describe.peek() else {
// No build tags.
return semver.parse().unwrap_or(fallback_version);
return semver.parse().ok();
};
if !build.starts_with(char::is_numeric) {
// It's not a valid "commit count" build tag from "git describe".
return fallback_version;
return None;
}
// Append the rest of the build parts with the correct `+` and `.` separators.
@ -122,19 +133,16 @@ pub fn app_version() -> Version {
semver.push('+');
semver.push_str(&build_parts);
semver.parse().unwrap_or(fallback_version)
semver.parse().ok()
}
/// The Zebra current release version.
//
// TODO: deduplicate this code with release_version in zebra_rpc::get_info()
pub fn release_version() -> String {
app_version()
.to_string()
.split('+')
.next()
.expect("always at least 1 slice")
.to_string()
/// The Zebra current release version, without any build metadata.
pub fn release_version() -> Version {
let mut release_version = build_version();
release_version.build = BuildMetadata::EMPTY;
release_version
}
/// The User-Agent string provided by the node.
@ -142,8 +150,6 @@ pub fn release_version() -> String {
/// This must be a valid [BIP 14] user agent.
///
/// [BIP 14]: https://github.com/bitcoin/bips/blob/master/bip-0014.mediawiki
//
// TODO: deduplicate this code with the user agent in zebra_rpc::get_info()
pub fn user_agent() -> String {
let release_version = release_version();
format!("/Zebra:{release_version}/")
@ -260,7 +266,7 @@ impl Application for ZebradApp {
let app_metadata = vec![
// cargo or git tag + short commit
("version", app_version().to_string()),
("version", build_version().to_string()),
// config
("Zcash network", config.network.network.to_string()),
// constants
@ -368,7 +374,7 @@ impl Application for ZebradApp {
#[cfg(feature = "sentry")]
let guard = sentry::init(sentry::ClientOptions {
debug: true,
release: Some(app_version().to_string().into()),
release: Some(build_version().to_string().into()),
..Default::default()
});

View File

@ -82,7 +82,7 @@ use zebra_consensus::router::BackgroundTaskHandles;
use zebra_rpc::server::RpcServer;
use crate::{
application::{app_version, user_agent},
application::{build_version, user_agent},
components::{
inbound::{self, InboundSetupData, MAX_INBOUND_RESPONSE_TIME},
mempool::{self, Mempool},
@ -215,7 +215,8 @@ impl StartCmd {
config.mining.clone(),
#[cfg(not(feature = "getblocktemplate-rpcs"))]
(),
app_version(),
build_version(),
user_agent(),
mempool.clone(),
read_only_state_service,
router_verifier,

View File

@ -16,7 +16,7 @@ use tracing_subscriber::{
EnvFilter,
};
use crate::{application::app_version, components::tracing::Config};
use crate::{application::build_version, components::tracing::Config};
#[cfg(feature = "flamegraph")]
use super::flame;
@ -341,7 +341,7 @@ impl<A: abscissa_core::Application> Component<A> for Tracing {
}
fn version(&self) -> abscissa_core::Version {
app_version()
build_version()
}
fn before_shutdown(&self, _kind: Shutdown) -> Result<(), FrameworkError> {