Skip to content

Inconsistent validation for disabledFeatures parameter #101

@0xKorok

Description

@0xKorok

Description

The disabledFeatures parameter has inconsistent validation. During contract initialization, values greater than 3 are rejected, but the runtime update function updateDisabledFeatures() accepts any uint32 value without validation.

Root Cause

Initialization validation (SSVBasedApps.sol):

if (config.disabledFeatures > 3) {
    revert InvalidDisabledFeatures();
}

Runtime update (ProtocolManager.sol):

function updateDisabledFeatures(uint32 disabledFeatures) external {
    ProtocolStorageLib.load().disabledFeatures = disabledFeatures;
    emit DisabledFeaturesUpdated(disabledFeatures);
    // No validation
}

Currently, only 2 features can be disabled (bits 0 and 1), making 3 the maximum valid value when both are disabled.

Impact

  1. Inconsistent behavior: Deployment rejects values > 3, but runtime updates accept any value
  2. Future compatibility issues: Setting undefined bits (e.g., disabledFeatures = 7) could unexpectedly disable future features when they're added

Proposed Solution

Add a constant to ValidationLib.sol and use consistent inline validation:

// ValidationLib.sol
uint32 constant MAX_DISABLED_FEATURES = 3;

Update both initialization and runtime validation to use this constant:

SSVBasedApps.sol initialization:

if (config.disabledFeatures > MAX_DISABLED_FEATURES) {
    revert InvalidDisabledFeatures();
}

ProtocolManager.sol runtime update:

function updateDisabledFeatures(uint32 disabledFeatures) external {
    if (disabledFeatures > MAX_DISABLED_FEATURES) {
        revert InvalidDisabledFeatures();
    }
    ProtocolStorageLib.load().disabledFeatures = disabledFeatures;
    emit DisabledFeaturesUpdated(disabledFeatures);
}

This approach:

  • Provides consistent validation across initialization and updates
  • Centralizes the maximum value definition in ValidationLib
  • Uses simple inline validation following existing code patterns
  • Makes future feature additions easier to manage by updating a single constant

Additional Context

The inconsistency suggests the validation was intended but inadvertently omitted from the runtime update function. While this requires admin error to trigger, consistent validation prevents unexpected behavior and improves maintainability.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions