Skip to content

[BUG] Silent error discard in ConformStateToParamUpdate: wrong variable checked after SetValidatorUnstakingIfBelowMinimum #493

Description

@Sertug17

Module

FSM (Finite State Machine) / Governance

Priority

High

Describe the Bug

In fsm/gov.go, the function ConformStateToParamUpdate() silently drops errors returned by SetValidatorUnstakingIfBelowMinimum. This means that when a governance parameter increases the minimum stake, validators that should be force-unstaked may remain active, leaving the chain state inconsistent.

Root Cause

Inside the closure passed to IterateAndExecute, the named return variable for the closure is e. However, after calling SetValidatorUnstakingIfBelowMinimum, the code checks err (the outer function-scope variable) instead of e (the closure's own error variable):

if err = s.IterateAndExecute(ValidatorPrefix(), func(key, value []byte) (e lib.ErrorI) {
    v, e := s.unmarshalValidator(value)
    if e != nil {
        return e
    }
    // BUG: checks `err` (outer scope, still nil at this point in the closure)
    //      instead of `e` (the closure's return variable)
    if _, e = s.SetValidatorUnstakingIfBelowMinimum(v, params.Validator); err != nil {
        return e
    }
    return
}); err != nil {

At the time the closure executes, err has not yet been assigned by IterateAndExecute (that assignment happens when the closure loop finishes). So err is always nil inside the closure, and any error from SetValidatorUnstakingIfBelowMinimum is silently discarded the closure returns nil and iteration continues to the next validator.

Impact

When MinimumStakeForValidators or MinimumStakeForDelegates is increased via a governance proposal and ConformStateToParamUpdate runs, if SetValidatorUnstakingIfBelowMinimum fails for any validator, the error is swallowed. This can result in:

  • Validators below the new minimum stake remaining active in committees when they should be force-unstaked
  • A partial and inconsistent state update some validators correctly unstaked, others silently skipped
  • Incorrect consensus committee membership after the governance change

Steps to Reproduce

  1. Deploy a Canopy node with validators staked at various amounts
  2. Submit a governance proposal to increase MinimumStakeForValidators
  3. If SetValidatorUnstakingIfBelowMinimum returns a non-nil error for any validator (e.g., due to a state write failure), that validator will remain active without any error being surfaced

Expected Behavior

If SetValidatorUnstakingIfBelowMinimum returns an error, ConformStateToParamUpdate should propagate it immediately and abort the state transition not silently continue.

Proposed Fix

Change the condition from err != nil to e != nil:

// BEFORE (buggy):
if _, e = s.SetValidatorUnstakingIfBelowMinimum(v, params.Validator); err != nil {
    return e
}

// AFTER (correct):
if _, e = s.SetValidatorUnstakingIfBelowMinimum(v, params.Validator); e != nil {
    return e
}

File / Line

fsm/gov.go function ConformStateToParamUpdate()

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