Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(x/staking/keeper): make Slash return early iff zero tokens to burn #18049

Merged

Commits on Oct 16, 2023

  1. perf(x/staking/keeper): make Slash return early iff zero tokens to burn

    This change makes Keeper.Slash return early if there are no tokens
    to burn! This change also skips invoking the
    .Hooks().BeforeValidatorSlashed hook because literally there is no
    slashing that can happen if there are no tokens to burn.
    
    Given that the result of burning zero tokens SHOULD be a no-operation
    (noop) but we go through the whole routine, assuming no zero tokens
    would be burnt, one could argue on a protocol implementation/conformation
    that with zero tokens to burn, invoking Keeper.RemoveValidatorTokens which
    invokes:
    
      Keeper.DeleteValidatorByPowerIndex
        -> (Keeper.DeleteValidatorByPowerIndex)
        -> validator.RemoveTokens
        -> Keeper.SetValidator
        -> Keeper.SetValidatorByPowerIndex
    
    was causing a potential self inflicted Byzantine risk because that operation
    is non-atomic (it ran through a series of operations that could fail on
    their own yet could not be rolled back and not idempotent), will rely
    on network operations yet the actual result will have the operations back
    to the original: more investigation is needed to examine the risk/impact
    of previously letting zero tokens be burnt and run through that process.
    
    Also while here, employed a Go pattern to reuse condition variables just
    as a code hygiene improvement and also given that as a Go reviewer, the
    unnecessary allocation of variables however small must be avoided.
    
    Fixes #18034
    odeke-em committed Oct 16, 2023
    Configuration menu
    Copy the full SHA
    bb54a89 View commit details
    Browse the repository at this point in the history