RemoteSigningConfig validation should include retry configuration

[Kewe63]

Summary

RemoteSigningConfig::validate() validates the endpoint and TLS fields, but does not validate its nested RetryConfig.

As a result, retry settings that RetryConfig::validate() explicitly considers invalid—such as initial_backoff > max_backoff or backoff_multiplier < 1.0—are accepted by the top-level configuration used to construct RemoteSignerClient.

The parent validator should include the retry configuration and return a contextual configuration error before building the gRPC client or retry policy.

Affected files

• crates/remote-signer/src/config.rs
• crates/remote-signer/src/client.rs

The validation omission is in RemoteSigningConfig::validate(). RemoteSignerClient::new() demonstrates the user-visible path because it relies on that top-level method.

Observed behavior

The top-level validator currently checks only the endpoint and TLS consistency:

pub fn validate(&self) -> Result<(), String> {
    url::Url::parse(&self.endpoint)
        .map_err(|e| format!("Invalid endpoint URL: {e}"))?;

    if self.enable_tls && self.tls_cert_path.is_none() {
        return Err("TLS enabled but no certificate path provided".to_string());
    }

    Ok(())
}

The nested retry type has its own validation rules:

pub fn validate(&self) -> Result<(), String> {
    if self.initial_backoff >= self.max_backoff {
        return Err("initial_backoff must be less than max_backoff".to_string());
    }

    if self.backoff_multiplier < 1.0 {
        return Err("backoff_multiplier must be at least 1.0".to_string());
    }

    Ok(())
}

However, RemoteSigningConfig::validate() never calls self.retry_config.validate().

RemoteSignerClient::new() calls only the parent validator:

config
    .validate()
    .map_err(RemoteSigningError::Configuration)?;

It therefore accepts a top-level configuration containing retry values that the nested validator rejects.

Expected behavior

• RemoteSigningConfig::validate() should validate every nested configuration it owns.
• Invalid retry backoff ordering should be rejected before client/channel initialization.
• A retry multiplier below the supported minimum should also be rejected through the parent configuration.
• The returned error should identify the retry configuration and preserve the specific nested validation reason.
• Valid default and custom retry configurations should remain accepted.

Reproduction

I added two focused regression tests against current main.

Invalid backoff ordering

#[test]
fn remote_config_rejects_invalid_retry_backoff_order() {
    let retry_config = RetryConfig::new(
        3,
        Duration::from_secs(10),
        Duration::from_secs(1),
    );
    assert!(retry_config.validate().is_err());

    let config = RemoteSigningConfig::default().with_retry_config(retry_config);
    assert!(
        config.validate().is_err(),
        "RemoteSigningConfig accepted initial_backoff greater than max_backoff"
    );
}

Invalid multiplier

#[test]
fn remote_config_rejects_invalid_retry_multiplier() {
    let retry_config = RetryConfig::default().with_backoff_multiplier(0.5);
    assert!(retry_config.validate().is_err());

    let config = RemoteSigningConfig::default().with_retry_config(retry_config);
    assert!(
        config.validate().is_err(),
        "RemoteSigningConfig accepted backoff_multiplier below 1.0"
    );
}

Command:

cargo +1.94.0 test \
  -p arc-remote-signer \
  remote_config_rejects_invalid_retry_ \
  -- --nocapture

Result:

running 2 tests

test config::tests::remote_config_rejects_invalid_retry_backoff_order ... FAILED
test config::tests::remote_config_rejects_invalid_retry_multiplier ... FAILED

RemoteSigningConfig accepted initial_backoff greater than max_backoff
RemoteSigningConfig accepted backoff_multiplier below 1.0

test result: FAILED. 0 passed; 2 failed; 7 filtered out

In both tests, RetryConfig::validate() rejects the value first, proving that the nested invariants already exist. The failure is specifically that the top-level validator returns Ok(()) for the same nested value.

Tested against commit:

97f8da0dc4faa703fe2d68ca007e40dab2c8a9ef

Root cause

RemoteSigningConfig owns a RetryConfig, but its validation implementation does not delegate to the nested type's existing validator.

This allows the nested and parent validation contracts to disagree: a retry configuration is invalid when checked directly but valid when supplied through the production RemoteSigningConfig path.

Why this matters

The retry policy is used for remote-signing RPCs on a consensus node. Accepting contradictory backoff bounds or an unsupported multiplier makes retry behavior differ from the configuration contract and delays discovery of operator mistakes until after client initialization.

RemoteSignerClient::new() already treats top-level validation failures as RemoteSigningError::Configuration, so including the nested validation would use the existing error path and fail before networking.

Suggested fix

Delegate to the nested validator inside RemoteSigningConfig::validate() and add context to its error:

self.retry_config
    .validate()
    .map_err(|e| format!("Invalid retry configuration: {e}"))?;

This should run before client/channel initialization. No retry algorithm changes are required.

Potential regression tests

Add top-level validation tests covering:

• the default retry configuration remains valid;
• a valid custom retry configuration remains valid;
• initial_backoff > max_backoff is rejected;
• initial_backoff == max_backoff is rejected, matching the existing nested contract;
• backoff_multiplier < 1.0 is rejected;
• the returned top-level error identifies retry configuration and includes the nested reason.

Scope

This should be a small composition fix in crates/remote-signer/src/config.rs plus focused unit tests.

It should not change:

• retry counts or backoff calculations;
• gRPC request behavior;
• remote-signer authentication or TLS behavior;
• public-key caching;
• consensus or protocol behavior.

Duplicate and active-work check

I searched open and closed issues and pull requests using:

• RemoteSigningConfig validation
• RetryConfig::validate
• initial_backoff
• max_backoff
• retry configuration
• backoff_multiplier

No direct duplicate or existing validation fix was found.

The currently open remote-signer work was inspected:

• PR fix(remote-signer): add retry to get_public_key() matching sign_message() behavior #269 adds retry behavior to get_public_key() and modifies client.rs/error.rs; it does not change parent configuration validation.
• PR fix(consensus): exit non-zero on transient remote-signer startup failure #158 changes startup error propagation and modifies remote-signer client/error handling; it does not change config.rs validation.
• PR Fix #66, #31 and #72 #78 currently changes only .gitignore; its stale body mentions earlier test movement but its live diff does not touch remote-signer configuration.
• Closed PR test: move RemoteSigningConfig/RetryConfig field tests to config.rs unit tests #152 only moved field-value tests and was superseded by move config builder to unit test #67; it did not add nested validation.

These changes are related to the subsystem but do not implement this issue.