fix(precompiles): validate V1/V2 activity state in initialize_if_migrated (ZELLIC-64)#2864
Open
fix(precompiles): validate V1/V2 activity state in initialize_if_migrated (ZELLIC-64)#2864
Conversation
📊 Tempo Precompiles CoverageprecompilesCoverage: 20762/21794 lines (95.26%) File details
contractsCoverage: 209/386 lines (54.15%) File details
Total: 20971/22180 lines (94.55%) |
…ated ZELLIC-64: initialize_if_migrated only checked validator count parity between V1 and V2 but did not verify each validator's activity state. A validator could be deactivated in V2 during migration while still active in V1 (or vice versa), leading to inconsistent state. Add a per-validator check that V1.active matches V2.deactivatedAtHeight and revert with MigrationStateMismatch(idx) on mismatch. Amp-Thread-ID: https://ampcode.com/threads/T-019c964b-0561-763e-9d3c-c62a3398ca66 Co-authored-by: Amp <amp@ampcode.com>
7672297 to
da61a3b
Compare
0xrusowsky
approved these changes
Feb 26, 2026
SuperFluffy
approved these changes
Feb 26, 2026
Comment on lines
+623
to
+627
| if v1_val.active && v2_val.deactivated_at_height != 0 { | ||
| Err(ValidatorConfigV2Error::migration_state_mismatch(i))? | ||
| } | ||
| if !v1_val.active && v2_val.deactivated_at_height == 0 { | ||
| Err(ValidatorConfigV2Error::migration_state_mismatch(i))? |
Contributor
There was a problem hiding this comment.
Let's simplify this:
if v1_val.active != (v2_val.deactivated == 0) {
return Err(ValidatorConfigV2Error::migration_state_mismatch(i));
}|
|
||
| /// @notice Thrown when a migrated validator's active/deactivated state does not match V1 | ||
| /// @param idx The index of the validator with mismatched state | ||
| error MigrationStateMismatch(uint64 idx); |
Contributor
There was a problem hiding this comment.
Should this be added to the spec?
| for (uint64 i = 0; i < v1Validators.length; i++) { | ||
| bool v1Active = v1Validators[i].active; | ||
| uint64 v2Deactivated = validatorsArray[i].deactivatedAtHeight; | ||
| if (v1Active && v2Deactivated != 0) { |
Contributor
There was a problem hiding this comment.
Can probably express this in simpler terms similar to the rust code.
joshieDo
reviewed
Feb 26, 2026
| let v1_val = v1.validators(v1.validators_array(i)?)?; | ||
| let v2_val = self.validators[i as usize].read()?; | ||
| if v1_val.active && v2_val.deactivated_at_height != 0 { | ||
| Err(ValidatorConfigV2Error::migration_state_mismatch(i))? |
Contributor
There was a problem hiding this comment.
some smol unit test would be nice
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ZELLIC-64:
initialize_if_migratedonly checked that the validator count matched between V1 and V2, but did not verify each validator's activity state was consistent. A validator could be deactivated in V2 during migration while still active in V1 (or vice versa), leading to inconsistent state after initialization.Changes
initialize_if_migrated/initializeIfMigrated:active == true→ V2deactivatedAtHeightmust be0active == false→ V2deactivatedAtHeightmust be> 0MigrationStateMismatch(uint64 idx)error to identify which validator has mismatched stateFiles changed
tips/ref-impls/src/interfaces/IValidatorConfigV2.sol— new error definitiontips/ref-impls/src/ValidatorConfigV2.sol— activity state validation loopcrates/contracts/src/precompiles/validator_config_v2.rs— new error in sol! macro + constructorcrates/precompiles/src/validator_config_v2/mod.rs— activity state validation loop