diff --git a/core/primitives-core/src/parameter.rs b/core/primitives-core/src/parameter.rs index 547dbe4900b..f54e9e493dc 100644 --- a/core/primitives-core/src/parameter.rs +++ b/core/primitives-core/src/parameter.rs @@ -18,10 +18,8 @@ use crate::config::ActionCosts; #[strum(serialize_all = "snake_case")] pub enum Parameter { // Gas economics config - BurntGasRewardNumerator, - BurntGasRewardDenominator, - PessimisticGasPriceInflationNumerator, - PessimisticGasPriceInflationDenominator, + BurntGasReward, + PessimisticGasPriceInflation, // Account creation config MinAllowedTopLevelAccountLength, diff --git a/core/primitives/res/runtime_configs/parameters.yaml b/core/primitives/res/runtime_configs/parameters.yaml index f6224056621..8fb5fb4a8cf 100644 --- a/core/primitives/res/runtime_configs/parameters.yaml +++ b/core/primitives/res/runtime_configs/parameters.yaml @@ -3,10 +3,14 @@ # The diffs are stored in files named `NN.txt`, where `NN` is the version. # Gas economics config -burnt_gas_reward_numerator: 3 -burnt_gas_reward_denominator: 10 -pessimistic_gas_price_inflation_numerator: 103 -pessimistic_gas_price_inflation_denominator: 100 +burnt_gas_reward: { + numerator: 3, + denominator: 10, +} +pessimistic_gas_price_inflation: { + numerator: 103, + denominator: 100, +} # Account creation config min_allowed_top_level_account_length: 32 diff --git a/core/primitives/res/runtime_configs/parameters_testnet.yaml b/core/primitives/res/runtime_configs/parameters_testnet.yaml index 84c72591272..0fa409d435c 100644 --- a/core/primitives/res/runtime_configs/parameters_testnet.yaml +++ b/core/primitives/res/runtime_configs/parameters_testnet.yaml @@ -1,8 +1,12 @@ # Gas economics config -burnt_gas_reward_numerator: 3 -burnt_gas_reward_denominator: 10 -pessimistic_gas_price_inflation_numerator: 103 -pessimistic_gas_price_inflation_denominator: 100 +burnt_gas_reward: { + numerator: 3, + denominator: 10, +} +pessimistic_gas_price_inflation: { + numerator: 103, + denominator: 100, +} # Account creation config min_allowed_top_level_account_length: 0 diff --git a/core/primitives/src/runtime/parameter_table.rs b/core/primitives/src/runtime/parameter_table.rs index 4d4edb9f6f6..a127cbd774d 100644 --- a/core/primitives/src/runtime/parameter_table.rs +++ b/core/primitives/src/runtime/parameter_table.rs @@ -1,23 +1,56 @@ use super::config::{AccountCreationConfig, RuntimeConfig}; +use near_primitives_core::account::id::ParseAccountError; use near_primitives_core::config::{ExtCostsConfig, VMConfig}; use near_primitives_core::parameter::{FeeParameter, Parameter}; use near_primitives_core::runtime::fees::{RuntimeFeesConfig, StorageUsageConfig}; +use near_primitives_core::types::AccountId; use num_rational::Rational; -use serde::de::DeserializeOwned; -use std::any::Any; use std::collections::BTreeMap; +/// Represents values supported by parameter config. +#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, PartialEq)] +#[serde(untagged)] +pub(crate) enum ParameterValue { + U64(u64), + Rational { numerator: isize, denominator: isize }, + String(String), +} + +impl ParameterValue { + fn as_u64(&self) -> Option { + match self { + ParameterValue::U64(v) => Some(*v), + _ => None, + } + } + + fn as_rational(&self) -> Option { + match self { + ParameterValue::Rational { numerator, denominator } => { + Some(Rational::new(*numerator, *denominator)) + } + _ => None, + } + } + + fn as_str(&self) -> Option<&str> { + match self { + ParameterValue::String(v) => Some(v), + _ => None, + } + } +} + pub(crate) struct ParameterTable { - parameters: BTreeMap, + parameters: BTreeMap, } /// Changes made to parameters between versions. pub(crate) struct ParameterTableDiff { - parameters: BTreeMap, + parameters: BTreeMap, Option)>, } -/// Error returned by ParameterTable::from_txt() that parses a runtime -/// configuration TXT file. +/// Error returned by ParameterTable::from_str() that parses a runtime configuration YAML file. #[derive(thiserror::Error, Debug)] pub(crate) enum InvalidConfigError { #[error("could not parse `{1}` as a parameter")] @@ -27,17 +60,21 @@ pub(crate) enum InvalidConfigError { #[error("could not parse YAML that defines the structure of the config")] InvalidYaml(#[source] serde_yaml::Error), #[error("config diff expected to contain old value `{1:?}` for parameter `{0}`")] - OldValueExists(Parameter, serde_yaml::Value), + OldValueExists(Parameter, ParameterValue), #[error( "unexpected old value `{1:?}` for parameter `{0}` in config diff, previous version does not have such a value" )] - NoOldValueExists(Parameter, serde_yaml::Value), + NoOldValueExists(Parameter, ParameterValue), #[error("expected old value `{1:?}` but found `{2:?}` for parameter `{0}` in config diff")] - WrongOldValue(Parameter, serde_yaml::Value, serde_yaml::Value), + WrongOldValue(Parameter, ParameterValue, ParameterValue), #[error("expected a value for `{0}` but found none")] MissingParameter(Parameter), - #[error("expected a value of type `{2}` for `{1}` but could not parse it from `{3:?}`")] - WrongValueType(#[source] serde_yaml::Error, Parameter, &'static str, serde_yaml::Value), + #[error("expected a value of type `{1}` for `{0}` but could not parse it from `{2:?}`")] + WrongValueType(Parameter, &'static str, ParameterValue), + #[error("expected an integer of type `{2}` for `{1}` but could not parse it from `{3}`")] + WrongIntegerType(#[source] std::num::TryFromIntError, Parameter, &'static str, u64), + #[error("expected an account id for `{1}` but could not parse it from `{2}`")] + WrongAccountId(#[source] ParseAccountError, Parameter, String), } impl std::str::FromStr for ParameterTable { @@ -48,7 +85,7 @@ impl std::str::FromStr for ParameterTable { return Ok(ParameterTable { parameters: BTreeMap::new() }); } - let yaml_map: BTreeMap = + let yaml_map: BTreeMap = serde_yaml::from_str(arg).map_err(|err| InvalidConfigError::InvalidYaml(err))?; let parameters = yaml_map @@ -57,7 +94,7 @@ impl std::str::FromStr for ParameterTable { let typed_key: Parameter = key .parse() .map_err(|err| InvalidConfigError::UnknownParameter(err, key.to_owned()))?; - Ok((typed_key, parse_parameter_txt_value(value)?)) + Ok((typed_key, parse_parameter_value(value)?)) }) .collect::, _>>()?; @@ -72,38 +109,33 @@ impl TryFrom<&ParameterTable> for RuntimeConfig { Ok(RuntimeConfig { fees: RuntimeFeesConfig { action_fees: enum_map::enum_map! { - action_cost => params.fee(action_cost) + action_cost => params.get_fee(action_cost)? }, - burnt_gas_reward: Rational::new( - params.get_parsed(Parameter::BurntGasRewardNumerator)?, - params.get_parsed(Parameter::BurntGasRewardDenominator)?, - ), - pessimistic_gas_price_inflation_ratio: Rational::new( - params.get_parsed(Parameter::PessimisticGasPriceInflationNumerator)?, - params.get_parsed(Parameter::PessimisticGasPriceInflationDenominator)?, - ), + burnt_gas_reward: params.get_rational(Parameter::BurntGasReward)?, + pessimistic_gas_price_inflation_ratio: params + .get_rational(Parameter::PessimisticGasPriceInflation)?, storage_usage_config: StorageUsageConfig { storage_amount_per_byte: params.get_u128(Parameter::StorageAmountPerByte)?, - num_bytes_account: params.get_parsed(Parameter::StorageNumBytesAccount)?, + num_bytes_account: params.get_number(Parameter::StorageNumBytesAccount)?, num_extra_bytes_record: params - .get_parsed(Parameter::StorageNumExtraBytesRecord)?, + .get_number(Parameter::StorageNumExtraBytesRecord)?, }, }, wasm_config: VMConfig { ext_costs: ExtCostsConfig { costs: enum_map::enum_map! { - cost => params.get_parsed(cost.param())? + cost => params.get_number(cost.param())? }, }, - grow_mem_cost: params.get_parsed(Parameter::WasmGrowMemCost)?, - regular_op_cost: params.get_parsed(Parameter::WasmRegularOpCost)?, + grow_mem_cost: params.get_number(Parameter::WasmGrowMemCost)?, + regular_op_cost: params.get_number(Parameter::WasmRegularOpCost)?, limit_config: serde_yaml::from_value(params.yaml_map(Parameter::vm_limits(), "")) .map_err(InvalidConfigError::InvalidYaml)?, }, account_creation_config: AccountCreationConfig { min_allowed_top_level_account_length: params - .get_parsed(Parameter::MinAllowedTopLevelAccountLength)?, - registrar_account_id: params.get_parsed(Parameter::RegistrarAccountId)?, + .get_number(Parameter::MinAllowedTopLevelAccountLength)?, + registrar_account_id: params.get_account_id(Parameter::RegistrarAccountId)?, }, }) } @@ -115,32 +147,28 @@ impl ParameterTable { diff: ParameterTableDiff, ) -> Result<(), InvalidConfigError> { for (key, (before, after)) in diff.parameters { - if before.is_null() { - match self.parameters.get(&key) { - Some(serde_yaml::Value::Null) | None => { - self.parameters.insert(key, after); - } - Some(old_value) => { - return Err(InvalidConfigError::OldValueExists(key, old_value.clone())) - } + let old_value = self.parameters.get(&key); + if old_value != before.as_ref() { + if old_value.is_none() { + return Err(InvalidConfigError::NoOldValueExists(key, before.unwrap().clone())); } - } else { - match self.parameters.get(&key) { - Some(serde_yaml::Value::Null) | None => { - return Err(InvalidConfigError::NoOldValueExists(key, before.clone())) - } - Some(old_value) => { - if *old_value != before { - return Err(InvalidConfigError::WrongOldValue( - key, - old_value.clone(), - before.clone(), - )); - } else { - self.parameters.insert(key, after); - } - } + if before.is_none() { + return Err(InvalidConfigError::OldValueExists( + key, + old_value.unwrap().clone(), + )); } + return Err(InvalidConfigError::WrongOldValue( + key, + old_value.unwrap().clone(), + before.unwrap().clone(), + )); + } + + if let Some(new_value) = after { + self.parameters.insert(key, new_value); + } else { + self.parameters.remove(&key); } } Ok(()) @@ -156,89 +184,111 @@ impl ParameterTable { let mut key: &'static str = param.into(); key = key.strip_prefix(remove_prefix).unwrap_or(key); if let Some(value) = self.get(*param) { - yaml.insert(key.into(), value.clone()); + yaml.insert( + key.into(), + // All parameter values can be serialized as YAML, so we don't ever expect this + // to fail. + serde_yaml::to_value(value.clone()) + .expect("failed to convert parameter value to YAML"), + ); } } yaml.into() } - fn get(&self, key: Parameter) -> Option<&serde_yaml::Value> { + fn get(&self, key: Parameter) -> Option<&ParameterValue> { self.parameters.get(&key) } /// Access action fee by `ActionCosts`. - fn fee( + fn get_fee( &self, cost: near_primitives_core::config::ActionCosts, - ) -> near_primitives_core::runtime::fees::Fee { - let yaml = self.fee_yaml(FeeParameter::from(cost)); - serde_yaml::from_value::(yaml) - .expect("just constructed a Fee YAML") + ) -> Result { + let key = FeeParameter::from(cost); + Ok(near_primitives_core::runtime::fees::Fee { + send_sir: self.get_number(format!("{key}_send_sir").parse().unwrap())?, + send_not_sir: self.get_number(format!("{key}_send_not_sir").parse().unwrap())?, + execution: self.get_number(format!("{key}_execution").parse().unwrap())?, + }) } - /// Read and parse a parameter from the `ParameterTable`. - fn get_parsed( - &self, - key: Parameter, - ) -> Result { + /// Read and parse a number parameter from the `ParameterTable`. + fn get_number(&self, key: Parameter) -> Result + where + T: TryFrom, + T::Error: Into, + { let value = self.parameters.get(&key).ok_or(InvalidConfigError::MissingParameter(key))?; - serde_yaml::from_value(value.clone()).map_err(|parse_err| { - InvalidConfigError::WrongValueType( - parse_err, + let value_u64 = value.as_u64().ok_or(InvalidConfigError::WrongValueType( + key, + std::any::type_name::(), + value.clone(), + ))?; + T::try_from(value_u64).map_err(|err| { + InvalidConfigError::WrongIntegerType( + err.into(), key, std::any::type_name::(), - value.clone(), + value_u64, ) }) } - /// Read and parse a parameter from the `ParameterTable`. + /// Read and parse a u128 parameter from the `ParameterTable`. fn get_u128(&self, key: Parameter) -> Result { let value = self.parameters.get(&key).ok_or(InvalidConfigError::MissingParameter(key))?; match value { - // Values larger than u64 are stored as quoted strings, so we parse them as YAML - // document to leverage deserialization to u128. - serde_yaml::Value::String(v) => serde_yaml::from_str(v).map_err(|parse_err| { - InvalidConfigError::WrongValueType( - parse_err, - key, - std::any::type_name::(), - value.clone(), - ) - }), - // If the value is a number (or any other type), the usual conversion should work. - _ => serde_yaml::from_value(value.clone()).map_err(|parse_err| { - InvalidConfigError::WrongValueType( - parse_err, - key, - std::any::type_name::(), - value.clone(), - ) - }), + ParameterValue::U64(v) => Ok(u128::from(*v)), + ParameterValue::String(s) => serde_yaml::from_str(s) + .map_err(|err| InvalidConfigError::ValueParseError(err, s.to_owned())), + _ => Err(InvalidConfigError::WrongValueType( + key, + std::any::type_name::(), + value.clone(), + )), } } - fn fee_yaml(&self, key: FeeParameter) -> serde_yaml::Value { - serde_yaml::to_value(BTreeMap::from([ - ("send_sir", self.get(format!("{key}_send_sir").parse().unwrap())), - ("send_not_sir", self.get(format!("{key}_send_not_sir").parse().unwrap())), - ("execution", self.get(format!("{key}_execution").parse().unwrap())), - ])) - .expect("failed to construct fee yaml") + /// Read and parse a string parameter from the `ParameterTable`. + fn get_account_id(&self, key: Parameter) -> Result { + let value = self.parameters.get(&key).ok_or(InvalidConfigError::MissingParameter(key))?; + let value_string = value.as_str().ok_or(InvalidConfigError::WrongValueType( + key, + std::any::type_name::(), + value.clone(), + ))?; + value_string.parse().map_err(|err| { + InvalidConfigError::WrongAccountId( + err, + Parameter::RegistrarAccountId, + value_string.to_string(), + ) + }) + } + + /// Read and parse a rational parameter from the `ParameterTable`. + fn get_rational(&self, key: Parameter) -> Result { + let value = self.parameters.get(&key).ok_or(InvalidConfigError::MissingParameter(key))?; + value.as_rational().ok_or(InvalidConfigError::WrongValueType( + key, + std::any::type_name::(), + value.clone(), + )) } } -/// Represents YAML values supported by parameter diff config. +/// Represents values supported by parameter diff config. #[derive(serde::Deserialize, Clone, Debug)] -struct ParameterDiffValue { - old: Option, - new: Option, +struct ParameterDiffConfigValue { + old: Option, + new: Option, } impl std::str::FromStr for ParameterTableDiff { type Err = InvalidConfigError; fn from_str(arg: &str) -> Result { - let yaml_map: BTreeMap = + let yaml_map: BTreeMap = serde_yaml::from_str(arg).map_err(|err| InvalidConfigError::InvalidYaml(err))?; let parameters = yaml_map @@ -248,17 +298,11 @@ impl std::str::FromStr for ParameterTableDiff { .parse() .map_err(|err| InvalidConfigError::UnknownParameter(err, key.to_owned()))?; - let old_value = if let Some(s) = &value.old { - parse_parameter_txt_value(s)? - } else { - serde_yaml::Value::Null - }; + let old_value = + if let Some(s) = &value.old { Some(parse_parameter_value(s)?) } else { None }; - let new_value = if let Some(s) = &value.new { - parse_parameter_txt_value(s)? - } else { - serde_yaml::Value::Null - }; + let new_value = + if let Some(s) = &value.new { Some(parse_parameter_value(s)?) } else { None }; Ok((typed_key, (old_value, new_value))) }) @@ -267,11 +311,38 @@ impl std::str::FromStr for ParameterTableDiff { } } +/// Parses a value from YAML to a more restricted type of parameter values. +fn parse_parameter_value(value: &serde_yaml::Value) -> Result { + Ok(serde_yaml::from_value(canonicalize_yaml_value(value)?) + .map_err(|err| InvalidConfigError::InvalidYaml(err))?) +} + +/// Recursively canonicalizes values inside of the YAML structure. +fn canonicalize_yaml_value( + value: &serde_yaml::Value, +) -> Result { + Ok(match value { + serde_yaml::Value::String(s) => canonicalize_yaml_string(s)?, + serde_yaml::Value::Mapping(m) => serde_yaml::Value::Mapping( + m.iter() + .map(|(key, value)| { + let canonical_value = canonicalize_yaml_value(value)?; + Ok((key.clone(), canonical_value)) + }) + .collect::>()?, + ), + _ => value.clone(), + }) +} + /// Parses a value from the custom format for runtime parameter definitions. /// -/// A value can be a positive integer or a string, both written without quotes. +/// A value can be a positive integer or a string, with or without quotes. /// Integers can use underlines as separators (for readability). -fn parse_parameter_txt_value(value: &str) -> Result { +/// +/// The main purpose of this function is to add support for integers with underscore digit +/// separators which we use in the config but are not supported in YAML. +fn canonicalize_yaml_string(value: &str) -> Result { if value.is_empty() { return Ok(serde_yaml::Value::Null); } @@ -294,7 +365,10 @@ fn parse_parameter_txt_value(value: &str) -> Result { - assert_eq!(expected, serde_yaml::to_value(3200000000i64).unwrap()); - assert_eq!(found, serde_yaml::to_value(3200000).unwrap()); + assert_eq!(expected, ParameterValue::U64(3200000000)); + assert_eq!(found, ParameterValue::U64(3200000)); } ); } @@ -554,7 +644,7 @@ max_memory_pages: { new: 512 } &["min_allowed_top_level_account_length: { new: 1_600_000 }"] ), InvalidConfigError::OldValueExists(Parameter::MinAllowedTopLevelAccountLength, expected) => { - assert_eq!(expected, serde_yaml::to_value(3200000000i64).unwrap()); + assert_eq!(expected, ParameterValue::U64(3200000000)); } ); } @@ -567,8 +657,36 @@ max_memory_pages: { new: 512 } &["wasm_regular_op_cost: { old: 3_200_000, new: 1_600_000 }"] ), InvalidConfigError::NoOldValueExists(Parameter::WasmRegularOpCost, found) => { - assert_eq!(found, serde_yaml::to_value(3200000).unwrap()); + assert_eq!(found, ParameterValue::U64(3200000)); } ); } + + #[test] + fn test_parameter_table_yaml_map() { + let params: ParameterTable = BASE_0.parse().unwrap(); + let yaml = params.yaml_map( + [ + Parameter::RegistrarAccountId, + Parameter::MinAllowedTopLevelAccountLength, + Parameter::StorageAmountPerByte, + Parameter::StorageNumBytesAccount, + Parameter::StorageNumExtraBytesRecord, + Parameter::BurntGasReward, + ] + .iter(), + "", + ); + assert_eq!( + yaml, + serde_yaml::to_value( + params + .parameters + .iter() + .map(|(key, value)| (key.to_string(), value)) + .collect::>() + ) + .unwrap() + ); + } }