add(chain): Adds a `network_name` field to `testnet::Parameters` (#8411)

* minor cleanup and rename

* Adds an empty NetworkParameters struct to Network::Testnet variant, updates production code.

* Updates tests

* Adds `NetworkKind` and uses it instead of `Network` in `HistoryTreeParts` and `transparent::Address`

* Adds a [network.testnet_parameters] section to the config, uses `NetworkKind` as zebra_network::Config::network field type, and converts 'Network' to `NetworkKind` before serializing

* Applies some suggestions from code review

* Applies suggestions from code review

* returns b58 prefix constants directly to remove From<NetworkKind> impl for zcash_primitives::consensus::Network

* Applies more suggestions from code review.

* moves conversions to zcash_primitives::consensus::Network to where they're used.

* Apply suggestions from code review

Co-authored-by: Marek <mail@marek.onl>

* rename `network` variables and method names typed as NetworkKind to `network_kind`

* use only test block heights for the network associated with them

* Applies more suggestions from code review.

* Rename `NetworkParameters` to `Parameters` and move it a new `testnet` module

* adds activation heights field

* updates stored test config, adds a quicker test for checking that stored configs can be parsed, adds an intermediate representation of activation heights

* implement Parameters for Network

* Passes &Network directly instead of converting to zp_consensus::Network where there were conversions

* fixes a bad merge (removes a network conversion in zcash_note_encryption)

* Adds a test for the Parameters impl for zebra_chain::Network

* fixes doc links

* - Makes the `activation_heights` config field optional by adding a #[serde(default)]
- Panics if a non-zero activation height is provided for the `Genesis` network upgrade
- Always sets the `Genesis` and `BeforeOverwinter` network upgrade activation heights to 0 and 1, `BeforeOverwinter` could be overwritten by a later network upgrade
- Makes the `activation_heights` field on `Parameters` private, adds/uses an accessor method instead, and adds a builder struct and `build()` method

* small refactor of activation_heights() method

* check that activation heights are in the right order

* Updates `NetworkUpgrade::activation_height()` to return the height of the next NetworkUpgrade if it doesn't find the activation height of `&self`

* checks that the miner address is of TestnetKind on Regtest and update assertion message

* Adds a DNetworkUpgradeActivationHeights struct for better control over how activation heights can be configured

* moves all ordered network upgrades to a constant, adds a no_duplicates test, moves struct with activation heights outside deserialization impl and accepts it in `ParametersBuilder::activation_heights()` instead of a Vec

* panics if any network upgrades are configured to activate at Height(0) because it's reserved for Genesis

* Simplifies the `ParametersBuilder::activation_heights()` method and removes an unnecessary test.

* Adds Sapling HRPs as fields on testnet params. (#8398)

* Applies suggestions from code review.

* Update zebra-chain/src/parameters/network_upgrade.rs

Co-authored-by: Marek <mail@marek.onl>

* Adds `network_name` field and accessor method on `Parameters`, uses it in the `Display` impl for `Network`

* Removes unnecessary `.filter()`

* adds constraints on valid network names and a test

* adds config field for setting network name, adds "with_" prefix to ParameterBuilder setter methods

* Adds `MainnetKind`, `TestnetKind`, and `RegtestKind` to reserved network names

* updates stored test configs and fixes `network_name()` docs

---------

Co-authored-by: Marek <mail@marek.onl>

zebra-chain/src/parameters/network.rs

@@ -138,15 +138,13 @@ impl fmt::Display for NetworkKind {
     }
 }
 
-impl From<&Network> for &'static str {
-    fn from(network: &Network) -> &'static str {
+impl<'a> From<&'a Network> for &'a str {
+    fn from(network: &'a Network) -> &'a str {
         match network {
             Network::Mainnet => "Mainnet",
             // TODO:
-            // - Add a `name` field to use here instead of checking `is_default_testnet()`
             // - zcashd calls the Regtest cache dir 'regtest' (#8327).
-            Network::Testnet(params) if params.is_default_testnet() => "Testnet",
-            Network::Testnet(_params) => "UnknownTestnet",
+            Network::Testnet(params) => params.network_name(),
         }
     }
 }

zebra-chain/src/parameters/network/testnet.rs

@@ -1,5 +1,5 @@
 //! Types and implementation for Testnet consensus parameters
-use std::collections::BTreeMap;
+use std::{collections::BTreeMap, fmt};
 
 use zcash_primitives::constants as zp_constants;
 
@@ -11,6 +11,19 @@ use crate::{
     },
 };
 
+/// Reserved network names that should not be allowed for configured Testnets.
+pub const RESERVED_NETWORK_NAMES: [&str; 6] = [
+    "Mainnet",
+    "Testnet",
+    "Regtest",
+    "MainnetKind",
+    "TestnetKind",
+    "RegtestKind",
+];
+
+/// Maximum length for a configured network name.
+pub const MAX_NETWORK_NAME_LENGTH: usize = 30;
+
 /// Configurable activation heights for Regtest and configured Testnets.
 #[derive(Deserialize, Default)]
 #[serde(rename_all = "PascalCase")]
@@ -35,6 +48,8 @@ pub struct ConfiguredActivationHeights {
 /// Builder for the [`Parameters`] struct.
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Serialize, Deserialize)]
 pub struct ParametersBuilder {
+    /// The name of this network to be used by the `Display` trait impl.
+    network_name: String,
     /// The network upgrade activation heights for this network, see [`Parameters::activation_heights`] for more details.
     activation_heights: BTreeMap<Height, NetworkUpgrade>,
     /// Sapling extended spending key human-readable prefix for this network
@@ -48,6 +63,7 @@ pub struct ParametersBuilder {
 impl Default for ParametersBuilder {
     fn default() -> Self {
         Self {
+            network_name: "UnknownTestnet".to_string(),
             // # Correctness
             //
             // `Genesis` network upgrade activation height must always be 0
@@ -69,9 +85,33 @@ impl Default for ParametersBuilder {
 }
 
 impl ParametersBuilder {
+    /// Sets the network name to be used in the [`Parameters`] being built.
+    pub fn with_network_name(mut self, network_name: impl fmt::Display) -> Self {
+        self.network_name = network_name.to_string();
+
+        assert!(
+            !RESERVED_NETWORK_NAMES.contains(&self.network_name.as_str()),
+            "cannot use reserved network name '{network_name}' as configured Testnet name, reserved names: {RESERVED_NETWORK_NAMES:?}"
+        );
+
+        assert!(
+            self.network_name.len() <= MAX_NETWORK_NAME_LENGTH,
+            "network name {network_name} is too long, must be {MAX_NETWORK_NAME_LENGTH} characters or less"
+        );
+
+        assert!(
+            self.network_name
+                .chars()
+                .all(|x| x.is_alphanumeric() || x == '_'),
+            "network name must include only alphanumeric characters or '_'"
+        );
+
+        self
+    }
+
     /// Checks that the provided network upgrade activation heights are in the correct order, then
     /// sets them as the new network upgrade activation heights.
-    pub fn activation_heights(
+    pub fn with_activation_heights(
         mut self,
         ConfiguredActivationHeights {
             // TODO: Find out if `BeforeOverwinter` is required at Height(1), allow for
@@ -99,7 +139,6 @@ impl ParametersBuilder {
             .chain(heartwood.into_iter().map(|h| (h, Heartwood)))
             .chain(canopy.into_iter().map(|h| (h, Canopy)))
             .chain(nu5.into_iter().map(|h| (h, Nu5)))
-            .filter(|&(_, nu)| nu != NetworkUpgrade::BeforeOverwinter)
             .map(|(h, nu)| (h.try_into().expect("activation height must be valid"), nu))
             .collect();
 
@@ -136,12 +175,14 @@ impl ParametersBuilder {
     /// Converts the builder to a [`Parameters`] struct
     pub fn finish(self) -> Parameters {
         let Self {
+            network_name,
             activation_heights,
             hrp_sapling_extended_spending_key,
             hrp_sapling_extended_full_viewing_key,
             hrp_sapling_payment_address,
         } = self;
         Parameters {
+            network_name,
             activation_heights,
             hrp_sapling_extended_spending_key,
             hrp_sapling_extended_full_viewing_key,
@@ -158,6 +199,8 @@ impl ParametersBuilder {
 /// Network consensus parameters for test networks such as Regtest and the default Testnet.
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Serialize, Deserialize)]
 pub struct Parameters {
+    /// The name of this network to be used by the `Display` trait impl.
+    network_name: String,
     /// The network upgrade activation heights for this network.
     ///
     /// Note: This value is ignored by `Network::activation_list()` when `zebra-chain` is
@@ -176,13 +219,9 @@ impl Default for Parameters {
     /// Returns an instance of the default public testnet [`Parameters`].
     fn default() -> Self {
         Self {
+            network_name: "Testnet".to_string(),
             activation_heights: TESTNET_ACTIVATION_HEIGHTS.iter().cloned().collect(),
-            hrp_sapling_extended_spending_key:
-                zp_constants::testnet::HRP_SAPLING_EXTENDED_SPENDING_KEY.to_string(),
-            hrp_sapling_extended_full_viewing_key:
-                zp_constants::testnet::HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY.to_string(),
-            hrp_sapling_payment_address: zp_constants::testnet::HRP_SAPLING_PAYMENT_ADDRESS
-                .to_string(),
+            ..Self::build().finish()
         }
     }
 }
@@ -198,6 +237,11 @@ impl Parameters {
         self == &Self::default()
     }
 
+    /// Returns the network name
+    pub fn network_name(&self) -> &str {
+        &self.network_name
+    }
+
     /// Returns the network upgrade activation heights
     pub fn activation_heights(&self) -> &BTreeMap<Height, NetworkUpgrade> {
         &self.activation_heights

zebra-chain/src/parameters/network/tests/vectors.rs

@@ -5,7 +5,9 @@ use zcash_primitives::consensus::{self as zp_consensus, Parameters};
 use crate::{
     block::Height,
     parameters::{
-        testnet::{self, ConfiguredActivationHeights},
+        testnet::{
+            self, ConfiguredActivationHeights, MAX_NETWORK_NAME_LENGTH, RESERVED_NETWORK_NAMES,
+        },
         Network, NetworkUpgrade, NETWORK_UPGRADES_IN_ORDER,
     },
 };
@@ -97,7 +99,7 @@ fn check_parameters_impl() {
 fn activates_network_upgrades_correctly() {
     let expected_activation_height = 1;
     let network = testnet::Parameters::build()
-        .activation_heights(ConfiguredActivationHeights {
+        .with_activation_heights(ConfiguredActivationHeights {
             nu5: Some(expected_activation_height),
             ..Default::default()
         })
@@ -125,3 +127,58 @@ fn activates_network_upgrades_correctly() {
         );
     }
 }
+
+/// Checks that configured testnet names are validated and used correctly.
+#[test]
+fn check_network_name() {
+    // Sets a no-op panic hook to avoid long output.
+    std::panic::set_hook(Box::new(|_| {}));
+
+    // Checks that reserved network names cannot be used for configured testnets.
+    for reserved_network_name in RESERVED_NETWORK_NAMES {
+        std::panic::catch_unwind(|| {
+            testnet::Parameters::build().with_network_name(reserved_network_name)
+        })
+        .expect_err("should panic when attempting to set network name as a reserved name");
+    }
+
+    // Check that max length is enforced, and that network names may only contain alphanumeric characters and '_'.
+    for invalid_network_name in [
+        "a".repeat(MAX_NETWORK_NAME_LENGTH + 1),
+        "!!!!non-alphanumeric-name".to_string(),
+    ] {
+        std::panic::catch_unwind(|| {
+            testnet::Parameters::build().with_network_name(invalid_network_name)
+        })
+        .expect_err("should panic when setting network name that's too long or contains non-alphanumeric characters (except '_')");
+    }
+
+    // Checks that network names are displayed correctly
+    assert_eq!(
+        Network::new_default_testnet().to_string(),
+        "Testnet",
+        "default testnet should be displayed as 'Testnet'"
+    );
+    assert_eq!(
+        Network::Mainnet.to_string(),
+        "Mainnet",
+        "Mainnet should be displayed as 'Mainnet'"
+    );
+
+    // TODO: Check Regtest
+
+    // Check that network name can contain alphanumeric characters and '_'.
+    let expected_name = "ConfiguredTestnet_1";
+    let network = testnet::Parameters::build()
+        // Check that network name can contain `MAX_NETWORK_NAME_LENGTH` characters
+        .with_network_name("a".repeat(MAX_NETWORK_NAME_LENGTH))
+        .with_network_name(expected_name)
+        .to_network();
+
+    // Check that configured network name is displayed
+    assert_eq!(
+        network.to_string(),
+        expected_name,
+        "network must be displayed as configured network name"
+    );
+}

zebra-network/src/config.rs

@@ -629,8 +629,9 @@ impl<'de> Deserialize<'de> for Config {
     {
         #[derive(Deserialize)]
         struct DTestnetParameters {
+            network_name: Option<String>,
             #[serde(default)]
-            pub(super) activation_heights: ConfiguredActivationHeights,
+            activation_heights: ConfiguredActivationHeights,
         }
 
         #[derive(Deserialize)]
@@ -678,15 +679,25 @@ impl<'de> Deserialize<'de> for Config {
         } = DConfig::deserialize(deserializer)?;
 
         // TODO: Panic here if the initial testnet peers are the default initial testnet peers.
-        let network = if let Some(DTestnetParameters { activation_heights }) = testnet_parameters {
+        let network = if let Some(DTestnetParameters {
+            network_name,
+            activation_heights,
+        }) = testnet_parameters
+        {
             assert_eq!(
                 network_kind,
                 NetworkKind::Testnet,
                 "set network to 'Testnet' to use configured testnet parameters"
             );
 
-            testnet::Parameters::build()
-                .activation_heights(activation_heights)
+            let mut params_builder = testnet::Parameters::build();
+
+            if let Some(network_name) = network_name {
+                params_builder = params_builder.with_network_name(network_name)
+            }
+
+            params_builder
+                .with_activation_heights(activation_heights)
                 .to_network()
         } else {
             // Convert to default `Network` for a `NetworkKind` if there are no testnet parameters.

zebrad/tests/common/configs/v1.7.0.toml

@@ -60,6 +60,9 @@ max_connections_per_ip = 1
 network = "Testnet"
 peerset_initial_target_size = 25
 
+[network.testnet_parameters]
+network_name = "ConfiguredTestnet_1"
+
 [network.testnet_parameters.activation_heights]
 BeforeOverwinter = 1
 Overwinter = 207_500