Repository: stellar/rs-soroban-env (rust)

Issue:
Use a vector instead of an array for holding cost types
Both [cost_tracker](https://github.com/stellar/rs-soroban-env/blob/f8d3bafa19b934741efbb6f8ef3311696b3556de/soroban-env-host/src/budget.rs#L37) and [cost_models](https://github.com/stellar/rs-soroban-env/blob/f8d3bafa19b934741efbb6f8ef3311696b3556de/soroban-env-host/src/budget/dimension.rs#L18) have their lengths set to the size of the xdr enum at compile time because they're arrays. but the actual number of cost types depend on the protocol. Because of this, core needs to [resize the array](https://github.com/stellar/stellar-core/pull/4276#discussion_r1556533691) when we're on v20 but using a v21 supported build. This is not ideal. The host should just expect the right number of cost types for the protocol instead.

Pull request:
Allow `Budget::try_from_configs` inputs to be less than current xdr-defined count
### What

Resolves #1388 

### Why

The current way of constructing the Budget from config enforces the config size must match the protocol-defined count (`ContractCostType::variants().len()`). While this ensures the validity of the config entry for the current protocol, it does not take into consideration config from an earlier protocol, with fewer number of cost types. 

### Known limitations

[TODO or N/A]


Patch:
diff --git a/soroban-env-host/src/budget.rs b/soroban-env-host/src/budget.rs
index 00552668..b91a8e15 100644
--- a/soroban-env-host/src/budget.rs
+++ b/soroban-env-host/src/budget.rs
@@ -33,8 +33,8 @@ pub struct CostTracker {
 
 #[derive(Clone)]
 struct BudgetTracker {
-    // Tracks the `(sum_of_iterations, total_input)` for each `CostType`
-    cost_tracker: [CostTracker; ContractCostType::variants().len()],
+    // Tracker for each `CostType`
+    cost_trackers: [CostTracker; ContractCostType::variants().len()],
     // Total number of times the meter is called
     meter_count: u32,
     #[cfg(any(test, feature = "testutils", feature = "bench"))]
@@ -46,15 +46,15 @@ struct BudgetTracker {
 impl Default for BudgetTracker {
     fn default() -> Self {
         let mut mt = Self {
-            cost_tracker: [CostTracker::default(); ContractCostType::variants().len()],
-            meter_count: Default::default(),
+            cost_trackers: [CostTracker::default(); ContractCostType::variants().len()],
+            meter_count: 0,
             #[cfg(any(test, feature = "testutils", feature = "bench"))]
-            wasm_memory: Default::default(),
-            time_tracker: [0_u64; ContractCostType::variants().len()],
+            wasm_memory: 0,
+            time_tracker: [0; ContractCostType::variants().len()],
         };
         for (ct, tracker) in ContractCostType::variants()
             .iter()
-            .zip(mt.cost_tracker.iter_mut())
+            .zip(mt.cost_trackers.iter_mut())
         {
             // Define what inputs actually mean. For any constant-cost types --
             // whether it is a true constant unit cost type, or empirically
@@ -125,7 +125,7 @@ impl BudgetTracker {
     #[cfg(any(test, feature = "testutils", feature = "bench"))]
     fn reset(&mut self) {
         self.meter_count = 0;
-        for tracker in &mut self.cost_tracker {
+        for tracker in &mut self.cost_trackers {
             tracker.iterations = 0;
             tracker.inputs = tracker.inputs.map(|_| 0);
             tracker.cpu = 0;
@@ -172,18 +172,14 @@ impl BudgetImpl {
         cpu_cost_params: ContractCostParams,
         mem_cost_params: ContractCostParams,
     ) -> Result<Self, HostError> {
-        let mut b = Self {
-            cpu_insns: BudgetDimension::try_from_config(cpu_cost_params)?,
-            mem_bytes: BudgetDimension::try_from_config(mem_cost_params)?,
-            tracker: Default::default(),
+        Ok(Self {
+            cpu_insns: BudgetDimension::try_from_config(cpu_cost_params, cpu_limit)?,
+            mem_bytes: BudgetDimension::try_from_config(mem_cost_params, mem_limit)?,
+            tracker: BudgetTracker::default(),
             is_in_shadow_mode: false,
             fuel_costs: load_calibrated_fuel_costs(),
             depth_limit: DEFAULT_HOST_DEPTH_LIMIT,
-        };
-
-        b.cpu_insns.reset(cpu_limit);
-        b.mem_bytes.reset(mem_limit);
-        Ok(b)
+        })
     }
 
     pub fn charge(
@@ -194,7 +190,7 @@ impl BudgetImpl {
     ) -> Result<(), HostError> {
         let tracker = self
             .tracker
-            .cost_tracker
+            .cost_trackers
             .get_mut(ty as usize)
             .ok_or_else(|| HostError::from((ScErrorType::Budget, ScErrorCode::InternalError)))?;
 
@@ -239,12 +235,9 @@ impl BudgetImpl {
 
     fn get_wasmi_fuel_remaining(&self) -> Result<u64, HostError> {
         let cpu_remaining = self.cpu_insns.get_remaining();
-        let Some(cost_model) = self
+        let cost_model = self
             .cpu_insns
-            .get_cost_model(ContractCostType::WasmInsnExec)
-        else {
-            return Err((ScErrorType::Budget, ScErrorCode::InternalError).into());
-        };
+            .get_cost_model(ContractCostType::WasmInsnExec)?;
         let cpu_per_fuel = cost_model.const_term.max(1);
         // Due to rounding, the amount of cpu converted to fuel will be slightly
         // less than the total cpu available. This is okay because 1. that rounded-off
@@ -281,7 +274,7 @@ impl Default for BudgetImpl {
 
         for ct in ContractCostType::variants() {
             // define the cpu cost model parameters
-            let Some(cpu) = &mut b.cpu_insns.get_cost_model_mut(ct) else {
+            let Ok(cpu) = b.cpu_insns.get_cost_model_mut(ct) else {
                 continue;
             };
             match ct {
@@ -494,7 +487,7 @@ impl Default for BudgetImpl {
             }
 
             // define the memory cost model parameters
-            let Some(mem) = b.mem_bytes.get_cost_model_mut(ct) else {
+            let Ok(mem) = b.mem_bytes.get_cost_model_mut(ct) else {
                 continue;
             };
             match ct {
@@ -731,10 +724,10 @@ impl Debug for BudgetImpl {
                 f,
                 "{:<35}{:<15}{:<15}{:<15}{:<15}{:<20}{:<20}{:<20}{:<20}",
                 format!("{:?}", ct),
-                self.tracker.cost_tracker[i].iterations,
-                format!("{:?}", self.tracker.cost_tracker[i].inputs),
-                self.tracker.cost_tracker[i].cpu,
-                self.tracker.cost_tracker[i].mem,
+                self.tracker.cost_trackers[i].iterations,
+                format!("{:?}", self.tracker.cost_trackers[i].inputs),
+                self.tracker.cost_trackers[i].cpu,
+                self.tracker.cost_trackers[i].mem,
                 self.cpu_insns.cost_models[i].const_term,
                 format!("{}", self.cpu_insns.cost_models[i].lin_term),
                 self.mem_bytes.cost_models[i].const_term,
@@ -791,8 +784,8 @@ impl Display for BudgetImpl {
                 f,
                 "{:<35}{:<15}{:<15}",
                 format!("{:?}", ct),
-                self.tracker.cost_tracker[i].cpu,
-                self.tracker.cost_tracker[i].mem,
+                self.tracker.cost_trackers[i].cpu,
+                self.tracker.cost_trackers[i].mem,
             )?;
         }
         writeln!(f, "{:=<65}", "")?;
@@ -820,7 +813,7 @@ impl BudgetImpl {
         println!();
         println!();
         for ct in ContractCostType::variants() {
-            let Some(cpu) = self.cpu_insns.get_cost_model(ct) else {
+            let Ok(cpu) = self.cpu_insns.get_cost_model(ct) else {
                 continue;
             };
             println!("case {}:", ct.name());
@@ -835,7 +828,7 @@ impl BudgetImpl {
         println!();
         println!();
         for ct in ContractCostType::variants() {
-            let Some(mem) = self.mem_bytes.get_cost_model(ct) else {
+            let Ok(mem) = self.mem_bytes.get_cost_model(ct) else {
                 continue;
             };
             println!("case {}:", ct.name());
@@ -1011,7 +1004,7 @@ impl Budget {
         self.0
             .try_borrow_or_err()?
             .tracker
-            .cost_tracker
+            .cost_trackers
             .get(ty as usize)
             .map(|x| *x)
             .ok_or_else(|| (ScErrorType::Budget, ScErrorCode::InternalError).into())
@@ -1053,3 +1046,65 @@ impl Budget {
         Ok(())
     }
 }
+
+#[test]
+fn test_budget_initialization() -> Result<(), HostError> {
+    use crate::xdr::{ContractCostParamEntry, ExtensionPoint};
+    let cpu_cost_params = ContractCostParams(
+        vec![
+            ContractCostParamEntry {
+                ext: ExtensionPoint::V0,
+                const_term: 35,
+                linear_term: 36,
+            },
+            ContractCostParamEntry {
+                ext: ExtensionPoint::V0,
+                const_term: 37,
+                linear_term: 38,
+            },
+        ]
+        .try_into()
+        .unwrap(),
+    );
+    let mem_cost_params = ContractCostParams(
+        vec![
+            ContractCostParamEntry {
+                ext: ExtensionPoint::V0,
+                const_term: 39,
+                linear_term: 40,
+            },
+            ContractCostParamEntry {
+                ext: ExtensionPoint::V0,
+                const_term: 41,
+                linear_term: 42,
+            },
+            ContractCostParamEntry {
+                ext: ExtensionPoint::V0,
+                const_term: 43,
+                linear_term: 44,
+            },
+        ]
+        .try_into()
+        .unwrap(),
+    );
+
+    let budget = Budget::try_from_configs(100, 100, cpu_cost_params, mem_cost_params)?;
+    assert_eq!(
+        budget.0.try_borrow_or_err()?.cpu_insns.cost_models.len(),
+        ContractCostType::variants().len()
+    );
+    assert_eq!(
+        budget.0.try_borrow_or_err()?.mem_bytes.cost_models.len(),
+        ContractCostType::variants().len()
+    );
+    assert_eq!(
+        budget.0.try_borrow_or_err()?.tracker.cost_trackers.len(),
+        ContractCostType::variants().len()
+    );
+    assert_eq!(
+        budget.0.try_borrow_or_err()?.tracker.time_tracker.len(),
+        ContractCostType::variants().len()
+    );
+
+    Ok(())
+}
diff --git a/soroban-env-host/src/budget/dimension.rs b/soroban-env-host/src/budget/dimension.rs
index 75a62a08..516be287 100644
--- a/soroban-env-host/src/budget/dimension.rs
+++ b/soroban-env-host/src/budget/dimension.rs
@@ -40,10 +40,10 @@ impl Default for BudgetDimension {
     fn default() -> Self {
         Self {
             cost_models: [MeteredCostComponent::default(); ContractCostType::variants().len()],
-            limit: Default::default(),
-            total_count: Default::default(),
-            shadow_limit: Default::default(),
-            shadow_total_count: Default::default(),
+            limit: 0,
+            total_count: 0,
+            shadow_limit: 0,
+            shadow_total_count: 0,
         }
     }
 }
@@ -71,37 +71,54 @@ impl Debug for BudgetDimension {
 }
 
 impl BudgetDimension {
-    pub(crate) fn try_from_config(cost_params: ContractCostParams) -> Result<Self, HostError> {
-        let cost_models = cost_params
-            .0
-            .iter()
-            .map(|p| MeteredCostComponent::try_from(p))
-            .collect::<Result<Vec<MeteredCostComponent>, HostError>>()?;
-
-        Ok(Self {
-            cost_models: cost_models.try_into().map_err(|_| {
-                // error means the cost_params has the wrong dimension
+    pub(crate) fn try_from_config(
+        cost_params: ContractCostParams,
+        limit: u64,
+    ) -> Result<Self, HostError> {
+        let mut bd = BudgetDimension::default();
+        for (i, cp) in cost_params.0.iter().enumerate() {
+            let cm = bd.cost_models.get_mut(i).ok_or_else(|| {
+                // the index of ContractCostParams exceeds length of the cost
+                // models means attempting to construct the budget from a config
+                // that is not yet supported by the protocol
                 HostError::from(Error::from_type_and_code(
                     ScErrorType::Budget,
                     ScErrorCode::InternalError,
                 ))
-            })?,
-            limit: Default::default(),
-            total_count: Default::default(),
-            shadow_limit: Default::default(),
-            shadow_total_count: Default::default(),
-        })
+            })?;
+            *cm = MeteredCostComponent::try_from(cp)?;
+        }
+        bd.limit = limit;
+        bd.shadow_limit = limit;
+        Ok(bd)
     }
 
-    pub(crate) fn get_cost_model(&self, ty: ContractCostType) -> Option<&MeteredCostComponent> {
-        self.cost_models.get(ty as usize)
+    pub(crate) fn get_cost_model(
+        &self,
+        ty: ContractCostType,
+    ) -> Result<&MeteredCostComponent, HostError> {
+        self.cost_models.get(ty as usize).ok_or_else(|| {
+            // cost models are initialized with the static size of the
+            // ContractCostType, so this call should always succeed
+            HostError::from(Error::from_type_and_code(
+                ScErrorType::Budget,
+                ScErrorCode::InternalError,
+            ))
+        })
     }
 
     pub(crate) fn get_cost_model_mut(
         &mut self,
         ty: ContractCostType,
-    ) -> Option<&mut MeteredCostComponent> {
-        self.cost_models.get_mut(ty as usize)
+    ) -> Result<&mut MeteredCostComponent, HostError> {
+        self.cost_models.get_mut(ty as usize).ok_or_else(|| {
+            // cost models are initialized with the static size of the
+            // ContractCostType, so this call should always succeed
+            HostError::from(Error::from_type_and_code(
+                ScErrorType::Budget,
+                ScErrorCode::InternalError,
+            ))
+        })
     }
 
     pub(crate) fn get_total_count(&self) -> u64 {
@@ -147,9 +164,7 @@ impl BudgetDimension {
         _is_cpu: IsCpu,
         is_shadow: IsShadowMode,
     ) -> Result<u64, HostError> {
-        let Some(cm) = self.get_cost_model(ty) else {
-            return Err((ScErrorType::Budget, ScErrorCode::InternalError).into());
-        };
+        let cm = self.get_cost_model(ty)?;
         let amount = cm.evaluate(input)?.saturating_mul(iterations);
 
         #[cfg(all(not(target_family = "wasm"), feature = "tracy"))]
diff --git a/soroban-env-host/src/budget/util.rs b/soroban-env-host/src/budget/util.rs
index bdcbb37e..786eee62 100644
--- a/soroban-env-host/src/budget/util.rs
+++ b/soroban-env-host/src/budget/util.rs
@@ -128,19 +128,12 @@ impl Budget {
         const_mem: u64,
         lin_mem: ScaledU64,
     ) -> Result<(), HostError> {
-        use crate::xdr::{ScErrorCode, ScErrorType};
-
         let mut bgt = self.0.try_borrow_mut_or_err()?;
-
-        let Some(cpu_model) = bgt.cpu_insns.get_cost_model_mut(ty) else {
-            return Err((ScErrorType::Budget, ScErrorCode::InternalError).into());
-        };
+        let cpu_model = bgt.cpu_insns.get_cost_model_mut(ty)?;
         cpu_model.const_term = const_cpu;
         cpu_model.lin_term = lin_cpu;
 
-        let Some(mem_model) = bgt.mem_bytes.get_cost_model_mut(ty) else {
-            return Err((ScErrorType::Budget, ScErrorCode::InternalError).into());
-        };
+        let mem_model = bgt.mem_bytes.get_cost_model_mut(ty)?;
         mem_model.const_term = const_mem;
         mem_model.lin_term = lin_mem;
         Ok(())
diff --git a/soroban-simulation/src/network_config.rs b/soroban-simulation/src/network_config.rs
index 851023c3..94eb4ef4 100644
--- a/soroban-simulation/src/network_config.rs
+++ b/soroban-simulation/src/network_config.rs
@@ -5,9 +5,8 @@ use soroban_env_host::fees::{
     compute_write_fee_per_1kb, FeeConfiguration, RentFeeConfiguration, WriteFeeConfiguration,
 };
 use soroban_env_host::xdr::{
-    ConfigSettingEntry, ConfigSettingId, ContractCostParamEntry, ContractCostParams,
-    ContractCostType, ExtensionPoint, LedgerEntry, LedgerEntryData, LedgerKey,
-    LedgerKeyConfigSetting,
+    ConfigSettingEntry, ConfigSettingId, ContractCostParams, LedgerEntry, LedgerEntryData,
+    LedgerKey, LedgerKeyConfigSetting,
 };
 use soroban_env_host::LedgerInfo;
 use std::rc::Rc;
@@ -50,24 +49,6 @@ fn load_configuration_setting(
     }
 }
 
-fn extend_cost_params(params: &mut ContractCostParams) -> Result<()> {
-    let target_size = ContractCostType::variants().len();
-    if params.0.len() >= target_size {
-        return Ok(());
-    }
-    let mut v = params.to_vec();
-    v.resize(
-        target_size,
-        ContractCostParamEntry {
-            ext: ExtensionPoint::V0,
-            const_term: 0,
-            linear_term: 0,
-        },
-    );
-    params.0 = v.try_into().context("too many contract cost params")?;
-    Ok(())
-}
-
 macro_rules! load_setting {
     ($snapshot:ident, $enum_variant:ident) => {
         match load_configuration_setting($snapshot, ConfigSettingId::$enum_variant)? {
@@ -95,13 +76,8 @@ impl NetworkConfig {
         let events = load_setting!(snapshot, ContractEventsV0);
         let bandwidth = load_setting!(snapshot, ContractBandwidthV0);
         let state_archival = load_setting!(snapshot, StateArchival);
-        let mut cpu_cost_params = load_setting!(snapshot, ContractCostParamsCpuInstructions);
-        let mut memory_cost_params = load_setting!(snapshot, ContractCostParamsMemoryBytes);
-        // This is a hack to get around env using hardcoded cost param length
-        // instead of the protocol-defined length:
-        // https://github.com/stellar/rs-soroban-env/issues/1388.
-        extend_cost_params(&mut cpu_cost_params)?;
-        extend_cost_params(&mut memory_cost_params)?;
+        let cpu_cost_params = load_setting!(snapshot, ContractCostParamsCpuInstructions);
+        let memory_cost_params = load_setting!(snapshot, ContractCostParamsMemoryBytes);
 
         let write_fee_configuration = WriteFeeConfiguration {
             bucket_list_target_size_bytes: ledger_cost.bucket_list_target_size_bytes,


Test patch:
diff --git a/soroban-simulation/src/test/network_config.rs b/soroban-simulation/src/test/network_config.rs
index da404fac..7857e8cc 100644
--- a/soroban-simulation/src/test/network_config.rs
+++ b/soroban-simulation/src/test/network_config.rs
@@ -133,20 +133,6 @@ fn test_load_config_from_snapshot() {
         NetworkConfig::load_from_snapshot(&snapshot_source, 150_000_000_000_000).unwrap();
     // From tests/resources `test_compute_write_fee`
     let write_fee = 1_000_000_000 + 50 * (1_000_000_000_i64 - 1_000_000) / 2;
-
-    let empty_entry = ContractCostParamEntry {
-        ext: ExtensionPoint::V0,
-        const_term: 0,
-        linear_term: 0,
-    };
-    let mut expected_cpu_cost_params_vec = cpu_cost_params.to_vec();
-    expected_cpu_cost_params_vec.resize(ContractCostType::variants().len(), empty_entry.clone());
-    let expected_cpu_cost_params =
-        ContractCostParams(expected_cpu_cost_params_vec.try_into().unwrap());
-    let mut expected_mem_cost_params_vec = memory_cost_params.to_vec();
-    expected_mem_cost_params_vec.resize(ContractCostType::variants().len(), empty_entry.clone());
-    let expected_mem_cost_params =
-        ContractCostParams(expected_mem_cost_params_vec.try_into().unwrap());
     assert_eq!(
         network_config,
         NetworkConfig {
@@ -168,8 +154,8 @@ fn test_load_config_from_snapshot() {
             },
             tx_max_instructions: 2,
             tx_memory_limit: 4,
-            cpu_cost_params: expected_cpu_cost_params,
-            memory_cost_params: expected_mem_cost_params,
+            cpu_cost_params,
+            memory_cost_params,
             min_temp_entry_ttl: 27,
             min_persistent_entry_ttl: 28,
             max_entry_ttl: 26,