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

Add migration for new ICS param (BlocksPerEpoch) #1756

Closed
Tracked by #1516
MSalopek opened this issue Apr 3, 2024 · 0 comments · Fixed by #1757
Closed
Tracked by #1516

Add migration for new ICS param (BlocksPerEpoch) #1756

MSalopek opened this issue Apr 3, 2024 · 0 comments · Fixed by #1757
Assignees
Labels
scope: provider Issues related to the provider chain source: devnet To indicate an issue surfaced in a devnet. type: bug Issues that need priority attention -- something isn't working

Comments

@MSalopek
Copy link
Contributor

MSalopek commented Apr 3, 2024

A bug was found during gaia upgrade tests where the new gaia node would crash during upgrade because provider parameters were not correctly configured.

On consensus version 3 (ICS@v4) BlocksPerEpoch parameter was added, but it was not added to the migration handler. This caused chains migrating from consensus version 2 to panic because GetBlocksPerEpoch attempts to access state that is invalid.

The error in question:

8:33PM INF migrating module provider from version 2 to version 3 module=server
panic: UnmarshalJSON cannot decode empty bytes

goroutine 8 [running]:
github.com/cosmos/cosmos-sdk/x/params/types.Subspace.Get({{_, _}, _, {_, _}, {_, _}, {_, _, _}, ...}, ...)
	github.com/cosmos/cosmos-sdk@v0.47.11-ics-lsm/x/params/types/subspace.go:110 +0x20c
github.com/cosmos/interchain-security/v4/x/ccv/provider/keeper.Keeper.GetBlocksPerEpoch(...)
	github.com/cosmos/interchain-security/v4@v4.1.0/x/ccv/provider/keeper/params.go:84
github.com/cosmos/interchain-security/v4/x/ccv/provider/keeper.Keeper.GetParams({{0x105b55728, 0x14000e32fb0}, {0x105b8b270, 0x140002abdd0}, {{0x105b8b270, 0x140002abdd0}, 0x14000137200, {0x105b55728, 0x14000e32ef0}, {0x105b557a0, ...}, ...}, ...}, ...)
	github.com/cosmos/interchain-security/v4@v4.1.0/x/ccv/provider/keeper/params.go:99 +0xa0c
github.com/cosmos/gaia/v16/app/upgrades/v16.CreateUpgradeHandler.func1({{0x105b76420, 0x107286460}, {0x105b8bda0, 0x14001077000}, {{0xb, 0x0}, {0x140015a9960, 0xb}, 0xf, {0xd3dd4e0, ...}, ...}, ...}, ...)
	github.com/cosmos/gaia/v16/app/upgrades/v16/upgrades.go:31 +0x244
github.com/cosmos/cosmos-sdk/x/upgrade/keeper.Keeper.ApplyUpgrade({{_, _}, _, {_, _}, {_, _}, _, {_, _}, ...}, ...)
	github.com/cosmos/cosmos-sdk@v0.47.11-ics-lsm/x/upgrade/keeper/keeper.go:359 +0x12c
github.com/cosmos/cosmos-sdk/x/upgrade.BeginBlocker(_, {{0x105b76420, 0x107286460}, {0x105b8bda0, 0x14001077000}, {{0xb, 0x0}, {0x140015a9960, 0xb}, 0xf, ...}, ...}, ...)

Here is the function in question:

// GetBlocksPerEpoch returns the number of blocks that constitute an epoch
func (k Keeper) GetBlocksPerEpoch(ctx sdk.Context) int64 {
	var b int64
       // problematic call - Get returns empty bytes that cannot be unmarshalled into int64
	k.paramSpace.Get(ctx, types.KeyBlocksPerEpoch, &b) 
	return b
}
@MSalopek MSalopek self-assigned this Apr 3, 2024
@MSalopek MSalopek added A:backport/v4.1.x type: bug Issues that need priority attention -- something isn't working scope: provider Issues related to the provider chain source: devnet To indicate an issue surfaced in a devnet. and removed needs-triage labels Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: provider Issues related to the provider chain source: devnet To indicate an issue surfaced in a devnet. type: bug Issues that need priority attention -- something isn't working
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

1 participant