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
- Deploy a Canopy node with validators staked at various amounts
- Submit a governance proposal to increase
MinimumStakeForValidators
- 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()
Module
FSM (Finite State Machine) / Governance
Priority
High
Describe the Bug
In
fsm/gov.go, the functionConformStateToParamUpdate()silently drops errors returned bySetValidatorUnstakingIfBelowMinimum. 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 ise. However, after callingSetValidatorUnstakingIfBelowMinimum, the code checkserr(the outer function-scope variable) instead ofe(the closure's own error variable):At the time the closure executes,
errhas not yet been assigned byIterateAndExecute(that assignment happens when the closure loop finishes). Soerris alwaysnilinside the closure, and any error fromSetValidatorUnstakingIfBelowMinimumis silently discarded the closure returnsniland iteration continues to the next validator.Impact
When
MinimumStakeForValidatorsorMinimumStakeForDelegatesis increased via a governance proposal andConformStateToParamUpdateruns, ifSetValidatorUnstakingIfBelowMinimumfails for any validator, the error is swallowed. This can result in:Steps to Reproduce
MinimumStakeForValidatorsSetValidatorUnstakingIfBelowMinimumreturns a non-nil error for any validator (e.g., due to a state write failure), that validator will remain active without any error being surfacedExpected Behavior
If
SetValidatorUnstakingIfBelowMinimumreturns an error,ConformStateToParamUpdateshould propagate it immediately and abort the state transition not silently continue.Proposed Fix
Change the condition from
err != niltoe != nil:File / Line
fsm/gov.gofunctionConformStateToParamUpdate()