Skip to content

Commit

Permalink
Fix #9393 (#9395)
Browse files Browse the repository at this point in the history
We made a single letter mistake and passed values larger than `i32::MAX` to a function that did only supports `0..=i32::MAX`. This fix corrects the problem, introduces the necessary protocol versioning, a regression test and an additional assert to validate the inputs in the aforementioned function.

I ended up using the parameters for the behaviour flag in anticipation that I wouldn’t need to do this later as part of #9364.
  • Loading branch information
nagisa authored and nikurt committed Aug 24, 2023
1 parent 4a44765 commit 5bed6b2
Show file tree
Hide file tree
Showing 43 changed files with 587 additions and 13 deletions.
2 changes: 1 addition & 1 deletion chain/jsonrpc/jsonrpc-tests/res/genesis_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,4 @@
],
"use_production_config": false,
"records": []
}
}
5 changes: 5 additions & 0 deletions core/primitives-core/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ pub struct VMConfig {
/// Gas cost of a regular operation.
pub regular_op_cost: u32,

/// Disable the fix for the #9393 issue in near-vm-runner.
pub disable_9393_fix: bool,

/// Describes limits for VM and Runtime.
pub limit_config: VMLimitConfig,
}
Expand Down Expand Up @@ -179,6 +182,7 @@ impl VMConfig {
ext_costs: ExtCostsConfig::test(),
grow_mem_cost: 1,
regular_op_cost: (SAFETY_MULTIPLIER as u32) * 1285457,
disable_9393_fix: false,
limit_config: VMLimitConfig::test(),
}
}
Expand All @@ -196,6 +200,7 @@ impl VMConfig {
ext_costs: ExtCostsConfig::free(),
grow_mem_cost: 0,
regular_op_cost: 0,
disable_9393_fix: false,
// We shouldn't have any costs in the limit config.
limit_config: VMLimitConfig { max_gas_burnt: u64::MAX, ..VMLimitConfig::test() },
}
Expand Down
3 changes: 3 additions & 0 deletions core/primitives-core/src/parameter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ pub enum Parameter {
Wasmer2StackLimit,
MaxLocalsPerContract,
AccountIdValidityRulesVersion,

#[strum(serialize = "disable_9393_fix")]
Disable9393Fix,
}

#[derive(
Expand Down
3 changes: 3 additions & 0 deletions core/primitives/res/runtime_configs/62.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@
# correct.
max_stack_height: { old: 16384, new: 262144 }
contract_prepare_version: { old: 1, new: 2 }

# There was a bug for a short period of time that we need to reproduce...
disable_9393_fix: { old: false, new: true }
1 change: 1 addition & 0 deletions core/primitives/res/runtime_configs/63.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
disable_9393_fix: { old: true, new: false }
1 change: 1 addition & 0 deletions core/primitives/res/runtime_configs/parameters.snap
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,5 @@ max_functions_number_per_contract 10_000
wasmer2_stack_limit 204_800
max_locals_per_contract 1_000_000
account_id_validity_rules_version 1
disable_9393_fix false

2 changes: 2 additions & 0 deletions core/primitives/res/runtime_configs/parameters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,5 @@ max_length_storage_value: 4_194_304
max_promises_per_function_call_action: 1_024
max_number_input_data_dependencies: 128
account_id_validity_rules_version: 0

disable_9393_fix: false
2 changes: 2 additions & 0 deletions core/primitives/res/runtime_configs/parameters_testnet.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,5 @@ max_length_storage_key: 4_194_304
max_length_storage_value: 4_194_304
max_promises_per_function_call_action: 1_024
max_number_input_data_dependencies: 128

disable_9393_fix: false
1 change: 1 addition & 0 deletions core/primitives/src/runtime/config_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ static CONFIG_DIFFS: &[(ProtocolVersion, &str)] = &[
(59, include_config!("59.yaml")),
(61, include_config!("61.yaml")),
(62, include_config!("62.yaml")),
(63, include_config!("63.yaml")),
];

/// Testnet parameters for versions <= 29, which (incorrectly) differed from mainnet parameters
Expand Down
19 changes: 19 additions & 0 deletions core/primitives/src/runtime/parameter_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub(crate) enum ParameterValue {
// for u128, but this is currently impossible to express in YAML (see
// `canonicalize_yaml_string`).
String(String),
Flag(bool),
}

#[derive(thiserror::Error, Debug)]
Expand Down Expand Up @@ -74,6 +75,22 @@ impl TryFrom<&ParameterValue> for u128 {
}
}

impl TryFrom<&ParameterValue> for bool {
type Error = ValueConversionError;

fn try_from(value: &ParameterValue) -> Result<Self, Self::Error> {
match value {
ParameterValue::Flag(b) => Ok(*b),
ParameterValue::String(s) => match &**s {
"true" => Ok(true),
"false" => Ok(false),
_ => Err(ValueConversionError::ParseType("bool", value.clone())),
},
_ => Err(ValueConversionError::ParseType("bool", value.clone())),
}
}
}

impl TryFrom<&ParameterValue> for Rational32 {
type Error = ValueConversionError;

Expand Down Expand Up @@ -177,6 +194,7 @@ impl core::fmt::Display for ParameterValue {
)
}
ParameterValue::String(v) => write!(f, "{v}"),
ParameterValue::Flag(b) => write!(f, "{b:?}"),
}
}
}
Expand Down Expand Up @@ -270,6 +288,7 @@ impl TryFrom<&ParameterTable> for RuntimeConfig {
},
grow_mem_cost: params.get(Parameter::WasmGrowMemCost)?,
regular_op_cost: params.get(Parameter::WasmRegularOpCost)?,
disable_9393_fix: params.get(Parameter::Disable9393Fix)?,
limit_config: serde_yaml::from_value(params.yaml_map(Parameter::vm_limits()))
.map_err(InvalidConfigError::InvalidYaml)?,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ expression: config_view
},
"grow_mem_cost": 1,
"regular_op_cost": 3856371,
"disable_9393_fix": false,
"limit_config": {
"max_gas_burnt": 200000000000000,
"max_stack_height": 16384,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ expression: config_view
},
"grow_mem_cost": 1,
"regular_op_cost": 3856371,
"disable_9393_fix": false,
"limit_config": {
"max_gas_burnt": 200000000000000,
"max_stack_height": 16384,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ expression: config_view
},
"grow_mem_cost": 1,
"regular_op_cost": 2207874,
"disable_9393_fix": false,
"limit_config": {
"max_gas_burnt": 200000000000000,
"max_stack_height": 16384,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ expression: config_view
},
"grow_mem_cost": 1,
"regular_op_cost": 822756,
"disable_9393_fix": false,
"limit_config": {
"max_gas_burnt": 200000000000000,
"max_stack_height": 16384,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ expression: config_view
},
"grow_mem_cost": 1,
"regular_op_cost": 822756,
"disable_9393_fix": false,
"limit_config": {
"max_gas_burnt": 200000000000000,
"max_stack_height": 16384,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ expression: config_view
},
"grow_mem_cost": 1,
"regular_op_cost": 822756,
"disable_9393_fix": false,
"limit_config": {
"max_gas_burnt": 300000000000000,
"max_stack_height": 16384,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ expression: config_view
},
"grow_mem_cost": 1,
"regular_op_cost": 822756,
"disable_9393_fix": false,
"limit_config": {
"max_gas_burnt": 300000000000000,
"max_stack_height": 16384,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ expression: config_view
},
"grow_mem_cost": 1,
"regular_op_cost": 822756,
"disable_9393_fix": false,
"limit_config": {
"max_gas_burnt": 300000000000000,
"max_stack_height": 16384,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ expression: config_view
},
"grow_mem_cost": 1,
"regular_op_cost": 822756,
"disable_9393_fix": false,
"limit_config": {
"max_gas_burnt": 300000000000000,
"max_stack_height": 16384,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ expression: config_view
},
"grow_mem_cost": 1,
"regular_op_cost": 822756,
"disable_9393_fix": false,
"limit_config": {
"max_gas_burnt": 300000000000000,
"max_stack_height": 16384,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ expression: config_view
},
"grow_mem_cost": 1,
"regular_op_cost": 822756,
"disable_9393_fix": true,
"limit_config": {
"max_gas_burnt": 300000000000000,
"max_stack_height": 262144,
Expand Down
Loading

0 comments on commit 5bed6b2

Please sign in to comment.