-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
RPC requests during node startup trigger error #14620
Comments
To me it makes sense that it returns an error. Height 0 isn't a thing, and this error looks easily handleable on the client's side (I might be very wrong tho). |
Yes, it can be handled on the client side, but my focus is mostly on how the request is being handled on the node (server) side. It's odd that the node would throw an error if "height 0 is not a thing". I would assume that the node would reject the call until the setup has been complete. It feels like the node is opening up RPC endpoints before the node is fully ready, which doesn't quite feel right. IMO, it's better for the node to be defensive and gracefully handle these requests before the node is ready to serve these requests. Let me know your thoughts! |
Which RPC endpoint are you referring to exactly? There's a bunch of moving pieces (Tendermint has plenty) and then the app itself, so it's not that straightforward. |
We ran into errors when making gRPC query requests. I can't share the exact gRPC query method (given that our codebase is private for now), but it's essentially any of the module specific query methods like this one |
And you made these queries right as the app started up? |
I tried making the chain run using sdk v0.46.4. And got a similar error but at a different height. It occurs when I try to query the chain. |
@arhamchordia was this on an upgraded binary? |
@alexanderbez I tried it on simd and there was no such error. |
No, when you see that error it's a function of your pruning settings and if the binary was upgraded or not. So did you upgrade the network and what are your pruning settings? |
I tried starting it up from scratch with a fresh genesis. Also the pruning settings are set to "default". After attempting to query multiple time. It ends up with an error and chain halts: |
We are seeing the same thing on Juno after bumping up to SDK 45.12 in our e2e. I noticed this as well before we bumped tendermint to the 35.25 fix too. https://github.com/CosmosContracts/juno/actions/runs/4106733477/jobs/7085521033#step:9:890 Additional Context:
failed to load state at height 69; version mismatch on immutable IAVL tree; version does not exist. Version has either been pruned, or is for a future block height (latest height: 69) I have replicated this locally without any custom modules too. So feel its something with the patch used to fix this issue |
Yeah I recommend that we re-open this. Something is off. |
@Reecepbcups what are the pruning settings in your instance? Are heights 70 and 71 queryable? What about 68? |
@alexanderbez pruning = nothing, single and multi node setups Other blocks around it are querying just fine (after I start the node back, since panic = exit) I patched it in my fork like so (how it was in 45.11) and it works just fine. My theories:
Looking into more this week |
This seems more probable to me. In fact, after some quick digging it is the case ->
Now that it errors,
I'm a bit suspect on this, I doubt it. |
ref: #13355 So it seems this was intentional. To return nil and error, meaning the version does not exist. So why is it the case that on a height that pre-upgrade it does not exist? |
@alexanderbez So we just had like 5 people look through this through the night Maybe on IAVL error should mention version AND dump the diff of root keys before and after the upgrade? not sure if this is possible but will remove so much headache in the future |
You mean you forgot to set the |
@Reecepbcups could you please elaborate a bit on the changes you made to fix that issue. We faced the similar. app.UpgradeKeeper.SetUpgradeHandler("upgrade", func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
return app.mm.RunMigrations(ctx, app.configurator, fromVM)
}) Error
|
@dzmitryhil You have to add the storeKeys for those modules as well. This is an extra thing on top of the upgrade handler https://github.com/CosmosContracts/juno/blob/main/app/upgrades/v13/constants.go#L22 (This will more than likely be in your app.go, we have a different format to make upgrades cleaner) In our case, we missed icacontrollertypes.StoreKey, since it was already on mainnet from a previous release, but not on our reset testnet |
Thanks a lot @Reecepbcups, it helped. |
I am using cosmos sdk v0.46.13 and when I am trying to send tx to tendermint, I got follow messages.
is this related to my hardhat task script or my tendermint side? how to fix this? |
On command
My app is built on top of Cosmos SDK v0.46.7. However, the SDK itself is not an issue. Try to find indirect dependency in your
Hope it saves you some extra hours of debug 🙌 |
Summary of Bug
RPC requests to nodes when a node is just getting initialized/started trigger the following error:
The error is being thrown here when creating the context for the query.
It seems that the IAVL tree does not exist at height 0, so the height hasn't been created yet (here is the VersionExists method on the IAVL struct).
Cosmos SDK version previously returned an empty IAVL tree in this scenario. PR that introduced the change is #13355.
This might be related to this other issue.
Version
v0.47.0-alpha2
Steps to Reproduce
The text was updated successfully, but these errors were encountered: