Feature/sma 54 max factor upper bound is hardcoded by mrnkslv · Pull Request #73 · RSquad/ton-rust-node

mrnkslv · 2026-04-08T14:24:16Z

Summary

Remove the hardcoded max_factor upper bound of 3.0 and read the actual limit from the network's config param 17 (max_stake_factor). Previously, nodectl rejected any max_factor above 3.0 even on networks (e.g. testnet) that allow up to 30.0.

Changes

Centralised conversion helpers

-ton_utils: MAX_STAKE_FACTOR_SCALE, max_stake_factor_raw_to_multiplier(), extract_max_factor(ConfigParamEnum) — единая точка для param 17 → множитель.
-commands/utils: fetch_network_max_factor() — get_config_param(17) + extract_max_factor.

Redundant methods removed

-client_json_rpc: remove network_max_stake_factor_raw() and network_max_stake_factor_multiplier() — callers now use get_config_param(17) directly with the conversion helpers above.

Validation

-ElectionsConfig::validate(Option<f32>): accept an optional upper bound.
None (offline, e.g. AppConfig::load): only checks max_factor >= 1.0 — no upper cap, so configs written for high-limit networks still load.
Some(m) (service startup / reload): enforces max_factor ∈ [1.0, m] using the live network value.
-config elections max-factor CLI: loads config first, checks elections are configured, then fetches param 17 via try_create_rpc_client (without vault) and validates against the network limit.
-config wallet stake --max-factor CLI: same RPC-based validation before submitting the stake.
-RuntimeConfigStore (service): new validate_elections_max_factor_vs_chain() helper validates on initialize() and reload(); on RPC failure logs a warning and falls back to validate(None) instead of crashing.

Elections runner

-runner: single calc_max_factor(network_max_stake_factor_raw, warn_if_clamped) -> (u32, f32): computes the Elector fixed-point max_factor and the float multiplier; clamps the configured value to the network cap from config param 17 (same semantics as before, using max_stake_factor_raw_to_multiplier for the float form). tracing::warn! when the value is clamped runs only when warn_if_clamped is true (e.g. in participate), so snapshot updates do not emit the same warning every tick.

Tests

-test_elections_validate_max_factor_respects_network_cap — Some(3.0) rejects 5.0, Some(5.0) accepts it.
-test_elections_validate_none_allows_max_factor_above_default_cap — None accepts 25.0, Some(3.0) rejects, Some(30.0) accepts.
-Timing validation tests (sleep_period_pct, waiting_period_pct) updated to use validate(None).

Documentation

-README: updated config elections max-factor, elections config section, and config wallet stake table to reflect that the upper bound comes from config param 17, not a hardcoded 3.0.

Closes SMA-54

linear · 2026-04-08T14:24:20Z

SMA-54 Max factor upper bound is hardcoded

Copilot

Pull request overview

This PR removes the hardcoded maximum stake factor limit of 3.0 and instead reads the actual network limit from config param 17 (max_stake_factor). This allows validators on networks with different limits (e.g., testnet with 30.0) to properly configure their max stake factors.

Changes:

Added utility functions to convert raw fixed-point stake factors to float multipliers
Added RPC methods to fetch the network's max stake factor from config param 17
Modified validation logic to accept optional upper bounds, allowing offline configs while enforcing network limits at startup
Updated CLI commands (config elections max-factor, config wallet stake) to validate against live network values
Enhanced error handling and messaging throughout to reference config param 17

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`ton_utils.rs`	Added MAX_STAKE_FACTOR_SCALE constant and conversion function
`client_json_rpc.rs`	Added methods to fetch network max stake factor from RPC
`app_config.rs`	Updated ElectionsConfig::validate to accept optional network upper bound
`runtime_config.rs`	Added validation against network max on service startup/reload
`runner.rs`	Updated stake validation to use network max, improved error messages
`config_elections_cmd.rs`	Added RPC-based validation for max-factor CLI command
`config_wallet_cmd.rs`	Added RPC-based validation for stake CLI command
`README.md`	Updated documentation to reflect network-based limits

This pull request is ready to be approved.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Keshoid · 2026-04-08T16:34:36Z

src/node-control/elections/src/runner.rs

    ) -> anyhow::Result<()> {
-        let max_factor = (self.calc_max_factor() * 65536.0) as u32;
+        let max_factor = ((self.calc_max_factor() * 65536.0) as u32)
+            .clamp(common::ton_utils::MAX_STAKE_FACTOR_SCALE, params.cfg17.max_stake_factor);


Log a warning if max factor is clamped:

let configured_factor = (self.calc_max_factor() * 65536.0) as u32; let max_factor = configured_factor.clamp(MAX_STAKE_FACTOR_SCALE, params.cfg17.max_stake_factor); if max_factor != raw { tracing::warn!( "max_factor clamped: configured={}, used={} (network limit from cfg17)", configured_factor as f32 / MAX_STAKE_FACTOR_SCALE, max_factor as f32 / MAX_STAKE_FACTOR_SCALE, ); }

Keshoid · 2026-04-08T16:37:31Z

src/node-control/elections/src/runner.rs

        params: &ConfigParams<'_>,
    ) -> anyhow::Result<()> {
-        let max_factor = (self.calc_max_factor() * 65536.0) as u32;
+        let max_factor = ((self.calc_max_factor() * 65536.0) as u32)


Can you use MAX_STAKE_FACTOR_SCALE instead of 65536.0?

Let's pass params.cfg17.max_stake_factor to self.calc_max_factor(params.cfg17.max_stake_factor) to calculate real max factor in one place.

Keshoid · 2026-04-08T16:42:45Z

src/node-control/elections/src/runner.rs

+            anyhow::bail!(
+                "<max-factor> must be between 1.0 and {} (network max_stake_factor from config param 17)",
+                common::ton_utils::max_stake_factor_raw_to_multiplier(network_max_stake_factor)
+            );


Let's remove this check because we already clamp max factor in a participate() function.

Keshoid · 2026-04-08T16:43:57Z

src/node-control/common/src/ton_utils.rs

 }

+/// Elector uses fixed-point `max_stake_factor`: raw value is multiplier × 65536 (e.g. 3× → `3 * 65536`).
+pub const MAX_STAKE_FACTOR_SCALE: u32 = 65536;


I think we can define it as f32 = 65536.0;

Keshoid · 2026-04-08T16:45:47Z

src/node-control/common/src/app_config.rs

+    ///
+    /// - `None`: only checks `max_factor >= 1.0` (e.g. [`AppConfig::load`] without RPC). No upper bound.
+    /// - `Some(m)`: `max_factor` must be in `[1.0, m]` where `m` is from config param 17 (service startup).
+    pub fn validate(&self, max_stake_factor_upper_bound: Option<f32>) -> anyhow::Result<()> {


You can omit word stake in a name max_stake_factor_upper_bound -> max_factor_upper_bound

src/node-control/common/src/app_config.rs

Keshoid · 2026-04-08T16:51:43Z

src/node-control/ton-http-api-client/src/v2/client_json_rpc.rs

+        let raw = self.network_max_stake_factor_raw().await?;
+        Ok(common::ton_utils::max_stake_factor_raw_to_multiplier(raw))
+    }
+


I think these functions a redundant because we already have get_config_param(17). Function max_stake_factor_raw_to_multiplier should be called on the caller side.

Keshoid · 2026-04-08T16:54:16Z

src/node-control/commands/src/commands/nodectl/config_wallet_cmd.rs

+            .network_max_stake_factor_multiplier()
+            .await
+            .context("read max_stake_factor from chain (config param 17)")?;
+        if !(1.0..=network_max).contains(&self.max_factor) {


Rename to network_max_factor

Keshoid · 2026-04-08T16:57:05Z

src/node-control/service/src/runtime_config.rs

+                .context("read max_stake_factor for elections config validation")?;
+            elections
+                .validate(Some(network_max))
+                .context("elections max_factor vs chain (config param 17)")?;


I think we don't need additional context here. The error message inside a validate function is self descriptive.

Keshoid · 2026-04-08T16:59:14Z

src/node-control/commands/src/commands/nodectl/config_elections_cmd.rs

    pub async fn run(&self, path: &Path) -> anyhow::Result<()> {
-        if !(1.0..=3.0).contains(&self.value) {
-            anyhow::bail!("max-factor must be in range [1.0..3.0]");
+        let (_app, _vault, rpc_client) = load_config_vault_rpc_client(path).await?;


You don't need vault here so create rpc_client separately (we already have such function which creates only rpc client).

Keshoid · 2026-04-08T17:01:32Z

src/node-control/service/src/runtime_config.rs

+            let network_max = rpc_client
+                .network_max_stake_factor_multiplier()
+                .await
+                .context("read max_stake_factor for elections config validation")?;


Service startup blocked by RPC failure. Both initialize() and reload() now fail if the RPC call fails. If the TON HTTP API is temporarily unreachable, the service won't start even if the configured max_factor is perfectly valid (e.g., the default 3.0). Consider whether falling back to validate(None) with a warning on RPC failure is preferable to hard-failing.

Keshoid · 2026-04-08T17:03:20Z

The "Changes" section says:
config_params: add parse_config_param_16() and parse_config_param_17()

These already exist in control-client/src/config_params.rs and aren't part of this diff. The description should be corrected to avoid confusion during future archaeology.

…ture/sma-54-max-factor-upper-bound-is-hardcoded-v2

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Keshoid · 2026-04-09T12:32:42Z

src/node-control/commands/src/commands/nodectl/config_elections_cmd.rs

+            );
+        }
+
+        config.elections.as_mut().unwrap().max_factor = self.value;


Try to avoid unwrap() semantic, we only use it it tests.

Keshoid · 2026-04-09T12:34:18Z

src/node-control/commands/src/commands/nodectl/utils.rs


+/// `max_stake_factor` from masterchain config param 17 as a float multiplier (e.g. `3.0`).
+pub async fn fetch_network_max_factor(rpc_client: &ClientJsonRpc) -> anyhow::Result<f32> {
+    common::ton_utils::extract_max_stake_factor(rpc_client.get_config_param(17).await?)


import with use .... We only use full paths with third-party crates.

Keshoid · 2026-04-09T12:37:09Z

src/node-control/common/src/ton_utils.rs

+}
+
+/// Extracts `max_stake_factor` from a `ConfigParamEnum` (must be param 17) as a float multiplier.
+pub fn extract_max_stake_factor(param: ton_block::ConfigParamEnum) -> anyhow::Result<f32> {


Use use ton_block::ConfigParamEnum to import type. We use full paths with third-party crates only.

Keshoid · 2026-04-09T12:45:56Z

src/node-control/service/src/runtime_config.rs

+        match rpc_client
+            .get_config_param(17)
+            .await
+            .and_then(common::ton_utils::extract_max_stake_factor)


Import type with use.

Keshoid · 2026-04-09T12:52:02Z

src/node-control/common/src/ton_utils.rs

+}
+
+/// Extracts `max_stake_factor` from a `ConfigParamEnum` (must be param 17) as a float multiplier.
+pub fn extract_max_stake_factor(param: ton_block::ConfigParamEnum) -> anyhow::Result<f32> {


Can you remove word stake from the name? Because max_factor is more often used for searching.

fix: max factor load from config

4ef0fcf

Copilot AI review requested due to automatic review settings April 8, 2026 14:24

Copilot started reviewing on behalf of mrnkslv April 8, 2026 14:24 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

mrnkslv changed the title ~~fix: max factor load from config~~ Feature/sma 54 max factor upper bound is hardcoded Apr 8, 2026

mrnkslv requested review from ITBear, Keshoid and Lapo4kaKek April 8, 2026 14:59

mrnkslv added 2 commits April 8, 2026 18:50

fix: default factor func not pub

60b598d

fix: fix test

28b7178

Keshoid requested changes Apr 8, 2026

View reviewed changes

Keshoid reviewed Apr 8, 2026

View reviewed changes

mrnkslv added 2 commits April 9, 2026 13:17

Merge remote-tracking branch 'origin/release/nodectl/v0.4.0' into fea…

c7719f2

…ture/sma-54-max-factor-upper-bound-is-hardcoded-v2

fix: corrections according to comments

ea9ff35

mrnkslv requested a review from Copilot April 9, 2026 11:22

Copilot started reviewing on behalf of mrnkslv April 9, 2026 11:23 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Keshoid reviewed Apr 9, 2026

View reviewed changes

fix: corrections according to comments

ffde621

mrnkslv requested a review from Copilot April 9, 2026 23:47

Copilot started reviewing on behalf of mrnkslv April 9, 2026 23:47 View session

mrnkslv requested a review from Keshoid April 9, 2026 23:53

Conversation

mrnkslv commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Centralised conversion helpers

Redundant methods removed

Validation

Elections runner

Tests

Documentation

Uh oh!

linear bot commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Keshoid commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrnkslv commented Apr 8, 2026 •

edited

Loading