From 58bd1489fc7ab6ce3fe73787cee77c8b35b41bf2 Mon Sep 17 00:00:00 2001 From: supra-yoga Date: Wed, 24 Sep 2025 12:41:34 +0530 Subject: [PATCH 1/6] Initial changes for scaling Gas Schedule --- .../aptos-gas-schedule-updator/src/lib.rs | 79 +++++++++++++++--- .../aptos-gas-schedule-updator/src/main.rs | 4 +- .../tests/gen_tests.rs | 4 +- types/src/on_chain_config/gas_schedule.rs | 81 +++++++++++++++++++ 4 files changed, 154 insertions(+), 14 deletions(-) diff --git a/aptos-move/aptos-gas-schedule-updator/src/lib.rs b/aptos-move/aptos-gas-schedule-updator/src/lib.rs index ea4184315f926..1e5b92956a670 100644 --- a/aptos-move/aptos-gas-schedule-updator/src/lib.rs +++ b/aptos-move/aptos-gas-schedule-updator/src/lib.rs @@ -7,6 +7,7 @@ //! The generated proposal includes a comment section, listing the contents of the //! gas schedule in a human readable format. +use std::fs; use anyhow::Result; use aptos_gas_schedule::{ AptosGasParameters, InitialGasSchedule, ToOnChainGasSchedule, LATEST_GAS_FEATURE_VERSION, @@ -18,6 +19,8 @@ use move_core_types::account_address::AccountAddress; use move_model::{code_writer::CodeWriter, emit, emitln, model::Loc}; use std::path::{Path, PathBuf}; +const DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH: String = String::from("./proposals"); + fn generate_blob(writer: &CodeWriter, data: &[u8]) { emitln!(writer, "vector["); writer.indent(); @@ -103,16 +106,61 @@ fn aptos_framework_path() -> PathBuf { ) } +#[derive(Parser, Debug)] +pub enum GasScheduleGenerator { + #[clap(short, long)] + GenerateNew(GenerateNewSchedule), + #[clap(short, long)] + ScaleCurrent(GenerateNewSchedule), +} + /// Command line arguments to the gas schedule update proposal generation tool. #[derive(Debug, Parser)] -pub struct GenArgs { - #[clap(short, long)] +pub struct GenerateNewSchedule { + #[clap(short, long, help = "Path to file to write the output script")] pub output: Option, #[clap(short, long)] pub gas_feature_version: Option, } +impl GenerateNewSchedule { + pub fn execute(self) -> Result<()> { + let feature_version = self + .gas_feature_version + .unwrap_or(LATEST_GAS_FEATURE_VERSION); + + let gas_schedule = current_gas_schedule(feature_version); + + generate_update_proposal(&gas_schedule, self.output.unwrap_or(DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH)) + } +} + + +#[derive(Debug, Parser)] +pub struct ScaleCurrentSchedule { + #[clap(short, long, help = "Path to file to write the output script")] + pub output: Option, + + #[clap(short, long, help = "Path to JSON file containing the GasScheduleV2 to use")] + pub current_schedule: String, + + #[clap(short, long, help = "Scale the Minimum Gas Price value with the given factor")] + pub scale_min_gas_price_by: f64, +} + + +impl ScaleCurrentSchedule { + pub fn execute(self) -> Result<()> { + let json_str = fs::read_to_string(self.current_schedule)?; + let mut current_schedule = GasScheduleV2::from_json_string(json_str); + + current_schedule.scale_min_gas_price_by(self.scale_min_gas_price_by); + + generate_update_proposal(¤t_schedule, self.output.unwrap_or(DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH)) + } +} + /// Constructs the current gas schedule in on-chain format. pub fn current_gas_schedule(feature_version: u64) -> GasScheduleV2 { GasScheduleV2 { @@ -122,27 +170,38 @@ pub fn current_gas_schedule(feature_version: u64) -> GasScheduleV2 { } /// Entrypoint for the update proposal generation tool. -pub fn generate_update_proposal(args: &GenArgs) -> Result<()> { +pub fn generate_update_proposal(gas_schedule: &GasScheduleV2, output_path: String) -> Result<()> { let mut pack = PackageBuilder::new("GasScheduleUpdate"); - let feature_version = args - .gas_feature_version - .unwrap_or(LATEST_GAS_FEATURE_VERSION); - pack.add_source( "update_gas_schedule.move", - &generate_script(¤t_gas_schedule(feature_version))?, + &generate_script(gas_schedule)?, ); // TODO: use relative path here pack.add_local_dep("SupraFramework", &aptos_framework_path().to_string_lossy()); - pack.write_to_disk(args.output.as_deref().unwrap_or("./proposal"))?; + pack.write_to_disk(PathBuf::from(output_path))?; Ok(()) } + +impl GasScheduleGenerator { + pub fn execute(self) -> Result<()> { + match self { + GasScheduleGenerator::GenerateNew(args) => { + args.execute() + } + GasScheduleGenerator::ScaleCurrent(args) => { + args.execute() + } + } + } +} + + #[test] fn verify_tool() { use clap::CommandFactory; - GenArgs::command().debug_assert() + GenerateNewSchedule::command().debug_assert() } diff --git a/aptos-move/aptos-gas-schedule-updator/src/main.rs b/aptos-move/aptos-gas-schedule-updator/src/main.rs index ad5951070238e..b6a1d6f0e0879 100644 --- a/aptos-move/aptos-gas-schedule-updator/src/main.rs +++ b/aptos-move/aptos-gas-schedule-updator/src/main.rs @@ -2,11 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 use anyhow::Result; -use aptos_gas_schedule_updator::{generate_update_proposal, GenArgs}; +use aptos_gas_schedule_updator::{generate_update_proposal, GenerateNewSchedule}; use clap::Parser; fn main() -> Result<()> { - let args = GenArgs::parse(); + let args = GenerateNewSchedule::parse(); generate_update_proposal(&args) } diff --git a/aptos-move/aptos-gas-schedule-updator/tests/gen_tests.rs b/aptos-move/aptos-gas-schedule-updator/tests/gen_tests.rs index d53b5538a380a..26b7815cf4e6d 100644 --- a/aptos-move/aptos-gas-schedule-updator/tests/gen_tests.rs +++ b/aptos-move/aptos-gas-schedule-updator/tests/gen_tests.rs @@ -2,13 +2,13 @@ // SPDX-License-Identifier: Apache-2.0 use aptos_framework::{BuildOptions, BuiltPackage}; -use aptos_gas_schedule_updator::{generate_update_proposal, GenArgs}; +use aptos_gas_schedule_updator::{generate_update_proposal, GenerateNewSchedule}; #[test] fn can_generate_and_build_update_proposal() { let output_dir = tempfile::tempdir().unwrap(); - generate_update_proposal(&GenArgs { + generate_update_proposal(&GenerateNewSchedule { gas_feature_version: None, output: Some(output_dir.path().to_string_lossy().to_string()), }) diff --git a/types/src/on_chain_config/gas_schedule.rs b/types/src/on_chain_config/gas_schedule.rs index 8f2811f4c51e8..47e29eae78fc2 100644 --- a/types/src/on_chain_config/gas_schedule.rs +++ b/types/src/on_chain_config/gas_schedule.rs @@ -5,6 +5,8 @@ use crate::on_chain_config::OnChainConfig; use serde::{Deserialize, Serialize}; use std::collections::{btree_map, BTreeMap}; +const KEY_MIN_PRICE_PER_GAS: &str = "txn.min_price_per_gas_unit"; + #[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] pub struct GasSchedule { pub entries: Vec<(String, u64)>, @@ -54,6 +56,22 @@ impl GasSchedule { } impl GasScheduleV2 { + + pub fn from_json_string(json_str: String) -> Self { + serde_json::from_str(&json_str).unwrap() + } + + pub fn scale_min_gas_price_by(&mut self, factor: f64) { + for (name, value) in &mut self.entries { + if name == KEY_MIN_PRICE_PER_GAS { + // Convert to f64, multiply, then convert back to u64 with saturation + let new_value = (*value as f64 * factor).round() as u64; + *value = new_value; + } + } + } + + pub fn into_btree_map(self) -> BTreeMap { // TODO: what if the gas schedule contains duplicated entries? self.entries.into_iter().collect() @@ -106,3 +124,66 @@ impl OnChainConfig for StorageGasSchedule { const MODULE_IDENTIFIER: &'static str = "storage_gas"; const TYPE_IDENTIFIER: &'static str = "StorageGas"; } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_scale_min_gas_price_by() { + let mut gas_schedule = GasScheduleV2 { + feature_version: 1, + entries: vec![ + ("other.param".to_string(), 100), + (KEY_MIN_PRICE_PER_GAS.to_string(), 50), + ("another.param".to_string(), 200), + ], + }; + + gas_schedule.scale_min_gas_price_by(0.5); + + // Check that only the min gas price was scaled + assert_eq!(gas_schedule.entries[0], ("other.param".to_string(), 100)); + assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 25)); + assert_eq!(gas_schedule.entries[2], ("another.param".to_string(), 200)); + + gas_schedule.scale_min_gas_price_by(2.0); + assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 50)); + + gas_schedule.scale_min_gas_price_by(10.0); + assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 500)); + + gas_schedule.scale_min_gas_price_by(0.1); + assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 500)); + } + + #[test] + fn test_scale_min_gas_price_by_rounding() { + let mut gas_schedule = GasScheduleV2 { + feature_version: 1, + entries: vec![ + (KEY_MIN_PRICE_PER_GAS.to_string(), 100), + ], + }; + + // Test rounding behavior (100 * 1.234 = 123.4, should round to 123) + gas_schedule.scale_min_gas_price_by(1.234); + assert_eq!(gas_schedule.entries[0].1, 123); + } + + #[test] + fn test_scale_min_gas_price_by_overflow_protection() { + let mut gas_schedule = GasScheduleV2 { + feature_version: 1, + entries: vec![ + (KEY_MIN_PRICE_PER_GAS.to_string(), u64::MAX), + ], + }; + + // Should not panic with large factor + gas_schedule.scale_min_gas_price_by(2.0); + // The result will be clamped to u64::MAX due to overflow in f64 to u64 conversion + println!("{:?}", gas_schedule); + assert!(gas_schedule.entries[0].1 > 0); + } +} From 0233688a432c01fec3d3c96de39ec90ce0230fb5 Mon Sep 17 00:00:00 2001 From: supra-yoga Date: Wed, 24 Sep 2025 17:29:41 +0530 Subject: [PATCH 2/6] Implement custom deserialization and add tests --- .../aptos-gas-schedule-updator/src/lib.rs | 27 +++---- .../aptos-gas-schedule-updator/src/main.rs | 6 +- .../tests/gen_tests.rs | 7 +- types/src/on_chain_config/gas_schedule.rs | 76 ++++++++++++++++++- 4 files changed, 89 insertions(+), 27 deletions(-) diff --git a/aptos-move/aptos-gas-schedule-updator/src/lib.rs b/aptos-move/aptos-gas-schedule-updator/src/lib.rs index 1e5b92956a670..1e0ec0f93d702 100644 --- a/aptos-move/aptos-gas-schedule-updator/src/lib.rs +++ b/aptos-move/aptos-gas-schedule-updator/src/lib.rs @@ -14,12 +14,12 @@ use aptos_gas_schedule::{ }; use aptos_package_builder::PackageBuilder; use aptos_types::on_chain_config::GasScheduleV2; -use clap::Parser; +use clap::{Args, Parser}; use move_core_types::account_address::AccountAddress; use move_model::{code_writer::CodeWriter, emit, emitln, model::Loc}; use std::path::{Path, PathBuf}; -const DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH: String = String::from("./proposals"); +const DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH: &str = "./proposals"; fn generate_blob(writer: &CodeWriter, data: &[u8]) { emitln!(writer, "vector["); @@ -108,19 +108,17 @@ fn aptos_framework_path() -> PathBuf { #[derive(Parser, Debug)] pub enum GasScheduleGenerator { - #[clap(short, long)] GenerateNew(GenerateNewSchedule), - #[clap(short, long)] - ScaleCurrent(GenerateNewSchedule), + ScaleCurrent(ScaleCurrentSchedule), } /// Command line arguments to the gas schedule update proposal generation tool. -#[derive(Debug, Parser)] +#[derive(Debug, Args)] pub struct GenerateNewSchedule { #[clap(short, long, help = "Path to file to write the output script")] pub output: Option, - #[clap(short, long)] + #[clap(short, long, help="Feature version of the GasSchedule generated")] pub gas_feature_version: Option, } @@ -132,12 +130,13 @@ impl GenerateNewSchedule { let gas_schedule = current_gas_schedule(feature_version); - generate_update_proposal(&gas_schedule, self.output.unwrap_or(DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH)) + generate_update_proposal(&gas_schedule, self.output + .unwrap_or_else(|| DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH.to_string())) } } -#[derive(Debug, Parser)] +#[derive(Debug, Args)] pub struct ScaleCurrentSchedule { #[clap(short, long, help = "Path to file to write the output script")] pub output: Option, @@ -157,7 +156,8 @@ impl ScaleCurrentSchedule { current_schedule.scale_min_gas_price_by(self.scale_min_gas_price_by); - generate_update_proposal(¤t_schedule, self.output.unwrap_or(DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH)) + generate_update_proposal(¤t_schedule, self.output + .unwrap_or_else(|| DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH.to_string())) } } @@ -198,10 +198,3 @@ impl GasScheduleGenerator { } } } - - -#[test] -fn verify_tool() { - use clap::CommandFactory; - GenerateNewSchedule::command().debug_assert() -} diff --git a/aptos-move/aptos-gas-schedule-updator/src/main.rs b/aptos-move/aptos-gas-schedule-updator/src/main.rs index b6a1d6f0e0879..5740fe71f2e2e 100644 --- a/aptos-move/aptos-gas-schedule-updator/src/main.rs +++ b/aptos-move/aptos-gas-schedule-updator/src/main.rs @@ -2,11 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 use anyhow::Result; -use aptos_gas_schedule_updator::{generate_update_proposal, GenerateNewSchedule}; +use aptos_gas_schedule_updator::GasScheduleGenerator; use clap::Parser; fn main() -> Result<()> { - let args = GenerateNewSchedule::parse(); - - generate_update_proposal(&args) + GasScheduleGenerator::parse().execute() } diff --git a/aptos-move/aptos-gas-schedule-updator/tests/gen_tests.rs b/aptos-move/aptos-gas-schedule-updator/tests/gen_tests.rs index 26b7815cf4e6d..9b880bf610c60 100644 --- a/aptos-move/aptos-gas-schedule-updator/tests/gen_tests.rs +++ b/aptos-move/aptos-gas-schedule-updator/tests/gen_tests.rs @@ -2,16 +2,17 @@ // SPDX-License-Identifier: Apache-2.0 use aptos_framework::{BuildOptions, BuiltPackage}; -use aptos_gas_schedule_updator::{generate_update_proposal, GenerateNewSchedule}; +use aptos_gas_schedule_updator::GenerateNewSchedule; #[test] fn can_generate_and_build_update_proposal() { let output_dir = tempfile::tempdir().unwrap(); - generate_update_proposal(&GenerateNewSchedule { + GenerateNewSchedule { gas_feature_version: None, output: Some(output_dir.path().to_string_lossy().to_string()), - }) + } + .execute() .unwrap(); BuiltPackage::build(output_dir.path().to_path_buf(), BuildOptions::default()).unwrap(); diff --git a/types/src/on_chain_config/gas_schedule.rs b/types/src/on_chain_config/gas_schedule.rs index 47e29eae78fc2..24632cc273c0c 100644 --- a/types/src/on_chain_config/gas_schedule.rs +++ b/types/src/on_chain_config/gas_schedule.rs @@ -2,6 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 use crate::on_chain_config::OnChainConfig; +use serde::de::{self, Deserializer}; +use serde::ser::{SerializeSeq, Serializer}; use serde::{Deserialize, Serialize}; use std::collections::{btree_map, BTreeMap}; @@ -15,6 +17,9 @@ pub struct GasSchedule { #[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] pub struct GasScheduleV2 { pub feature_version: u64, + #[serde( + deserialize_with = "deserialize_gas_schedule_entries", + )] pub entries: Vec<(String, u64)>, } @@ -110,6 +115,47 @@ impl GasScheduleV2 { } } +#[derive(Deserialize)] +#[serde(untagged)] +enum GasEntryHelper { + Tuple((String, u64)), + Map { key: String, val: GasEntryValue }, +} + +#[derive(Deserialize)] +#[serde(untagged)] +enum GasEntryValue { + Number(u64), + String(String), +} + +fn deserialize_gas_schedule_entries<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let raw_entries: Vec = Vec::deserialize(deserializer)?; + raw_entries + .into_iter() + .map(|entry| match entry { + GasEntryHelper::Tuple(pair) => Ok(pair), + GasEntryHelper::Map { key, val } => { + let parsed_val = match val { + GasEntryValue::Number(n) => n, + GasEntryValue::String(s) => s.parse::().map_err(|e| { + de::Error::custom(format!( + "failed to parse gas entry value for {}: {}", + key, e + )) + })?, + }; + Ok((key, parsed_val)) + }, + }) + .collect() +} + impl OnChainConfig for GasSchedule { const MODULE_IDENTIFIER: &'static str = "gas_schedule"; const TYPE_IDENTIFIER: &'static str = "GasSchedule"; @@ -154,7 +200,7 @@ mod tests { assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 500)); gas_schedule.scale_min_gas_price_by(0.1); - assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 500)); + assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 50)); } #[test] @@ -183,7 +229,31 @@ mod tests { // Should not panic with large factor gas_schedule.scale_min_gas_price_by(2.0); // The result will be clamped to u64::MAX due to overflow in f64 to u64 conversion - println!("{:?}", gas_schedule); - assert!(gas_schedule.entries[0].1 > 0); + assert_eq!(gas_schedule.entries[0].1, u64::MAX); + } + + #[test] + fn test_deserialize_map_entries_with_string_values() { + let json = r#"{ + "feature_version": 42, + "entries": [ + { "key": "foo", "val": "123" }, + { "key": "bar", "val": 456 }, + { "key": "txn.min_price_per_gas_unit", "val": 50 } + ] + }"# + .to_string(); + + let mut schedule = GasScheduleV2::from_json_string(json); + + assert_eq!(schedule.feature_version, 42); + assert_eq!(schedule.entries.len(), 3); + assert_eq!(schedule.entries[0], ("foo".to_string(), 123)); + assert_eq!(schedule.entries[1], ("bar".to_string(), 456)); + assert_eq!(schedule.entries[2], ("txn.min_price_per_gas_unit".to_string(), 50)); + + schedule.scale_min_gas_price_by(10.0); + + assert_eq!(schedule.entries[2], ("txn.min_price_per_gas_unit".to_string(), 500)); } } From 96fc0c5f89d82908ffcd324b2d6f5841ebf69e89 Mon Sep 17 00:00:00 2001 From: supra-yoga Date: Thu, 9 Oct 2025 13:01:54 +0530 Subject: [PATCH 3/6] Change GasSchedule Updator CLI --- .../aptos-gas-schedule-updator/Cargo.toml | 2 + .../src/change_set.rs | 50 +++++++++++++ .../aptos-gas-schedule-updator/src/lib.rs | 72 ++++++++++++++++--- aptos-move/aptos-gas-schedule/src/ver.rs | 3 +- types/src/on_chain_config/gas_schedule.rs | 22 +++--- 5 files changed, 129 insertions(+), 20 deletions(-) create mode 100644 aptos-move/aptos-gas-schedule-updator/src/change_set.rs diff --git a/aptos-move/aptos-gas-schedule-updator/Cargo.toml b/aptos-move/aptos-gas-schedule-updator/Cargo.toml index 1b256ea430900..fc99efd9e41db 100644 --- a/aptos-move/aptos-gas-schedule-updator/Cargo.toml +++ b/aptos-move/aptos-gas-schedule-updator/Cargo.toml @@ -21,6 +21,8 @@ move-model = { workspace = true } anyhow = { workspace = true } bcs = { workspace = true } +serde = { workspace = true } +serde_json = { workspace = true } clap = { workspace = true } [dev-dependencies] diff --git a/aptos-move/aptos-gas-schedule-updator/src/change_set.rs b/aptos-move/aptos-gas-schedule-updator/src/change_set.rs new file mode 100644 index 0000000000000..74259e5baa31e --- /dev/null +++ b/aptos-move/aptos-gas-schedule-updator/src/change_set.rs @@ -0,0 +1,50 @@ +use std::collections::HashMap; +use serde::{Deserialize, Serialize}; + +pub type Entries = HashMap; + +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] +pub struct GasScheduleChangeSet { + additions: Entries, + deletions: Entries +} + +impl GasScheduleChangeSet { + pub fn from_json_string(json_str: String) -> anyhow::Result { + serde_json::from_str(&json_str).map_err(|e| anyhow::anyhow!(e)) + } + + pub fn deletion_entries(&self) -> Entries { + self.deletions.clone() + } + + pub fn addition_entries(&self) -> Entries { + self.additions.clone() + } + + pub fn is_empty(&self) -> bool { + self.additions.is_empty() && self.deletions.is_empty() + } +} + +#[test] +fn test_deserialize_change_set() { + let json = r#"{ + "additions": { + "foo": 123, + "bar": 600999 + }, + "deletions": { + "bar": 456 + } + }"# + .to_string(); + + let mut change_set = GasScheduleChangeSet::from_json_string(json).unwrap(); + + assert_eq!(change_set.additions.len(), 2); + assert_eq!(change_set.deletions.len(), 1); + assert_eq!(change_set.additions.get("foo").unwrap(), &123u64); + assert_eq!(change_set.additions.get("bar").unwrap(), &600999u64); + assert_eq!(change_set.deletions.get("bar").unwrap(), &456u64); +} \ No newline at end of file diff --git a/aptos-move/aptos-gas-schedule-updator/src/lib.rs b/aptos-move/aptos-gas-schedule-updator/src/lib.rs index 1e0ec0f93d702..587fb05b29df7 100644 --- a/aptos-move/aptos-gas-schedule-updator/src/lib.rs +++ b/aptos-move/aptos-gas-schedule-updator/src/lib.rs @@ -7,8 +7,10 @@ //! The generated proposal includes a comment section, listing the contents of the //! gas schedule in a human readable format. +mod change_set; + use std::fs; -use anyhow::Result; +use anyhow::{anyhow, Result}; use aptos_gas_schedule::{ AptosGasParameters, InitialGasSchedule, ToOnChainGasSchedule, LATEST_GAS_FEATURE_VERSION, }; @@ -18,6 +20,7 @@ use clap::{Args, Parser}; use move_core_types::account_address::AccountAddress; use move_model::{code_writer::CodeWriter, emit, emitln, model::Loc}; use std::path::{Path, PathBuf}; +use crate::change_set::GasScheduleChangeSet; const DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH: &str = "./proposals"; @@ -40,7 +43,7 @@ fn generate_blob(writer: &CodeWriter, data: &[u8]) { } fn generate_script(gas_schedule: &GasScheduleV2) -> Result { - let gas_schedule_blob = bcs::to_bytes(gas_schedule).unwrap(); + let gas_schedule_blob = bcs::to_bytes(gas_schedule)?; assert!(gas_schedule_blob.len() < 65536); @@ -110,6 +113,7 @@ fn aptos_framework_path() -> PathBuf { pub enum GasScheduleGenerator { GenerateNew(GenerateNewSchedule), ScaleCurrent(ScaleCurrentSchedule), + UpdateSchedule(UpdateSchedule) } /// Command line arguments to the gas schedule update proposal generation tool. @@ -142,19 +146,68 @@ pub struct ScaleCurrentSchedule { pub output: Option, #[clap(short, long, help = "Path to JSON file containing the GasScheduleV2 to use")] - pub current_schedule: String, + pub current_schedule_path: String, - #[clap(short, long, help = "Scale the Minimum Gas Price value with the given factor")] - pub scale_min_gas_price_by: f64, + #[clap(short, long, help = "Scale the Minimum Gas Unit Price value by the given factor")] + pub scale_min_gas_unit_price_by: f64, } impl ScaleCurrentSchedule { pub fn execute(self) -> Result<()> { - let json_str = fs::read_to_string(self.current_schedule)?; - let mut current_schedule = GasScheduleV2::from_json_string(json_str); + let json_str = fs::read_to_string(self.current_schedule_path)?; + let mut current_schedule = GasScheduleV2::from_json_string(json_str)?; - current_schedule.scale_min_gas_price_by(self.scale_min_gas_price_by); + current_schedule.scale_min_gas_unit_price_by(self.scale_min_gas_unit_price_by); + + generate_update_proposal(¤t_schedule, self.output + .unwrap_or_else(|| DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH.to_string())) + } +} + +#[derive(Parser, Debug)] +pub struct UpdateSchedule { + #[clap(short, long, help = "Path to file to write the output script")] + pub output: Option, + + #[clap(short, long, help = "Path to JSON file containing the GasScheduleV2 to update")] + pub current_schedule_path: String, + + #[clap(short, long, help = "Path to JSON file containing change set to the GasScheduleV2")] + pub change_set_path: String, +} + +impl UpdateSchedule { + pub fn execute(self) -> Result<()> { + let change_set_json_str = fs::read_to_string(self.change_set_path)?; + let change_set = GasScheduleChangeSet::from_json_string(change_set_json_str)?; + + if change_set.is_empty() { + return Err(anyhow::anyhow!("The change set is empty")); + } + + let current_schedule_json_str = fs::read_to_string(self.current_schedule_path)?; + let mut current_schedule = GasScheduleV2::from_json_string(current_schedule_json_str)?; + + for addition_entry in change_set.addition_entries() { + if !current_schedule.entries.contains(&addition_entry) { + current_schedule.entries.push(addition_entry); + } else { + return Err(anyhow!("Addition entry {:?} is already found in GasSchedule", addition_entry)); + } + } + + for deletion_entry in change_set.deletion_entries() { + if current_schedule.entries.contains(&deletion_entry) { + current_schedule.entries.retain(|entry| entry != &deletion_entry); + } else { + return Err(anyhow!("Deletion entry {:?} not found in GasSchedule", deletion_entry)); + } + } + + // Bump the feature version as we are adding or removing params + current_schedule.feature_version.checked_add(1) + .ok_or(anyhow::anyhow!("Overflow when bumping feature version"))?; generate_update_proposal(¤t_schedule, self.output .unwrap_or_else(|| DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH.to_string())) @@ -195,6 +248,9 @@ impl GasScheduleGenerator { GasScheduleGenerator::ScaleCurrent(args) => { args.execute() } + GasScheduleGenerator::UpdateSchedule(args) => { + args.execute() + } } } } diff --git a/aptos-move/aptos-gas-schedule/src/ver.rs b/aptos-move/aptos-gas-schedule/src/ver.rs index b1c44ddeca24b..9cd8e473901f9 100644 --- a/aptos-move/aptos-gas-schedule/src/ver.rs +++ b/aptos-move/aptos-gas-schedule/src/ver.rs @@ -73,7 +73,7 @@ /// global operations. /// - V1 /// - TBA -pub const LATEST_GAS_FEATURE_VERSION: u64 = gas_feature_versions::RELEASE_V1_16_SUPRA_V1_7_14; +pub const LATEST_GAS_FEATURE_VERSION: u64 = gas_feature_versions::RELEASE_V1_16_SUPRA_V1_7_15; pub mod gas_feature_versions { pub const RELEASE_V1_8: u64 = 11; @@ -89,4 +89,5 @@ pub mod gas_feature_versions { pub const RELEASE_V1_16_SUPRA_V1_5_1: u64 = 22; pub const RELEASE_V1_16_SUPRA_V1_6_0: u64 = 23; pub const RELEASE_V1_16_SUPRA_V1_7_14: u64 = 24; + pub const RELEASE_V1_16_SUPRA_V1_7_15: u64 = 25; } diff --git a/types/src/on_chain_config/gas_schedule.rs b/types/src/on_chain_config/gas_schedule.rs index 24632cc273c0c..99e04947c8a2a 100644 --- a/types/src/on_chain_config/gas_schedule.rs +++ b/types/src/on_chain_config/gas_schedule.rs @@ -62,11 +62,11 @@ impl GasSchedule { impl GasScheduleV2 { - pub fn from_json_string(json_str: String) -> Self { - serde_json::from_str(&json_str).unwrap() + pub fn from_json_string(json_str: String) -> anyhow::Result { + serde_json::from_str(&json_str).map_err(|e| anyhow::anyhow!(e)) } - pub fn scale_min_gas_price_by(&mut self, factor: f64) { + pub fn scale_min_gas_unit_price_by(&mut self, factor: f64) { for (name, value) in &mut self.entries { if name == KEY_MIN_PRICE_PER_GAS { // Convert to f64, multiply, then convert back to u64 with saturation @@ -186,20 +186,20 @@ mod tests { ], }; - gas_schedule.scale_min_gas_price_by(0.5); + gas_schedule.scale_min_gas_unit_price_by(0.5); // Check that only the min gas price was scaled assert_eq!(gas_schedule.entries[0], ("other.param".to_string(), 100)); assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 25)); assert_eq!(gas_schedule.entries[2], ("another.param".to_string(), 200)); - gas_schedule.scale_min_gas_price_by(2.0); + gas_schedule.scale_min_gas_unit_price_by(2.0); assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 50)); - gas_schedule.scale_min_gas_price_by(10.0); + gas_schedule.scale_min_gas_unit_price_by(10.0); assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 500)); - gas_schedule.scale_min_gas_price_by(0.1); + gas_schedule.scale_min_gas_unit_price_by(0.1); assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 50)); } @@ -213,7 +213,7 @@ mod tests { }; // Test rounding behavior (100 * 1.234 = 123.4, should round to 123) - gas_schedule.scale_min_gas_price_by(1.234); + gas_schedule.scale_min_gas_unit_price_by(1.234); assert_eq!(gas_schedule.entries[0].1, 123); } @@ -227,7 +227,7 @@ mod tests { }; // Should not panic with large factor - gas_schedule.scale_min_gas_price_by(2.0); + gas_schedule.scale_min_gas_unit_price_by(2.0); // The result will be clamped to u64::MAX due to overflow in f64 to u64 conversion assert_eq!(gas_schedule.entries[0].1, u64::MAX); } @@ -244,7 +244,7 @@ mod tests { }"# .to_string(); - let mut schedule = GasScheduleV2::from_json_string(json); + let mut schedule = GasScheduleV2::from_json_string(json).unwrap(); assert_eq!(schedule.feature_version, 42); assert_eq!(schedule.entries.len(), 3); @@ -252,7 +252,7 @@ mod tests { assert_eq!(schedule.entries[1], ("bar".to_string(), 456)); assert_eq!(schedule.entries[2], ("txn.min_price_per_gas_unit".to_string(), 50)); - schedule.scale_min_gas_price_by(10.0); + schedule.scale_min_gas_unit_price_by(10.0); assert_eq!(schedule.entries[2], ("txn.min_price_per_gas_unit".to_string(), 500)); } From 6188a755361ce313681656bd6601cdbd2da3e05c Mon Sep 17 00:00:00 2001 From: supra-yoga Date: Thu, 9 Oct 2025 16:36:27 +0530 Subject: [PATCH 4/6] Minor optimizations --- .../src/change_set.rs | 26 ++-- .../aptos-gas-schedule-updator/src/lib.rs | 116 +++++++++++------- types/src/on_chain_config/gas_schedule.rs | 48 +++++--- 3 files changed, 119 insertions(+), 71 deletions(-) diff --git a/aptos-move/aptos-gas-schedule-updator/src/change_set.rs b/aptos-move/aptos-gas-schedule-updator/src/change_set.rs index 74259e5baa31e..206262865ebc7 100644 --- a/aptos-move/aptos-gas-schedule-updator/src/change_set.rs +++ b/aptos-move/aptos-gas-schedule-updator/src/change_set.rs @@ -1,12 +1,12 @@ -use std::collections::HashMap; use serde::{Deserialize, Serialize}; +use std::collections::HashMap; pub type Entries = HashMap; #[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] pub struct GasScheduleChangeSet { additions: Entries, - deletions: Entries + deletions: Entries, } impl GasScheduleChangeSet { @@ -14,12 +14,20 @@ impl GasScheduleChangeSet { serde_json::from_str(&json_str).map_err(|e| anyhow::anyhow!(e)) } - pub fn deletion_entries(&self) -> Entries { - self.deletions.clone() + pub fn deletions(&self) -> &Entries { + &self.deletions + } + + pub fn additions(&self) -> &Entries { + &self.additions } - pub fn addition_entries(&self) -> Entries { - self.additions.clone() + pub fn deletion_entries(&self) -> &Entries { + self.deletions() + } + + pub fn addition_entries(&self) -> &Entries { + self.additions() } pub fn is_empty(&self) -> bool { @@ -38,13 +46,13 @@ fn test_deserialize_change_set() { "bar": 456 } }"# - .to_string(); + .to_string(); - let mut change_set = GasScheduleChangeSet::from_json_string(json).unwrap(); + let change_set = GasScheduleChangeSet::from_json_string(json).unwrap(); assert_eq!(change_set.additions.len(), 2); assert_eq!(change_set.deletions.len(), 1); assert_eq!(change_set.additions.get("foo").unwrap(), &123u64); assert_eq!(change_set.additions.get("bar").unwrap(), &600999u64); assert_eq!(change_set.deletions.get("bar").unwrap(), &456u64); -} \ No newline at end of file +} diff --git a/aptos-move/aptos-gas-schedule-updator/src/lib.rs b/aptos-move/aptos-gas-schedule-updator/src/lib.rs index 587fb05b29df7..65edcb7efb248 100644 --- a/aptos-move/aptos-gas-schedule-updator/src/lib.rs +++ b/aptos-move/aptos-gas-schedule-updator/src/lib.rs @@ -9,7 +9,7 @@ mod change_set; -use std::fs; +use crate::change_set::GasScheduleChangeSet; use anyhow::{anyhow, Result}; use aptos_gas_schedule::{ AptosGasParameters, InitialGasSchedule, ToOnChainGasSchedule, LATEST_GAS_FEATURE_VERSION, @@ -19,8 +19,8 @@ use aptos_types::on_chain_config::GasScheduleV2; use clap::{Args, Parser}; use move_core_types::account_address::AccountAddress; use move_model::{code_writer::CodeWriter, emit, emitln, model::Loc}; +use std::fs; use std::path::{Path, PathBuf}; -use crate::change_set::GasScheduleChangeSet; const DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH: &str = "./proposals"; @@ -113,7 +113,7 @@ fn aptos_framework_path() -> PathBuf { pub enum GasScheduleGenerator { GenerateNew(GenerateNewSchedule), ScaleCurrent(ScaleCurrentSchedule), - UpdateSchedule(UpdateSchedule) + UpdateSchedule(UpdateSchedule), } /// Command line arguments to the gas schedule update proposal generation tool. @@ -122,7 +122,7 @@ pub struct GenerateNewSchedule { #[clap(short, long, help = "Path to file to write the output script")] pub output: Option, - #[clap(short, long, help="Feature version of the GasSchedule generated")] + #[clap(short, long, help = "Feature version of the GasSchedule generated")] pub gas_feature_version: Option, } @@ -134,25 +134,34 @@ impl GenerateNewSchedule { let gas_schedule = current_gas_schedule(feature_version); - generate_update_proposal(&gas_schedule, self.output - .unwrap_or_else(|| DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH.to_string())) + generate_update_proposal( + &gas_schedule, + self.output + .unwrap_or_else(|| DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH.to_string()), + ) } } - #[derive(Debug, Args)] pub struct ScaleCurrentSchedule { #[clap(short, long, help = "Path to file to write the output script")] pub output: Option, - #[clap(short, long, help = "Path to JSON file containing the GasScheduleV2 to use")] + #[clap( + short, + long, + help = "Path to JSON file containing the GasScheduleV2 to use" + )] pub current_schedule_path: String, - #[clap(short, long, help = "Scale the Minimum Gas Unit Price value by the given factor")] + #[clap( + short, + long, + help = "Scale the Minimum Gas Unit Price value by the given factor" + )] pub scale_min_gas_unit_price_by: f64, } - impl ScaleCurrentSchedule { pub fn execute(self) -> Result<()> { let json_str = fs::read_to_string(self.current_schedule_path)?; @@ -160,8 +169,11 @@ impl ScaleCurrentSchedule { current_schedule.scale_min_gas_unit_price_by(self.scale_min_gas_unit_price_by); - generate_update_proposal(¤t_schedule, self.output - .unwrap_or_else(|| DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH.to_string())) + generate_update_proposal( + ¤t_schedule, + self.output + .unwrap_or_else(|| DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH.to_string()), + ) } } @@ -170,10 +182,18 @@ pub struct UpdateSchedule { #[clap(short, long, help = "Path to file to write the output script")] pub output: Option, - #[clap(short, long, help = "Path to JSON file containing the GasScheduleV2 to update")] + #[clap( + short, + long, + help = "Path to JSON file containing the GasScheduleV2 to update" + )] pub current_schedule_path: String, - #[clap(short, long, help = "Path to JSON file containing change set to the GasScheduleV2")] + #[clap( + short, + long, + help = "Path to JSON file containing change set to the GasScheduleV2" + )] pub change_set_path: String, } @@ -189,28 +209,50 @@ impl UpdateSchedule { let current_schedule_json_str = fs::read_to_string(self.current_schedule_path)?; let mut current_schedule = GasScheduleV2::from_json_string(current_schedule_json_str)?; - for addition_entry in change_set.addition_entries() { - if !current_schedule.entries.contains(&addition_entry) { - current_schedule.entries.push(addition_entry); - } else { - return Err(anyhow!("Addition entry {:?} is already found in GasSchedule", addition_entry)); + for (name, value) in change_set.additions().iter() { + if current_schedule + .entries + .iter() + .any(|entry| entry.0 == *name && entry.1 == *value) + { + return Err(anyhow!( + "Addition entry ({}, {}) is already found in GasSchedule", + name, + value + )); } + + current_schedule.entries.push((name.clone(), *value)); } - for deletion_entry in change_set.deletion_entries() { - if current_schedule.entries.contains(&deletion_entry) { - current_schedule.entries.retain(|entry| entry != &deletion_entry); + for (name, value) in change_set.deletions().iter() { + if let Some(position) = current_schedule + .entries + .iter() + .position(|entry| entry.0 == *name && entry.1 == *value) + { + current_schedule.entries.remove(position); } else { - return Err(anyhow!("Deletion entry {:?} not found in GasSchedule", deletion_entry)); + return Err(anyhow!( + "Deletion entry ({}, {}) not found in GasSchedule", + name, + value + )); } } // Bump the feature version as we are adding or removing params - current_schedule.feature_version.checked_add(1) - .ok_or(anyhow::anyhow!("Overflow when bumping feature version"))?; - - generate_update_proposal(¤t_schedule, self.output - .unwrap_or_else(|| DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH.to_string())) + let new_feature_version = current_schedule + .feature_version + .checked_add(1) + .ok_or_else(|| anyhow!("Overflow when bumping feature version"))?; + current_schedule.feature_version = new_feature_version; + + generate_update_proposal( + ¤t_schedule, + self.output + .unwrap_or_else(|| DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH.to_string()), + ) } } @@ -226,10 +268,7 @@ pub fn current_gas_schedule(feature_version: u64) -> GasScheduleV2 { pub fn generate_update_proposal(gas_schedule: &GasScheduleV2, output_path: String) -> Result<()> { let mut pack = PackageBuilder::new("GasScheduleUpdate"); - pack.add_source( - "update_gas_schedule.move", - &generate_script(gas_schedule)?, - ); + pack.add_source("update_gas_schedule.move", &generate_script(gas_schedule)?); // TODO: use relative path here pack.add_local_dep("SupraFramework", &aptos_framework_path().to_string_lossy()); @@ -238,19 +277,12 @@ pub fn generate_update_proposal(gas_schedule: &GasScheduleV2, output_path: Strin Ok(()) } - impl GasScheduleGenerator { pub fn execute(self) -> Result<()> { match self { - GasScheduleGenerator::GenerateNew(args) => { - args.execute() - } - GasScheduleGenerator::ScaleCurrent(args) => { - args.execute() - } - GasScheduleGenerator::UpdateSchedule(args) => { - args.execute() - } + GasScheduleGenerator::GenerateNew(args) => args.execute(), + GasScheduleGenerator::ScaleCurrent(args) => args.execute(), + GasScheduleGenerator::UpdateSchedule(args) => args.execute(), } } } diff --git a/types/src/on_chain_config/gas_schedule.rs b/types/src/on_chain_config/gas_schedule.rs index 99e04947c8a2a..d574c5b017146 100644 --- a/types/src/on_chain_config/gas_schedule.rs +++ b/types/src/on_chain_config/gas_schedule.rs @@ -17,9 +17,7 @@ pub struct GasSchedule { #[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] pub struct GasScheduleV2 { pub feature_version: u64, - #[serde( - deserialize_with = "deserialize_gas_schedule_entries", - )] + #[serde(deserialize_with = "deserialize_gas_schedule_entries")] pub entries: Vec<(String, u64)>, } @@ -61,7 +59,6 @@ impl GasSchedule { } impl GasScheduleV2 { - pub fn from_json_string(json_str: String) -> anyhow::Result { serde_json::from_str(&json_str).map_err(|e| anyhow::anyhow!(e)) } @@ -76,7 +73,6 @@ impl GasScheduleV2 { } } - pub fn into_btree_map(self) -> BTreeMap { // TODO: what if the gas schedule contains duplicated entries? self.entries.into_iter().collect() @@ -129,9 +125,7 @@ enum GasEntryValue { String(String), } -fn deserialize_gas_schedule_entries<'de, D>( - deserializer: D, -) -> Result, D::Error> +fn deserialize_gas_schedule_entries<'de, D>(deserializer: D) -> Result, D::Error> where D: Deserializer<'de>, { @@ -190,26 +184,36 @@ mod tests { // Check that only the min gas price was scaled assert_eq!(gas_schedule.entries[0], ("other.param".to_string(), 100)); - assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 25)); + assert_eq!( + gas_schedule.entries[1], + (KEY_MIN_PRICE_PER_GAS.to_string(), 25) + ); assert_eq!(gas_schedule.entries[2], ("another.param".to_string(), 200)); gas_schedule.scale_min_gas_unit_price_by(2.0); - assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 50)); + assert_eq!( + gas_schedule.entries[1], + (KEY_MIN_PRICE_PER_GAS.to_string(), 50) + ); gas_schedule.scale_min_gas_unit_price_by(10.0); - assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 500)); + assert_eq!( + gas_schedule.entries[1], + (KEY_MIN_PRICE_PER_GAS.to_string(), 500) + ); gas_schedule.scale_min_gas_unit_price_by(0.1); - assert_eq!(gas_schedule.entries[1], (KEY_MIN_PRICE_PER_GAS.to_string(), 50)); + assert_eq!( + gas_schedule.entries[1], + (KEY_MIN_PRICE_PER_GAS.to_string(), 50) + ); } #[test] fn test_scale_min_gas_price_by_rounding() { let mut gas_schedule = GasScheduleV2 { feature_version: 1, - entries: vec![ - (KEY_MIN_PRICE_PER_GAS.to_string(), 100), - ], + entries: vec![(KEY_MIN_PRICE_PER_GAS.to_string(), 100)], }; // Test rounding behavior (100 * 1.234 = 123.4, should round to 123) @@ -221,9 +225,7 @@ mod tests { fn test_scale_min_gas_price_by_overflow_protection() { let mut gas_schedule = GasScheduleV2 { feature_version: 1, - entries: vec![ - (KEY_MIN_PRICE_PER_GAS.to_string(), u64::MAX), - ], + entries: vec![(KEY_MIN_PRICE_PER_GAS.to_string(), u64::MAX)], }; // Should not panic with large factor @@ -250,10 +252,16 @@ mod tests { assert_eq!(schedule.entries.len(), 3); assert_eq!(schedule.entries[0], ("foo".to_string(), 123)); assert_eq!(schedule.entries[1], ("bar".to_string(), 456)); - assert_eq!(schedule.entries[2], ("txn.min_price_per_gas_unit".to_string(), 50)); + assert_eq!( + schedule.entries[2], + ("txn.min_price_per_gas_unit".to_string(), 50) + ); schedule.scale_min_gas_unit_price_by(10.0); - assert_eq!(schedule.entries[2], ("txn.min_price_per_gas_unit".to_string(), 500)); + assert_eq!( + schedule.entries[2], + ("txn.min_price_per_gas_unit".to_string(), 500) + ); } } From bf49adbc48f4dc56320cb95f381674be76064e30 Mon Sep 17 00:00:00 2001 From: supra-yoga Date: Thu, 9 Oct 2025 23:17:14 +0530 Subject: [PATCH 5/6] feat(cli): refactor Schedule updator commands --- Cargo.lock | 2 + .../src/change_set.rs | 23 +++-- .../aptos-gas-schedule-updator/src/lib.rs | 69 ++++++--------- types/src/on_chain_config/gas_schedule.rs | 85 ------------------- 4 files changed, 42 insertions(+), 137 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4845505134e92..234267d9b80e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1898,6 +1898,8 @@ dependencies = [ "clap 4.4.14", "move-core-types", "move-model", + "serde", + "serde_json", "tempfile", ] diff --git a/aptos-move/aptos-gas-schedule-updator/src/change_set.rs b/aptos-move/aptos-gas-schedule-updator/src/change_set.rs index 206262865ebc7..dcca4f1b1aeff 100644 --- a/aptos-move/aptos-gas-schedule-updator/src/change_set.rs +++ b/aptos-move/aptos-gas-schedule-updator/src/change_set.rs @@ -7,6 +7,7 @@ pub type Entries = HashMap; pub struct GasScheduleChangeSet { additions: Entries, deletions: Entries, + mutations: Entries, } impl GasScheduleChangeSet { @@ -22,16 +23,16 @@ impl GasScheduleChangeSet { &self.additions } - pub fn deletion_entries(&self) -> &Entries { - self.deletions() + pub fn mutations(&self) -> &Entries { + &self.mutations } - pub fn addition_entries(&self) -> &Entries { - self.additions() + pub fn is_empty(&self) -> bool { + self.additions.is_empty() && self.deletions.is_empty() && self.mutations.is_empty() } - pub fn is_empty(&self) -> bool { - self.additions.is_empty() && self.deletions.is_empty() + pub fn should_bump_feature_version(&self) -> bool { + !self.additions.is_empty() || !self.deletions.is_empty() } } @@ -40,10 +41,13 @@ fn test_deserialize_change_set() { let json = r#"{ "additions": { "foo": 123, - "bar": 600999 + "bar": 100999 }, "deletions": { "bar": 456 + }, + "mutations": { + "foo": 789 } }"# .to_string(); @@ -52,7 +56,8 @@ fn test_deserialize_change_set() { assert_eq!(change_set.additions.len(), 2); assert_eq!(change_set.deletions.len(), 1); + assert_eq!(change_set.mutations.len(), 1); assert_eq!(change_set.additions.get("foo").unwrap(), &123u64); - assert_eq!(change_set.additions.get("bar").unwrap(), &600999u64); - assert_eq!(change_set.deletions.get("bar").unwrap(), &456u64); + assert_eq!(change_set.additions.get("bar").unwrap(), &100999u64); + assert_eq!(change_set.deletions.get("foo").unwrap(), &789u64); } diff --git a/aptos-move/aptos-gas-schedule-updator/src/lib.rs b/aptos-move/aptos-gas-schedule-updator/src/lib.rs index 65edcb7efb248..481f7e477440b 100644 --- a/aptos-move/aptos-gas-schedule-updator/src/lib.rs +++ b/aptos-move/aptos-gas-schedule-updator/src/lib.rs @@ -112,7 +112,6 @@ fn aptos_framework_path() -> PathBuf { #[derive(Parser, Debug)] pub enum GasScheduleGenerator { GenerateNew(GenerateNewSchedule), - ScaleCurrent(ScaleCurrentSchedule), UpdateSchedule(UpdateSchedule), } @@ -142,41 +141,6 @@ impl GenerateNewSchedule { } } -#[derive(Debug, Args)] -pub struct ScaleCurrentSchedule { - #[clap(short, long, help = "Path to file to write the output script")] - pub output: Option, - - #[clap( - short, - long, - help = "Path to JSON file containing the GasScheduleV2 to use" - )] - pub current_schedule_path: String, - - #[clap( - short, - long, - help = "Scale the Minimum Gas Unit Price value by the given factor" - )] - pub scale_min_gas_unit_price_by: f64, -} - -impl ScaleCurrentSchedule { - pub fn execute(self) -> Result<()> { - let json_str = fs::read_to_string(self.current_schedule_path)?; - let mut current_schedule = GasScheduleV2::from_json_string(json_str)?; - - current_schedule.scale_min_gas_unit_price_by(self.scale_min_gas_unit_price_by); - - generate_update_proposal( - ¤t_schedule, - self.output - .unwrap_or_else(|| DEFAULT_GAS_SCHEDULE_SCRIPT_UPDATE_PATH.to_string()), - ) - } -} - #[derive(Parser, Debug)] pub struct UpdateSchedule { #[clap(short, long, help = "Path to file to write the output script")] @@ -241,12 +205,32 @@ impl UpdateSchedule { } } - // Bump the feature version as we are adding or removing params - let new_feature_version = current_schedule - .feature_version - .checked_add(1) - .ok_or_else(|| anyhow!("Overflow when bumping feature version"))?; - current_schedule.feature_version = new_feature_version; + for (name, value) in change_set.mutations().iter() { + if let Some(position) = current_schedule + .entries + .iter() + .position(|entry| entry.0 == *name ) + { + // Remove old entry and insert new entry + current_schedule.entries.remove(position); + current_schedule.entries.push((name.clone(), *value)); + } else { + return Err(anyhow!( + "Mutation entry ({}, {}) not found in GasSchedule", + name, + value + )); + } + } + + // Bump the feature version if we are adding or removing params + if change_set.should_bump_feature_version() { + let new_feature_version = current_schedule + .feature_version + .checked_add(1) + .ok_or_else(|| anyhow!("Overflow when bumping feature version"))?; + current_schedule.feature_version = new_feature_version; + } generate_update_proposal( ¤t_schedule, @@ -281,7 +265,6 @@ impl GasScheduleGenerator { pub fn execute(self) -> Result<()> { match self { GasScheduleGenerator::GenerateNew(args) => args.execute(), - GasScheduleGenerator::ScaleCurrent(args) => args.execute(), GasScheduleGenerator::UpdateSchedule(args) => args.execute(), } } diff --git a/types/src/on_chain_config/gas_schedule.rs b/types/src/on_chain_config/gas_schedule.rs index d574c5b017146..c015bf334e76b 100644 --- a/types/src/on_chain_config/gas_schedule.rs +++ b/types/src/on_chain_config/gas_schedule.rs @@ -3,12 +3,9 @@ use crate::on_chain_config::OnChainConfig; use serde::de::{self, Deserializer}; -use serde::ser::{SerializeSeq, Serializer}; use serde::{Deserialize, Serialize}; use std::collections::{btree_map, BTreeMap}; -const KEY_MIN_PRICE_PER_GAS: &str = "txn.min_price_per_gas_unit"; - #[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] pub struct GasSchedule { pub entries: Vec<(String, u64)>, @@ -63,16 +60,6 @@ impl GasScheduleV2 { serde_json::from_str(&json_str).map_err(|e| anyhow::anyhow!(e)) } - pub fn scale_min_gas_unit_price_by(&mut self, factor: f64) { - for (name, value) in &mut self.entries { - if name == KEY_MIN_PRICE_PER_GAS { - // Convert to f64, multiply, then convert back to u64 with saturation - let new_value = (*value as f64 * factor).round() as u64; - *value = new_value; - } - } - } - pub fn into_btree_map(self) -> BTreeMap { // TODO: what if the gas schedule contains duplicated entries? self.entries.into_iter().collect() @@ -169,71 +156,6 @@ impl OnChainConfig for StorageGasSchedule { mod tests { use super::*; - #[test] - fn test_scale_min_gas_price_by() { - let mut gas_schedule = GasScheduleV2 { - feature_version: 1, - entries: vec![ - ("other.param".to_string(), 100), - (KEY_MIN_PRICE_PER_GAS.to_string(), 50), - ("another.param".to_string(), 200), - ], - }; - - gas_schedule.scale_min_gas_unit_price_by(0.5); - - // Check that only the min gas price was scaled - assert_eq!(gas_schedule.entries[0], ("other.param".to_string(), 100)); - assert_eq!( - gas_schedule.entries[1], - (KEY_MIN_PRICE_PER_GAS.to_string(), 25) - ); - assert_eq!(gas_schedule.entries[2], ("another.param".to_string(), 200)); - - gas_schedule.scale_min_gas_unit_price_by(2.0); - assert_eq!( - gas_schedule.entries[1], - (KEY_MIN_PRICE_PER_GAS.to_string(), 50) - ); - - gas_schedule.scale_min_gas_unit_price_by(10.0); - assert_eq!( - gas_schedule.entries[1], - (KEY_MIN_PRICE_PER_GAS.to_string(), 500) - ); - - gas_schedule.scale_min_gas_unit_price_by(0.1); - assert_eq!( - gas_schedule.entries[1], - (KEY_MIN_PRICE_PER_GAS.to_string(), 50) - ); - } - - #[test] - fn test_scale_min_gas_price_by_rounding() { - let mut gas_schedule = GasScheduleV2 { - feature_version: 1, - entries: vec![(KEY_MIN_PRICE_PER_GAS.to_string(), 100)], - }; - - // Test rounding behavior (100 * 1.234 = 123.4, should round to 123) - gas_schedule.scale_min_gas_unit_price_by(1.234); - assert_eq!(gas_schedule.entries[0].1, 123); - } - - #[test] - fn test_scale_min_gas_price_by_overflow_protection() { - let mut gas_schedule = GasScheduleV2 { - feature_version: 1, - entries: vec![(KEY_MIN_PRICE_PER_GAS.to_string(), u64::MAX)], - }; - - // Should not panic with large factor - gas_schedule.scale_min_gas_unit_price_by(2.0); - // The result will be clamped to u64::MAX due to overflow in f64 to u64 conversion - assert_eq!(gas_schedule.entries[0].1, u64::MAX); - } - #[test] fn test_deserialize_map_entries_with_string_values() { let json = r#"{ @@ -256,12 +178,5 @@ mod tests { schedule.entries[2], ("txn.min_price_per_gas_unit".to_string(), 50) ); - - schedule.scale_min_gas_unit_price_by(10.0); - - assert_eq!( - schedule.entries[2], - ("txn.min_price_per_gas_unit".to_string(), 500) - ); } } From ea2946a6e1fd39f5800e480d67f47c96a6e76c25 Mon Sep 17 00:00:00 2001 From: supra-yoga Date: Mon, 13 Oct 2025 15:40:29 +0530 Subject: [PATCH 6/6] Add comments --- .../src/change_set.rs | 12 +++++ .../aptos-gas-schedule-updator/src/lib.rs | 50 +++++++++++++++++++ types/src/on_chain_config/gas_schedule.rs | 8 +++ 3 files changed, 70 insertions(+) diff --git a/aptos-move/aptos-gas-schedule-updator/src/change_set.rs b/aptos-move/aptos-gas-schedule-updator/src/change_set.rs index dcca4f1b1aeff..6e96befbf8cdc 100644 --- a/aptos-move/aptos-gas-schedule-updator/src/change_set.rs +++ b/aptos-move/aptos-gas-schedule-updator/src/change_set.rs @@ -3,6 +3,10 @@ use std::collections::HashMap; pub type Entries = HashMap; +/// A set of changes to be applied to a gas schedule. +/// Additions are new entries to be added to the gas schedule. +/// Deletions are entries to be removed from the gas schedule. +/// Mutations are entries to be updated in the gas schedule. #[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)] pub struct GasScheduleChangeSet { additions: Entries, @@ -11,6 +15,8 @@ pub struct GasScheduleChangeSet { } impl GasScheduleChangeSet { + + /// Deserialize a GasScheduleChangeSet from a JSON string. pub fn from_json_string(json_str: String) -> anyhow::Result { serde_json::from_str(&json_str).map_err(|e| anyhow::anyhow!(e)) } @@ -27,16 +33,22 @@ impl GasScheduleChangeSet { &self.mutations } + /// Returns true if the change set is empty. pub fn is_empty(&self) -> bool { self.additions.is_empty() && self.deletions.is_empty() && self.mutations.is_empty() } + /// Returns true if the change set contains any additions or deletions. + /// This indicates that a feature version bump is required. + /// A mutation alone does not require a feature version bump. + // Check `aptos-core/aptos-move/aptos-gas-schedule/src/ver.rs` for more context. pub fn should_bump_feature_version(&self) -> bool { !self.additions.is_empty() || !self.deletions.is_empty() } } #[test] +// Test deserialization of GasScheduleChangeSet from JSON string. fn test_deserialize_change_set() { let json = r#"{ "additions": { diff --git a/aptos-move/aptos-gas-schedule-updator/src/lib.rs b/aptos-move/aptos-gas-schedule-updator/src/lib.rs index 481f7e477440b..95f830df71eab 100644 --- a/aptos-move/aptos-gas-schedule-updator/src/lib.rs +++ b/aptos-move/aptos-gas-schedule-updator/src/lib.rs @@ -109,18 +109,40 @@ fn aptos_framework_path() -> PathBuf { ) } +/// Command line interface for the gas schedule update proposal generation tool. +/// It supports two modes: +/// 1. Generate a new gas schedule from the current hardcoded values. +/// 2. Update an existing gas schedule with a change set. +/// The generated proposal is written to a Move package in the specified output directory. +/// If no output directory is specified, it defaults to `./proposals`. +/// The generated package contains a single Move script `update_gas_schedule.move`. +/// This script can be submitted as a governance proposal to update the on-chain gas schedule. +/// The script includes a comment section listing the contents of the gas schedule in a human readable format. #[derive(Parser, Debug)] pub enum GasScheduleGenerator { + /// Generate a new gas schedule from the current hardcoded values. + /// Optionally specify the feature version of the gas schedule. + /// If not specified, it defaults to the latest feature version. GenerateNew(GenerateNewSchedule), + /// Update an existing gas schedule with a change set. + /// The change set is specified in a JSON file. + /// The current gas schedule is also specified in a JSON file. + /// The change set can include additions, deletions, and mutations of gas parameters. + /// If the change set includes any additions or deletions, the feature version of the gas + /// schedule is bumped by 1. UpdateSchedule(UpdateSchedule), } /// Command line arguments to the gas schedule update proposal generation tool. #[derive(Debug, Args)] pub struct GenerateNewSchedule { + /// Path to file to write the output script. + /// If not specified, it defaults to `./proposals`. #[clap(short, long, help = "Path to file to write the output script")] pub output: Option, + /// Feature version of the gas schedule to generate. + /// If not specified, it defaults to the latest feature version. #[clap(short, long, help = "Feature version of the GasSchedule generated")] pub gas_feature_version: Option, } @@ -143,9 +165,14 @@ impl GenerateNewSchedule { #[derive(Parser, Debug)] pub struct UpdateSchedule { + /// Path to file to write the output script. + /// If not specified, it defaults to `./proposals`. #[clap(short, long, help = "Path to file to write the output script")] pub output: Option, + /// Path to JSON file containing the current GasScheduleV2 to update. + /// The JSON file should be in the format produced by the `to_json_string` method + /// of the `GasScheduleV2` struct. #[clap( short, long, @@ -153,6 +180,9 @@ pub struct UpdateSchedule { )] pub current_schedule_path: String, + /// Path to JSON file containing the change set to apply to the current GasScheduleV2. + /// The JSON file should be in the format produced by the `to_json_string` method + /// of the `GasScheduleChangeSet` struct. #[clap( short, long, @@ -173,6 +203,7 @@ impl UpdateSchedule { let current_schedule_json_str = fs::read_to_string(self.current_schedule_path)?; let mut current_schedule = GasScheduleV2::from_json_string(current_schedule_json_str)?; + // Apply additions to the gas schedule for (name, value) in change_set.additions().iter() { if current_schedule .entries @@ -189,6 +220,7 @@ impl UpdateSchedule { current_schedule.entries.push((name.clone(), *value)); } + // Apply deletions to existing entries in the gas schedule for (name, value) in change_set.deletions().iter() { if let Some(position) = current_schedule .entries @@ -205,6 +237,7 @@ impl UpdateSchedule { } } + // Apply mutations to existing entries in the gas schedule for (name, value) in change_set.mutations().iter() { if let Some(position) = current_schedule .entries @@ -223,6 +256,17 @@ impl UpdateSchedule { } } + + // Sort the entries by name to ensure deterministic order + current_schedule.entries.sort_by(|a, b| a.0.cmp(&b.0)); + // Ensure no duplicate entries exist + let mut seen = std::collections::HashSet::new(); + for (name, _) in ¤t_schedule.entries { + if !seen.insert(name) { + return Err(anyhow!("Duplicate entry ({}) found in GasSchedule", name)); + } + } + // Bump the feature version if we are adding or removing params if change_set.should_bump_feature_version() { let new_feature_version = current_schedule @@ -232,6 +276,12 @@ impl UpdateSchedule { current_schedule.feature_version = new_feature_version; } + println!( + "Updated gas schedule to feature version {}", + current_schedule.feature_version + ); + + // Generate the update proposal script generate_update_proposal( ¤t_schedule, self.output diff --git a/types/src/on_chain_config/gas_schedule.rs b/types/src/on_chain_config/gas_schedule.rs index c015bf334e76b..dfbf7c3349f34 100644 --- a/types/src/on_chain_config/gas_schedule.rs +++ b/types/src/on_chain_config/gas_schedule.rs @@ -98,6 +98,12 @@ impl GasScheduleV2 { } } +// Helper enum to facilitate deserialization of gas schedule entries that can be either +// tuples or maps. +// Examples of valid formats: +// 1. Tuple format: ["foo", 123] +// 2. Map format: { "key": "foo", "val": 123 } +// 3. Map format with string value: { "key": "foo", "val": "123" } #[derive(Deserialize)] #[serde(untagged)] enum GasEntryHelper { @@ -105,6 +111,7 @@ enum GasEntryHelper { Map { key: String, val: GasEntryValue }, } +// Helper enum to represent the value in the map format, which can be either a number or a string. #[derive(Deserialize)] #[serde(untagged)] enum GasEntryValue { @@ -112,6 +119,7 @@ enum GasEntryValue { String(String), } +// Custom deserializer for gas schedule entries to handle both tuple and map formats. fn deserialize_gas_schedule_entries<'de, D>(deserializer: D) -> Result, D::Error> where D: Deserializer<'de>,