-
Notifications
You must be signed in to change notification settings - Fork 228
fix(precompiles): validate V1/V2 activity state in initialize_if_migrated (ZELLIC-64) #2864
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -611,10 +611,23 @@ impl ValidatorConfigV2 { | |
|
|
||
| // NOTE: this count comparison is sufficient because `add_validator` and | ||
| // `rotate_validator` are blocked until the contract is initialized. | ||
| if self.validator_count()? < v1.validator_count()? { | ||
| let v2_count = self.validator_count()?; | ||
| let v1_count = v1.validator_count()?; | ||
| if v2_count < v1_count { | ||
| Err(ValidatorConfigV2Error::migration_not_complete())? | ||
| } | ||
|
|
||
| for i in 0..v1_count { | ||
| 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))? | ||
| } | ||
| if !v1_val.active && v2_val.deactivated_at_height == 0 { | ||
| Err(ValidatorConfigV2Error::migration_state_mismatch(i))? | ||
|
Comment on lines
+623
to
+627
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's simplify this: if v1_val.active != (v2_val.deactivated == 0) {
return Err(ValidatorConfigV2Error::migration_state_mismatch(i));
} |
||
| } | ||
| } | ||
|
|
||
| let v1_next_dkg = v1.get_next_full_dkg_ceremony()?; | ||
| self.next_dkg_ceremony.write(v1_next_dkg)?; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -356,6 +356,17 @@ contract ValidatorConfigV2 is IValidatorConfigV2 { | |
| revert MigrationNotComplete(); | ||
| } | ||
|
|
||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can probably express this in simpler terms similar to the rust code. |
||
| revert MigrationStateMismatch(i); | ||
| } | ||
| if (!v1Active && v2Deactivated == 0) { | ||
| revert MigrationStateMismatch(i); | ||
| } | ||
| } | ||
|
|
||
| nextDkgCeremony = v1.getNextFullDkgCeremony(); | ||
|
|
||
| _initialized = true; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,10 @@ interface IValidatorConfigV2 { | |
| /// @notice Thrown when migration is not complete (not all V1 validators migrated) | ||
| error MigrationNotComplete(); | ||
|
|
||
| /// @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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be added to the spec? |
||
|
|
||
| /// @notice Thrown when migration index is out of order | ||
| error InvalidMigrationIndex(); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some smol unit test would be nice