Skip to content

fix(x/slashing): migrate signing info on SignedBlocksWindow change#26017

Open
SaswataPatra wants to merge 4 commits intocosmos:mainfrom
SaswataPatra:fix/slashing-window-migration
Open

fix(x/slashing): migrate signing info on SignedBlocksWindow change#26017
SaswataPatra wants to merge 4 commits intocosmos:mainfrom
SaswataPatra:fix/slashing-window-migration

Conversation

@SaswataPatra
Copy link
Copy Markdown

Fixes #12541

When the SignedBlocksWindow parameter changes via governance, validator signing info and missed block bitmaps must be migrated to prevent incorrect MissedBlocksCounter values and orphaned bitmap data.

This implementation uses proportional scaling with bitmap adjustment to preserve validators' relative miss rates while ensuring bitmap-counter invariants remain intact. The approach prevents both unfair immediate slashing and validators receiving undeserved clean slates.

The migration logic:

  1. Calculates proportional counter: oldCounter × (newWindow / oldWindow)
  2. Truncates bitmap to new window size
  3. Adjusts bitmap to match proportional counter:
    • Excess misses: removes from earliest positions
    • Insufficient misses: adds at positions visited last
  4. Updates IndexOffset: oldOffset % newWindow

This ensures bitmap and counter remain synchronized while maintaining fairness across window size changes.

Description

Closes: #XXXX

Fixes cosmos#12541

When the SignedBlocksWindow parameter changes via governance, validator
signing info and missed block bitmaps must be migrated to prevent
incorrect MissedBlocksCounter values and orphaned bitmap data.

This implementation uses proportional scaling with bitmap adjustment to
preserve validators' relative miss rates while ensuring bitmap-counter
invariants remain intact. The approach prevents both unfair immediate
slashing and validators receiving undeserved clean slates.

The migration logic:
1. Calculates proportional counter: oldCounter × (newWindow / oldWindow)
2. Truncates bitmap to new window size
3. Adjusts bitmap to match proportional counter:
   - Excess misses: removes from earliest positions
   - Insufficient misses: adds at positions visited last
4. Updates IndexOffset: oldOffset % newWindow

This ensures bitmap and counter remain synchronized while maintaining
fairness across window size changes.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 26, 2026

Greptile Summary

This PR implements automatic migration of validator signing info and missed block bitmaps when the SignedBlocksWindow parameter changes via governance. The migration uses proportional scaling to preserve validators' relative miss rates while maintaining bitmap-counter consistency.

Key changes:

  • Triggers migration before updating params when SignedBlocksWindow changes
  • Proportionally scales MissedBlocksCounter: (oldCounter × newWindow) / oldWindow
  • Truncates bitmap to new window size and adjusts to match scaled counter
  • Updates IndexOffset to wrap within new window

Critical issues found:

  • Error propagation bug: Errors during migration (from DeleteMissedBlockBitmap, SetMissedBlockBitmapValue, SetValidatorSigningInfo) are not returned to the caller. The handler returns true to stop iteration, but IterateValidatorSigningInfos always returns nil. This causes partial migration failures to succeed silently, leaving some validators migrated and others not.
  • Non-atomic migration: If migration fails partway through (which currently fails silently), the system enters an inconsistent state where some validators use the new window size and others don't.

Test coverage: Comprehensive tests for single-validator scenarios covering window shrinkage, expansion, and edge cases. However, tests don't cover multi-validator scenarios or error conditions.

Confidence Score: 2/5

  • This PR has a critical bug that causes migration errors to be silently ignored, leading to inconsistent validator state
  • The error propagation bug in MigrateSignedBlocksWindow is a critical issue that prevents proper error handling. When migration fails for any validator (e.g., database errors, bitmap operations failures), the function silently stops processing remaining validators but returns success to the caller. This results in partial migration where some validators are updated to the new window size while others remain on the old window, creating state inconsistency. The migration logic and tests are otherwise well-designed, but this bug makes the feature unsafe for production use.
  • Pay close attention to x/slashing/keeper/signing_info.go - the MigrateSignedBlocksWindow function needs error propagation fixes before this can be safely merged

Important Files Changed

Filename Overview
x/slashing/keeper/msg_server.go Adds migration trigger when SignedBlocksWindow parameter changes - clean implementation with proper ordering
x/slashing/keeper/signing_info.go Implements migration logic with proportional scaling - has critical error propagation bug that silently fails
x/slashing/keeper/signing_info_migration_test.go Comprehensive tests for single-validator scenarios - missing multi-validator and error handling tests

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[UpdateParams Called] --> B{SignedBlocksWindow Changed?}
    B -->|No| C[Update Params]
    B -->|Yes| D[Get Old Window Size]
    D --> E[Call MigrateSignedBlocksWindow]
    E --> F[Iterate All Validators]
    F --> G[Calculate Proportional Counter]
    G --> H[Read Missed Positions from Old Bitmap]
    H --> I[Delete Old Bitmap]
    I --> J{actualMissedCount vs proportionalCounter?}
    J -->|More misses| K[Remove excess from left]
    J -->|Fewer misses| L[Add fabricated misses at positions visited last]
    J -->|Equal| M[Keep existing positions]
    K --> N[Write Adjusted Bitmap]
    L --> N
    M --> N
    N --> O[Update MissedBlocksCounter]
    O --> P[Update IndexOffset % newWindow]
    P --> Q[Save ValidatorSigningInfo]
    Q --> R{More Validators?}
    R -->|Yes| F
    R -->|No| S{Migration Successful?}
    S -->|Yes| C
    S -->|No| T[Return Error - Params NOT Updated]
    
    style T fill:#ff6b6b
    style S fill:#ffd93d
    style Q fill:#6bcf7f
Loading

Last reviewed commit: 67634c0

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Added comprehensive test cases for the migration of validator signing info and missed block bitmaps when the SignedBlocksWindow parameter changes. The new tests cover scenarios with multiple validators and handle corrupted state guards, ensuring that the migration logic correctly adjusts missed blocks and counters while maintaining fairness and integrity across validators.
Copy link
Copy Markdown
Author

@SaswataPatra SaswataPatra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in latest commit: we now collect validators first and migrate in a loop so errors propagate; also added overflow guard by clamping MissedBlocksCounter to oldWindow.”

@aljo242
Copy link
Copy Markdown
Contributor

aljo242 commented Mar 24, 2026

@SaswataPatra conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validator missed blocks count is incorrect after decreasing slashing window

2 participants