Minor fixes to testnet 78 migration #4596
Conversation
…ix bug with validator migration
zbuc
added
consensus-breaking
|
Nice, thanks for following up (refs #4567). I'll dig into migration testing for 78 today! |
|
Looks like the revised migration applies fine, but the chain cannot restart after the migration is run, because the halt bit is still set to true. A common mistake: I did the exact same thing when writing the Testnet77 migration (#4503) and had to follow up with a narrow fix in #4508. Unsurprisingly I see that the competing Testnet77 migration in #4564 does not make this same mistake, hat-tip to @erwanor on that. But let's fix it here, so we can proceed with migration testing, and sort out conflicts in #4654 separately. |
|
Oof, good catch. I had refactored the migration code to de-duplicate all the common operations in migrations but then reverted it and forgot to add those steps back to the testnet 78 migration. |
Thanks for tackling this, even if it didn't land. I remain in favor of cleaning up the migration tooling (#4294), but don't mind writing more migrations as practice to inform what the shape of that cleanup should be. |
| let next_proposal_id: u64 = delta.next_proposal_id().await?; | ||
|
|
||
| // Range each proposal and truncate the fields. | ||
| for proposal_id in 0..next_proposal_id { | ||
| tracing::info!("truncating proposal: {}", proposal_id); |
There was a problem hiding this comment.
@zbuc doesn't this have the same issue of pulling data using stronger rules that existed when it was stored?
There was a problem hiding this comment.
Yes, will follow up in a new PR
erwanor
added
migration
… in migration (#4606) ## Describe your changes Don't deserialize proto types to avoid applying new domain validations to old types in migration 78 ## Issue ticket number and link #4596 (comment) ## Checklist before requesting a review - [x ] If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: > Migration change
Describe your changes
Configure migrations to run testnet 78 migration, add more logging, fix bug with validator migration
Checklist before requesting a review
If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: