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

Reboot SwingSet on bulldozer upgrade #7684

Merged
merged 1 commit into from
May 11, 2023

Conversation

michaelfig
Copy link
Member

@michaelfig michaelfig commented May 10, 2023

refs: #6644

Description

When upgrading to agoric-upgrade-10, SwingSet will be reinitialized. This PR adds that initialization to the upgrade handler so that it completes during upgrade before any new blocks are produced.

Security Considerations

Improves security:

  • no user transactions can interpose until the upgrade is complete
  • the chain will halt if there are errors during that upgrade, without committing to potentially corrupted state.

Scaling Considerations

Avoids costly upgrade work impacting blocks beyond the upgrade block itself.

Documentation Considerations

Testing Considerations

@michaelfig michaelfig added the agd Agoric (Golang) Daemon label May 10, 2023
@michaelfig michaelfig self-assigned this May 10, 2023
@dckc
Copy link
Member

dckc commented May 10, 2023

suggsest: refs #6644

Copy link
Contributor

@JimLarson JimLarson left a comment

Choose a reason for hiding this comment

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

LGTM with the changes below - except for the Dockerfile changes which I can't really speak to.

@@ -794,7 +794,21 @@ func upgrade10Handler(app *GaiaApp, targetUpgrade string) func(sdk.Context, upgr
app.VstorageKeeper.MigrateNoDataPlaceholders(ctx) // upgrade-10 only
normalizeProvisionAccount(ctx, app.AccountKeeper)

return app.mm.RunMigrations(ctx, app.configurator, fromVm)
mvm, err := app.mm.RunMigrations(ctx, app.configurator, fromVm)
Copy link
Contributor

Choose a reason for hiding this comment

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

Check if err is non-nil here, and if so return. We don't want to proceed with swingset initialization in this case.

if err2 != nil {
// NOTE: A failed BOOTSTRAP_BLOCK means that the SwingSet state is inconsistent.
// Panic here, in the hopes that a replay from scratch will fix the problem.
panic(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be err2, but if you return on error right after RunMigrations() above, you can just reuse err and this line will be correct.

@michaelfig michaelfig force-pushed the mfig-bulldozer-reboot-swingset branch from 1704044 to 3bb75e3 Compare May 11, 2023 00:09
@michaelfig michaelfig enabled auto-merge May 11, 2023 00:11
@michaelfig michaelfig added this pull request to the merge queue May 11, 2023
Merged via the queue into master with commit 2d2297f May 11, 2023
@michaelfig michaelfig deleted the mfig-bulldozer-reboot-swingset branch May 11, 2023 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agd Agoric (Golang) Daemon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants