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

baseapp: Init contradictorily checks if app.cms == nil after invoking app.setState which blindly dereferences app.cms + app.Seal which will cause nil pointer panics or prevent any progress since sealed=true #18726

Closed
odeke-em opened this issue Dec 13, 2023 · 0 comments · Fixed by #18727
Labels

Comments

@odeke-em
Copy link
Collaborator

If we examine this code

// needed for the export command which inits from store but never calls initchain
app.setState(execModeCheck, emptyHeader)
app.Seal()
if app.cms == nil {
return errors.New("commit multi-store must not be nil")
}
return app.cms.GetPruning().Validate()
we can see that app.setState is invoked and its code is
func (app *BaseApp) setState(mode execMode, header cmtproto.Header) {
and it nakedly invokes app.cms
image

But later down we have a check for app.cms == nil

if app.cms == nil {
return errors.New("commit multi-store must not be nil")
}

image

That's a contradiction right there and would result in a nil panic, even more by invoking app.Seal() firstly we make it so that any failure thats fixed by now setting a cms then that's rechecked then .Init() is retried will just fail with no meaningful progress. We should move that check up before setState

@odeke-em odeke-em added the T:Bug label Dec 13, 2023
odeke-em added a commit that referenced this issue Dec 13, 2023
This change ensures that we correctly check if the app has a nil
commit multistore before trying to dereference it or invoking .Seal()

Fixes #18726
odeke-em added a commit that referenced this issue Dec 13, 2023
This change ensures that we correctly check if the app has a nil
commit multistore before trying to dereference it or invoking .Seal()

Fixes #18726
odeke-em added a commit that referenced this issue Dec 13, 2023
This change ensures that we correctly check if the app has a nil
commit multistore before trying to dereference it or invoking .Seal()

Fixes #18726
odeke-em added a commit that referenced this issue Dec 13, 2023
This change ensures that we correctly check if the app has a nil
commit multistore before trying to dereference it or invoking .Seal()

Fixes #18726
odeke-em added a commit that referenced this issue Dec 13, 2023
This change ensures that we correctly check if the app has a nil
commit multistore before trying to dereference it or invoking .Seal()

Fixes #18726
odeke-em added a commit that referenced this issue Dec 13, 2023
This change ensures that we correctly check if the app has a nil
commit multistore before trying to dereference it or invoking .Seal()

Fixes #18726
odeke-em added a commit that referenced this issue Dec 13, 2023
This change ensures that we correctly check if the app has a nil
commit multistore before trying to dereference it or invoking .Seal()

Fixes #18726
odeke-em added a commit that referenced this issue Dec 13, 2023
This change ensures that we correctly check if the app has a nil
commit multistore before trying to dereference it or invoking .Seal()

Fixes #18726
odeke-em added a commit that referenced this issue Dec 13, 2023
This change ensures that we correctly check if the app has a nil
commit multistore before trying to dereference it or invoking .Seal()

Fixes #18726
odeke-em added a commit that referenced this issue Dec 13, 2023
This change ensures that we correctly check if the app has a nil
commit multistore before trying to dereference it or invoking .Seal()

Fixes #18726
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant