From 21c803bb323c7adce1de132c0a86162c2987066b Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Wed, 12 Jun 2024 16:25:25 -0400 Subject: [PATCH 01/22] feat!: add a delay between quorum and upgrade height --- app/app.go | 2 +- x/signal/integration_test.go | 10 +++++++++- x/signal/keeper.go | 31 +++++++++++++++++++++++-------- x/signal/keeper_test.go | 14 ++++++++++++-- 4 files changed, 45 insertions(+), 12 deletions(-) diff --git a/app/app.go b/app/app.go index c6e4f15373..b61e1cdd53 100644 --- a/app/app.go +++ b/app/app.go @@ -464,7 +464,7 @@ func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo } } // from v2 to v3 and onwards we use a signalling mechanism - } else if shouldUpgrade, newVersion := app.SignalKeeper.ShouldUpgrade(); shouldUpgrade { + } else if shouldUpgrade, newVersion := app.SignalKeeper.ShouldUpgrade(ctx); shouldUpgrade { // Version changes must be increasing. Downgrades are not permitted if newVersion > currentVersion { app.SetAppVersion(ctx, newVersion) diff --git a/x/signal/integration_test.go b/x/signal/integration_test.go index 17b3e3f751..1e38344efe 100644 --- a/x/signal/integration_test.go +++ b/x/signal/integration_test.go @@ -54,7 +54,15 @@ func TestUpgradeIntegration(t *testing.T) { _, err = app.SignalKeeper.TryUpgrade(ctx, nil) require.NoError(t, err) - shouldUpgrade, version := app.SignalKeeper.ShouldUpgrade() + shouldUpgrade, version := app.SignalKeeper.ShouldUpgrade(ctx) + require.False(t, shouldUpgrade) + require.EqualValues(t, 0, version) + + // advance the block height by 48 hours worth of 12 second blocks. + upgradeHeightDelay := int64(48 * 60 * 60 / 12) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + upgradeHeightDelay) + + shouldUpgrade, version = app.SignalKeeper.ShouldUpgrade(ctx) require.True(t, shouldUpgrade) require.EqualValues(t, 2, version) } diff --git a/x/signal/keeper.go b/x/signal/keeper.go index 6138b3d999..782a850867 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -18,6 +18,11 @@ var ( // defaultSignalThreshold is 5/6 or approximately 83.33% defaultSignalThreshold = sdk.NewDec(5).Quo(sdk.NewDec(6)) + + // defaultUpgradeHeightDelay is the number of blocks after a quorum has been + // reached that the chain should upgrade to the new version. Assuming a + // block interval of 12 seconds, this is 48 hours. + defaultUpgradeHeightDelay = int64(48 * 60 * 60 / 12) // 48 hours * 60 minutes * 60 seconds / 12 seconds per block = 14,400 blocks. ) // Threshold is the fraction of voting power that is required @@ -35,10 +40,13 @@ type Keeper struct { storeKey storetypes.StoreKey // quorumVersion is the version that has received a quorum of validators - // to signal for it. This variable is relevant just for the scope of the - // lifetime of the block. + // to signal for it. It is set to 0 if no version has reached quorum. quorumVersion uint64 + // upgradeHeight is the height at which the network should upgrade to the + // quorumVersion. It is set to 0 if no version has reached quorum. + upgradeHeight int64 + // stakingKeeper is used to fetch validators to calculate the total power // signalled to a version. stakingKeeper StakingKeeper @@ -89,6 +97,7 @@ func (k *Keeper) TryUpgrade(ctx context.Context, _ *types.MsgTryUpgrade) (*types hasQuorum, version := k.TallyVotingPower(sdkCtx, threshold.Int64()) if hasQuorum { k.quorumVersion = version + k.upgradeHeight = sdkCtx.BlockHeight() + defaultUpgradeHeightDelay } return &types.MsgTryUpgradeResponse{}, nil } @@ -172,15 +181,20 @@ func (k Keeper) GetVotingPowerThreshold(ctx sdk.Context) sdkmath.Int { return thresholdFraction.MulInt(totalVotingPower).Ceil().TruncateInt() } -// ShouldUpgrade returns true if the signalling mechanism has concluded -// that the network is ready to upgrade. It also returns the version -// that the node should upgrade to. -func (k *Keeper) ShouldUpgrade() (bool, uint64) { - return k.quorumVersion != 0, k.quorumVersion +// ShouldUpgrade returns whether the signalling mechanism has concluded that the +// network is ready to upgrade and the version to upgrade to. It returns false +// and 0 if no version has reached quorum. +func (k *Keeper) ShouldUpgrade(ctx sdk.Context) (shouldUpgrade bool, version uint64) { + shouldUpgrade = k.quorumVersion != 0 && ctx.BlockHeight() >= k.upgradeHeight + if shouldUpgrade { + return shouldUpgrade, k.quorumVersion + } + return false, 0 } // ResetTally resets the tally after a version change. It iterates over the -// store and deletes all versions. It also resets the quorumVersion to 0. +// store and deletes all versions. It also resets the quorumVersion and +// upgradeHeight to 0. func (k *Keeper) ResetTally(ctx sdk.Context) { store := ctx.KVStore(k.storeKey) iterator := store.Iterator(nil, nil) @@ -189,6 +203,7 @@ func (k *Keeper) ResetTally(ctx sdk.Context) { store.Delete(iterator.Key()) } k.quorumVersion = 0 + k.upgradeHeight = 0 } func VersionToBytes(version uint64) []byte { diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index bf2693f09f..7eadbca2fe 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -158,7 +158,7 @@ func TestTallyingLogic(t *testing.T) { _, err = upgradeKeeper.TryUpgrade(goCtx, &types.MsgTryUpgrade{}) require.NoError(t, err) - shouldUpgrade, version := upgradeKeeper.ShouldUpgrade() + shouldUpgrade, version := upgradeKeeper.ShouldUpgrade(ctx) require.False(t, shouldUpgrade) require.Equal(t, uint64(0), version) @@ -171,9 +171,19 @@ func TestTallyingLogic(t *testing.T) { _, err = upgradeKeeper.TryUpgrade(goCtx, &types.MsgTryUpgrade{}) require.NoError(t, err) - shouldUpgrade, version = upgradeKeeper.ShouldUpgrade() + + shouldUpgrade, version = upgradeKeeper.ShouldUpgrade(ctx) + require.False(t, shouldUpgrade) + require.Equal(t, uint64(0), version) + + // advance the block height by 48 hours worth of 12 second blocks. + upgradeHeightDelay := int64(48 * 60 * 60 / 12) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + upgradeHeightDelay) + + shouldUpgrade, version = upgradeKeeper.ShouldUpgrade(ctx) require.True(t, shouldUpgrade) require.Equal(t, uint64(2), version) + // update the version to 2 ctx = ctx.WithBlockHeader(tmproto.Header{ Version: tmversion.Consensus{ From c78f98c214c5d0cbf334efcd87e952d5dcf746ba Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Wed, 12 Jun 2024 16:37:22 -0400 Subject: [PATCH 02/22] feat: reject signal version and try upgrade during upgrade pending --- x/signal/keeper.go | 15 +++++++++++++++ x/signal/types/errors.go | 5 ++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/x/signal/keeper.go b/x/signal/keeper.go index 782a850867..1b601b1f4a 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -65,6 +65,10 @@ func NewKeeper( // SignalVersion is a method required by the MsgServer interface. func (k Keeper) SignalVersion(ctx context.Context, req *types.MsgSignalVersion) (*types.MsgSignalVersionResponse, error) { + if k.isUpgradePending() { + return &types.MsgSignalVersionResponse{}, types.ErrUpgradePending.Wrapf("can not signal version") + } + sdkCtx := sdk.UnwrapSDKContext(ctx) valAddr, err := sdk.ValAddressFromBech32(req.ValidatorAddress) if err != nil { @@ -92,6 +96,10 @@ func (k Keeper) SignalVersion(ctx context.Context, req *types.MsgSignalVersion) // If one version has quorum, it is set as the quorum version // which the application can use as signal to upgrade to that version. func (k *Keeper) TryUpgrade(ctx context.Context, _ *types.MsgTryUpgrade) (*types.MsgTryUpgradeResponse, error) { + if k.isUpgradePending() { + return &types.MsgTryUpgradeResponse{}, types.ErrUpgradePending.Wrapf("can not try upgrade") + } + sdkCtx := sdk.UnwrapSDKContext(ctx) threshold := k.GetVotingPowerThreshold(sdkCtx) hasQuorum, version := k.TallyVotingPower(sdkCtx, threshold.Int64()) @@ -213,3 +221,10 @@ func VersionToBytes(version uint64) []byte { func VersionFromBytes(version []byte) uint64 { return binary.BigEndian.Uint64(version) } + +// isUpgradePending returns true if a version has reached quorum and the chain +// will upgrade to quorumVersion at upgradeHeight. While the keeper has an +// upgrade pending actions like SignalVersion and TryUpgrade will be rejected. +func (k *Keeper) isUpgradePending() bool { + return k.quorumVersion != 0 && k.upgradeHeight != 0 +} diff --git a/x/signal/types/errors.go b/x/signal/types/errors.go index 1ff6064d9b..efcf2a4282 100644 --- a/x/signal/types/errors.go +++ b/x/signal/types/errors.go @@ -4,4 +4,7 @@ import ( "cosmossdk.io/errors" ) -var ErrInvalidVersion = errors.Register(ModuleName, 1, "signalled version must be either the current version or one greater") +var ( + ErrInvalidVersion = errors.Register(ModuleName, 1, "signalled version must be either the current version or one greater") + ErrUpgradePending = errors.Register(ModuleName, 2, "upgrade is already pending") +) From c270c424020b146168a10361c190eae4b073dab0 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Wed, 12 Jun 2024 16:40:19 -0400 Subject: [PATCH 03/22] test: that error is returned for upgrade pending --- x/signal/integration_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/x/signal/integration_test.go b/x/signal/integration_test.go index 1e38344efe..1f48f6c22d 100644 --- a/x/signal/integration_test.go +++ b/x/signal/integration_test.go @@ -54,6 +54,21 @@ func TestUpgradeIntegration(t *testing.T) { _, err = app.SignalKeeper.TryUpgrade(ctx, nil) require.NoError(t, err) + // Verify that if a subsequent call to TryUpgrade is made, it returns an + // error because an upgrade is already pending. + _, err = app.SignalKeeper.TryUpgrade(ctx, nil) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrUpgradePending) + + // Verify that if a validator tries to change their signal version, it + // returns an error because an upgrade is pending. + _, err = app.SignalKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ + ValidatorAddress: valAddr.String(), + Version: 3, + }) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrUpgradePending) + shouldUpgrade, version := app.SignalKeeper.ShouldUpgrade(ctx) require.False(t, shouldUpgrade) require.EqualValues(t, 0, version) From 9a2dda8eb9506d0dcd809dc933e3e954a056d849 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Thu, 13 Jun 2024 15:58:24 -0400 Subject: [PATCH 04/22] chore: increase upgrade height delay to 1 week --- x/signal/integration_test.go | 4 +--- x/signal/keeper.go | 2 +- x/signal/keeper_test.go | 6 +++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/x/signal/integration_test.go b/x/signal/integration_test.go index 1f48f6c22d..47cd434b47 100644 --- a/x/signal/integration_test.go +++ b/x/signal/integration_test.go @@ -73,9 +73,7 @@ func TestUpgradeIntegration(t *testing.T) { require.False(t, shouldUpgrade) require.EqualValues(t, 0, version) - // advance the block height by 48 hours worth of 12 second blocks. - upgradeHeightDelay := int64(48 * 60 * 60 / 12) - ctx = ctx.WithBlockHeight(ctx.BlockHeight() + upgradeHeightDelay) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + defaultUpgradeHeightDelay) shouldUpgrade, version = app.SignalKeeper.ShouldUpgrade(ctx) require.True(t, shouldUpgrade) diff --git a/x/signal/keeper.go b/x/signal/keeper.go index 1b601b1f4a..cb9ba8e1b5 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -22,7 +22,7 @@ var ( // defaultUpgradeHeightDelay is the number of blocks after a quorum has been // reached that the chain should upgrade to the new version. Assuming a // block interval of 12 seconds, this is 48 hours. - defaultUpgradeHeightDelay = int64(48 * 60 * 60 / 12) // 48 hours * 60 minutes * 60 seconds / 12 seconds per block = 14,400 blocks. + defaultUpgradeHeightDelay = int64(7 * 24 * 60 * 60 / 12) // 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 50,400 blocks. ) // Threshold is the fraction of voting power that is required diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index 7eadbca2fe..ada4fec6d8 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -24,6 +24,8 @@ import ( tmdb "github.com/tendermint/tm-db" ) +const defaultUpgradeHeightDelay = int64(7 * 24 * 60 * 60 / 12) // 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 50,400 blocks. + func TestGetVotingPowerThreshold(t *testing.T) { bigInt := big.NewInt(0) bigInt.SetString("23058430092136939509", 10) @@ -176,9 +178,7 @@ func TestTallyingLogic(t *testing.T) { require.False(t, shouldUpgrade) require.Equal(t, uint64(0), version) - // advance the block height by 48 hours worth of 12 second blocks. - upgradeHeightDelay := int64(48 * 60 * 60 / 12) - ctx = ctx.WithBlockHeight(ctx.BlockHeight() + upgradeHeightDelay) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + defaultUpgradeHeightDelay) shouldUpgrade, version = upgradeKeeper.ShouldUpgrade(ctx) require.True(t, shouldUpgrade) From cf3061463ec8e349c8fd833fa2402c825414f9f4 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Thu, 13 Jun 2024 16:29:57 -0400 Subject: [PATCH 05/22] test: TryUpgrade --- x/signal/keeper.go | 15 ++++++---- x/signal/keeper_test.go | 59 ++++++++++++++++++++++++++++++++++++++-- x/signal/types/errors.go | 5 ++-- 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/x/signal/keeper.go b/x/signal/keeper.go index cb9ba8e1b5..339c6098f7 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -103,7 +103,10 @@ func (k *Keeper) TryUpgrade(ctx context.Context, _ *types.MsgTryUpgrade) (*types sdkCtx := sdk.UnwrapSDKContext(ctx) threshold := k.GetVotingPowerThreshold(sdkCtx) hasQuorum, version := k.TallyVotingPower(sdkCtx, threshold.Int64()) - if hasQuorum { + if hasQuorum && version <= sdkCtx.BlockHeader().Version.App { + return &types.MsgTryUpgradeResponse{}, types.ErrInvalidUpgradeVersion.Wrapf("can not upgrade to version %v because it is less than or equal to current version %v", version, sdkCtx.BlockHeader().Version.App) + } + if hasQuorum && version > sdkCtx.BlockHeader().Version.App { k.quorumVersion = version k.upgradeHeight = sdkCtx.BlockHeight() + defaultUpgradeHeightDelay } @@ -192,10 +195,12 @@ func (k Keeper) GetVotingPowerThreshold(ctx sdk.Context) sdkmath.Int { // ShouldUpgrade returns whether the signalling mechanism has concluded that the // network is ready to upgrade and the version to upgrade to. It returns false // and 0 if no version has reached quorum. -func (k *Keeper) ShouldUpgrade(ctx sdk.Context) (shouldUpgrade bool, version uint64) { - shouldUpgrade = k.quorumVersion != 0 && ctx.BlockHeight() >= k.upgradeHeight - if shouldUpgrade { - return shouldUpgrade, k.quorumVersion +func (k *Keeper) ShouldUpgrade(ctx sdk.Context) (isQuorumVersion bool, version uint64) { + hasQuorumVersion := k.quorumVersion != 0 + hasUpgradeHeightBeenReached := ctx.BlockHeight() >= k.upgradeHeight + + if hasQuorumVersion && hasUpgradeHeightBeenReached { + return true, k.quorumVersion } return false, 0 } diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index ada4fec6d8..1383c4164c 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -175,15 +175,17 @@ func TestTallyingLogic(t *testing.T) { require.NoError(t, err) shouldUpgrade, version = upgradeKeeper.ShouldUpgrade(ctx) - require.False(t, shouldUpgrade) + require.False(t, shouldUpgrade) // should be false because upgrade height hasn't been reached. require.Equal(t, uint64(0), version) ctx = ctx.WithBlockHeight(ctx.BlockHeight() + defaultUpgradeHeightDelay) shouldUpgrade, version = upgradeKeeper.ShouldUpgrade(ctx) - require.True(t, shouldUpgrade) + require.True(t, shouldUpgrade) // should be true because upgrade height has been reached. require.Equal(t, uint64(2), version) + upgradeKeeper.ResetTally(ctx) + // update the version to 2 ctx = ctx.WithBlockHeader(tmproto.Header{ Version: tmversion.Consensus{ @@ -198,6 +200,16 @@ func TestTallyingLogic(t *testing.T) { Version: 3, }) require.NoError(t, err) + _, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ + ValidatorAddress: testutil.ValAddrs[1].String(), + Version: 2, + }) + require.NoError(t, err) + _, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ + ValidatorAddress: testutil.ValAddrs[2].String(), + Version: 2, + }) + require.NoError(t, err) res, err = upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ Version: 2, @@ -296,6 +308,49 @@ func TestResetTally(t *testing.T) { assert.Equal(t, uint64(0), resp.VotingPower) } +func TestTryUpgrade(t *testing.T) { + t.Run("should return an error if an upgrade is already pending", func(t *testing.T) { + upgradeKeeper, ctx, _ := setup(t) + goCtx := sdk.WrapSDKContext(ctx) + + _, err := upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[0].String(), Version: 2}) + require.NoError(t, err) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[1].String(), Version: 2}) + require.NoError(t, err) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[2].String(), Version: 2}) + require.NoError(t, err) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[3].String(), Version: 2}) + require.NoError(t, err) + + // This TryUpgrade should succeed. + _, err = upgradeKeeper.TryUpgrade(goCtx, &types.MsgTryUpgrade{}) + require.NoError(t, err) + + // This TryUpgrade should fail because an upgrade is pending. + _, err = upgradeKeeper.TryUpgrade(goCtx, &types.MsgTryUpgrade{}) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrUpgradePending) + }) + + t.Run("should return an error if quorum version is less than or equal to the current version", func(t *testing.T) { + upgradeKeeper, ctx, _ := setup(t) + goCtx := sdk.WrapSDKContext(ctx) + + _, err := upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[0].String(), Version: 1}) + require.NoError(t, err) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[1].String(), Version: 1}) + require.NoError(t, err) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[2].String(), Version: 1}) + require.NoError(t, err) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[3].String(), Version: 1}) + require.NoError(t, err) + + _, err = upgradeKeeper.TryUpgrade(goCtx, &types.MsgTryUpgrade{}) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrInvalidUpgradeVersion) + }) +} + func setup(t *testing.T) (signal.Keeper, sdk.Context, *mockStakingKeeper) { signalStore := sdk.NewKVStoreKey(types.StoreKey) db := tmdb.NewMemDB() diff --git a/x/signal/types/errors.go b/x/signal/types/errors.go index efcf2a4282..266b36b54b 100644 --- a/x/signal/types/errors.go +++ b/x/signal/types/errors.go @@ -5,6 +5,7 @@ import ( ) var ( - ErrInvalidVersion = errors.Register(ModuleName, 1, "signalled version must be either the current version or one greater") - ErrUpgradePending = errors.Register(ModuleName, 2, "upgrade is already pending") + ErrInvalidVersion = errors.Register(ModuleName, 1, "signalled version must be either the current version or one greater") + ErrUpgradePending = errors.Register(ModuleName, 2, "upgrade is already pending") + ErrInvalidUpgradeVersion = errors.Register(ModuleName, 3, "invalid upgrade version") ) From 126d095556b602973a3e86ab55565f6d28764057 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Fri, 14 Jun 2024 14:15:52 -0400 Subject: [PATCH 06/22] chore: increase upgrade height delay to 3 weeks --- x/signal/keeper.go | 2 +- x/signal/keeper_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x/signal/keeper.go b/x/signal/keeper.go index 339c6098f7..c7367c6b22 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -22,7 +22,7 @@ var ( // defaultUpgradeHeightDelay is the number of blocks after a quorum has been // reached that the chain should upgrade to the new version. Assuming a // block interval of 12 seconds, this is 48 hours. - defaultUpgradeHeightDelay = int64(7 * 24 * 60 * 60 / 12) // 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 50,400 blocks. + defaultUpgradeHeightDelay = int64(3 * 7 * 24 * 60 * 60 / 12) // 3 weeks * 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 151,200 blocks. ) // Threshold is the fraction of voting power that is required diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index 1383c4164c..415f502ab4 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -24,7 +24,7 @@ import ( tmdb "github.com/tendermint/tm-db" ) -const defaultUpgradeHeightDelay = int64(7 * 24 * 60 * 60 / 12) // 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 50,400 blocks. +const defaultUpgradeHeightDelay = int64(3 * 7 * 24 * 60 * 60 / 12) // 3 weeks * 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 151,200 blocks. func TestGetVotingPowerThreshold(t *testing.T) { bigInt := big.NewInt(0) From cd566cfcdb005f7483e0370efa54bf26ece64ad0 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Fri, 14 Jun 2024 15:02:35 -0400 Subject: [PATCH 07/22] feat!: persist upgrade in store Breaks TestResetTally I think because the current keeper iterates over all keys and values and delete them. I think we want to find a way to constrain the keys and values that validator signals are in. Or separate those signals from the key used to persist the upgrade. --- app/app.go | 2 +- app/module/configurator_test.go | 2 +- proto/celestia/signal/v1/upgrade.proto | 15 ++ x/signal/keeper.go | 82 ++++-- x/signal/keeper_test.go | 8 +- x/signal/types/keys.go | 5 + x/signal/types/upgrade.pb.go | 341 +++++++++++++++++++++++++ 7 files changed, 423 insertions(+), 32 deletions(-) create mode 100644 proto/celestia/signal/v1/upgrade.proto create mode 100644 x/signal/types/keys.go create mode 100644 x/signal/types/upgrade.pb.go diff --git a/app/app.go b/app/app.go index b61e1cdd53..b46e90b2f4 100644 --- a/app/app.go +++ b/app/app.go @@ -276,7 +276,7 @@ func New( ), ) - app.SignalKeeper = signal.NewKeeper(keys[signaltypes.StoreKey], app.StakingKeeper) + app.SignalKeeper = signal.NewKeeper(appCodec, keys[signaltypes.StoreKey], app.StakingKeeper) app.IBCKeeper = ibckeeper.NewKeeper( appCodec, diff --git a/app/module/configurator_test.go b/app/module/configurator_test.go index 605c04cc49..5788d84108 100644 --- a/app/module/configurator_test.go +++ b/app/module/configurator_test.go @@ -37,7 +37,7 @@ func TestConfigurator(t *testing.T) { stateStore.MountStoreWithDB(storeKey, storetypes.StoreTypeIAVL, db) require.NoError(t, stateStore.LoadLatestVersion()) - keeper := signal.NewKeeper(storeKey, nil) + keeper := signal.NewKeeper(config.Codec, storeKey, nil) require.NotNil(t, keeper) upgradeModule := signal.NewAppModule(keeper) manager, err := module.NewManager([]module.VersionedModule{ diff --git a/proto/celestia/signal/v1/upgrade.proto b/proto/celestia/signal/v1/upgrade.proto new file mode 100644 index 0000000000..02d3a8a27c --- /dev/null +++ b/proto/celestia/signal/v1/upgrade.proto @@ -0,0 +1,15 @@ +syntax = "proto3"; +package celestia.signal.v1; + +option go_package = "github.com/celestiaorg/celestia-app/x/signal/types"; + +// Upgrade is a type that represents a network upgrade. +message Upgrade { + // AppVersion is the version that has received a quorum of validators to + // signal for it. It is set to 0 if no version has reached quorum. + uint64 app_version = 1; + + // UpgradeHeight is the height at which the network should upgrade to the + // AppVersion. It is set to 0 if no version has reached quorum. + int64 upgrade_height = 2; +} diff --git a/x/signal/keeper.go b/x/signal/keeper.go index c7367c6b22..69f60c10b4 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -6,6 +6,7 @@ import ( sdkmath "cosmossdk.io/math" "github.com/celestiaorg/celestia-app/v2/x/signal/types" + "github.com/cosmos/cosmos-sdk/codec" storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" @@ -35,18 +36,13 @@ func Threshold(_ uint64) sdk.Dec { } type Keeper struct { + // binaryCodec is used to marshal and unmarshal data from the store. + binaryCodec codec.BinaryCodec + // storeKey is key that is used to fetch the signal store from the multi // store. storeKey storetypes.StoreKey - // quorumVersion is the version that has received a quorum of validators - // to signal for it. It is set to 0 if no version has reached quorum. - quorumVersion uint64 - - // upgradeHeight is the height at which the network should upgrade to the - // quorumVersion. It is set to 0 if no version has reached quorum. - upgradeHeight int64 - // stakingKeeper is used to fetch validators to calculate the total power // signalled to a version. stakingKeeper StakingKeeper @@ -54,10 +50,12 @@ type Keeper struct { // NewKeeper returns a signal keeper. func NewKeeper( + binaryCodec codec.BinaryCodec, storeKey storetypes.StoreKey, stakingKeeper StakingKeeper, ) Keeper { return Keeper{ + binaryCodec: binaryCodec, storeKey: storeKey, stakingKeeper: stakingKeeper, } @@ -65,11 +63,12 @@ func NewKeeper( // SignalVersion is a method required by the MsgServer interface. func (k Keeper) SignalVersion(ctx context.Context, req *types.MsgSignalVersion) (*types.MsgSignalVersionResponse, error) { - if k.isUpgradePending() { + sdkCtx := sdk.UnwrapSDKContext(ctx) + + if k.isUpgradePending(sdkCtx) { return &types.MsgSignalVersionResponse{}, types.ErrUpgradePending.Wrapf("can not signal version") } - sdkCtx := sdk.UnwrapSDKContext(ctx) valAddr, err := sdk.ValAddressFromBech32(req.ValidatorAddress) if err != nil { return nil, err @@ -96,19 +95,23 @@ func (k Keeper) SignalVersion(ctx context.Context, req *types.MsgSignalVersion) // If one version has quorum, it is set as the quorum version // which the application can use as signal to upgrade to that version. func (k *Keeper) TryUpgrade(ctx context.Context, _ *types.MsgTryUpgrade) (*types.MsgTryUpgradeResponse, error) { - if k.isUpgradePending() { + sdkCtx := sdk.UnwrapSDKContext(ctx) + + if k.isUpgradePending(sdkCtx) { return &types.MsgTryUpgradeResponse{}, types.ErrUpgradePending.Wrapf("can not try upgrade") } - sdkCtx := sdk.UnwrapSDKContext(ctx) threshold := k.GetVotingPowerThreshold(sdkCtx) hasQuorum, version := k.TallyVotingPower(sdkCtx, threshold.Int64()) - if hasQuorum && version <= sdkCtx.BlockHeader().Version.App { - return &types.MsgTryUpgradeResponse{}, types.ErrInvalidUpgradeVersion.Wrapf("can not upgrade to version %v because it is less than or equal to current version %v", version, sdkCtx.BlockHeader().Version.App) - } - if hasQuorum && version > sdkCtx.BlockHeader().Version.App { - k.quorumVersion = version - k.upgradeHeight = sdkCtx.BlockHeight() + defaultUpgradeHeightDelay + if hasQuorum { + if version <= sdkCtx.BlockHeader().Version.App { + return &types.MsgTryUpgradeResponse{}, types.ErrInvalidUpgradeVersion.Wrapf("can not upgrade to version %v because it is less than or equal to current version %v", version, sdkCtx.BlockHeader().Version.App) + } + upgrade := types.Upgrade{ + AppVersion: version, + UpgradeHeight: sdkCtx.BlockHeader().Height + defaultUpgradeHeightDelay, + } + k.setUpgrade(sdkCtx, upgrade) } return &types.MsgTryUpgradeResponse{}, nil } @@ -196,11 +199,14 @@ func (k Keeper) GetVotingPowerThreshold(ctx sdk.Context) sdkmath.Int { // network is ready to upgrade and the version to upgrade to. It returns false // and 0 if no version has reached quorum. func (k *Keeper) ShouldUpgrade(ctx sdk.Context) (isQuorumVersion bool, version uint64) { - hasQuorumVersion := k.quorumVersion != 0 - hasUpgradeHeightBeenReached := ctx.BlockHeight() >= k.upgradeHeight + upgrade, ok := k.getUpgrade(ctx) + if !ok { + return false, 0 + } - if hasQuorumVersion && hasUpgradeHeightBeenReached { - return true, k.quorumVersion + hasUpgradeHeightBeenReached := ctx.BlockHeight() >= upgrade.UpgradeHeight + if hasUpgradeHeightBeenReached { + return true, upgrade.AppVersion } return false, 0 } @@ -215,8 +221,7 @@ func (k *Keeper) ResetTally(ctx sdk.Context) { for ; iterator.Valid(); iterator.Next() { store.Delete(iterator.Key()) } - k.quorumVersion = 0 - k.upgradeHeight = 0 + k.setUpgrade(ctx, types.Upgrade{}) } func VersionToBytes(version uint64) []byte { @@ -228,8 +233,29 @@ func VersionFromBytes(version []byte) uint64 { } // isUpgradePending returns true if a version has reached quorum and the chain -// will upgrade to quorumVersion at upgradeHeight. While the keeper has an -// upgrade pending actions like SignalVersion and TryUpgrade will be rejected. -func (k *Keeper) isUpgradePending() bool { - return k.quorumVersion != 0 && k.upgradeHeight != 0 +// will upgrade to the upgrade app version at the upgrade height. While the +// keeper has an upgrade pending actions like SignalVersion and TryUpgrade will +// be rejected. +func (k *Keeper) isUpgradePending(ctx sdk.Context) bool { + _, ok := k.getUpgrade(ctx) + return ok +} + +// getUpgrade returns the upgrade from the store if it exists along with a +// boolean indicating its existence. +func (k *Keeper) getUpgrade(ctx sdk.Context) (upgrade types.Upgrade, ok bool) { + store := ctx.KVStore(k.storeKey) + bz := store.Get(types.UpgradeKey) + if bz == nil { + return types.Upgrade{}, false + } + k.binaryCodec.MustUnmarshal(bz, &upgrade) + return upgrade, true +} + +// setUpgrade sets the upgrade in the store. +func (k *Keeper) setUpgrade(ctx sdk.Context, upgrade types.Upgrade) { + store := ctx.KVStore(k.storeKey) + b := k.binaryCodec.MustMarshal(&upgrade) + store.Set(types.UpgradeKey, b) } diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index 415f502ab4..8d2d7f7304 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -10,6 +10,8 @@ import ( "github.com/cosmos/cosmos-sdk/store" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/celestiaorg/celestia-app/v2/app" + "github.com/celestiaorg/celestia-app/v2/app/encoding" "github.com/celestiaorg/celestia-app/v2/x/signal" "github.com/celestiaorg/celestia-app/v2/x/signal/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" @@ -64,8 +66,9 @@ func TestGetVotingPowerThreshold(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + config := encoding.MakeConfig(app.ModuleEncodingRegisters...) stakingKeeper := newMockStakingKeeper(tc.validators) - k := signal.NewKeeper(nil, stakingKeeper) + k := signal.NewKeeper(config.Codec, nil, stakingKeeper) got := k.GetVotingPowerThreshold(sdk.Context{}) assert.Equal(t, tc.want, got, fmt.Sprintf("want %v, got %v", tc.want.String(), got.String())) }) @@ -372,7 +375,8 @@ func setup(t *testing.T) (signal.Keeper, sdk.Context, *mockStakingKeeper) { }, ) - upgradeKeeper := signal.NewKeeper(signalStore, mockStakingKeeper) + config := encoding.MakeConfig(app.ModuleEncodingRegisters...) + upgradeKeeper := signal.NewKeeper(config.Codec, signalStore, mockStakingKeeper) return upgradeKeeper, mockCtx, mockStakingKeeper } diff --git a/x/signal/types/keys.go b/x/signal/types/keys.go new file mode 100644 index 0000000000..19f8f852e3 --- /dev/null +++ b/x/signal/types/keys.go @@ -0,0 +1,5 @@ +package types + +// UpgradeKey is the key in the signal store used to persist a upgrade if one is +// pending. +var UpgradeKey = []byte{0x00} diff --git a/x/signal/types/upgrade.pb.go b/x/signal/types/upgrade.pb.go new file mode 100644 index 0000000000..24aaff5a44 --- /dev/null +++ b/x/signal/types/upgrade.pb.go @@ -0,0 +1,341 @@ +// Code generated by protoc-gen-gogo. DO NOT EDIT. +// source: celestia/signal/v1/upgrade.proto + +package types + +import ( + fmt "fmt" + proto "github.com/gogo/protobuf/proto" + io "io" + math "math" + math_bits "math/bits" +) + +// Reference imports to suppress errors if they are not otherwise used. +var _ = proto.Marshal +var _ = fmt.Errorf +var _ = math.Inf + +// This is a compile-time assertion to ensure that this generated file +// is compatible with the proto package it is being compiled against. +// A compilation error at this line likely means your copy of the +// proto package needs to be updated. +const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package + +// Upgrade is a type that represents a network upgrade. +type Upgrade struct { + // AppVersion is the version that has received a quorum of validators to + // signal for it. It is set to 0 if no version has reached quorum. + AppVersion uint64 `protobuf:"varint,1,opt,name=app_version,json=appVersion,proto3" json:"app_version,omitempty"` + // UpgradeHeight is the height at which the network should upgrade to the + // AppVersion. It is set to 0 if no version has reached quorum. + UpgradeHeight int64 `protobuf:"varint,2,opt,name=upgrade_height,json=upgradeHeight,proto3" json:"upgrade_height,omitempty"` +} + +func (m *Upgrade) Reset() { *m = Upgrade{} } +func (m *Upgrade) String() string { return proto.CompactTextString(m) } +func (*Upgrade) ProtoMessage() {} +func (*Upgrade) Descriptor() ([]byte, []int) { + return fileDescriptor_7872d1b4aca9f179, []int{0} +} +func (m *Upgrade) XXX_Unmarshal(b []byte) error { + return m.Unmarshal(b) +} +func (m *Upgrade) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { + if deterministic { + return xxx_messageInfo_Upgrade.Marshal(b, m, deterministic) + } else { + b = b[:cap(b)] + n, err := m.MarshalToSizedBuffer(b) + if err != nil { + return nil, err + } + return b[:n], nil + } +} +func (m *Upgrade) XXX_Merge(src proto.Message) { + xxx_messageInfo_Upgrade.Merge(m, src) +} +func (m *Upgrade) XXX_Size() int { + return m.Size() +} +func (m *Upgrade) XXX_DiscardUnknown() { + xxx_messageInfo_Upgrade.DiscardUnknown(m) +} + +var xxx_messageInfo_Upgrade proto.InternalMessageInfo + +func (m *Upgrade) GetAppVersion() uint64 { + if m != nil { + return m.AppVersion + } + return 0 +} + +func (m *Upgrade) GetUpgradeHeight() int64 { + if m != nil { + return m.UpgradeHeight + } + return 0 +} + +func init() { + proto.RegisterType((*Upgrade)(nil), "celestia.signal.v1.Upgrade") +} + +func init() { proto.RegisterFile("celestia/signal/v1/upgrade.proto", fileDescriptor_7872d1b4aca9f179) } + +var fileDescriptor_7872d1b4aca9f179 = []byte{ + // 196 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe2, 0x52, 0x48, 0x4e, 0xcd, 0x49, + 0x2d, 0x2e, 0xc9, 0x4c, 0xd4, 0x2f, 0xce, 0x4c, 0xcf, 0x4b, 0xcc, 0xd1, 0x2f, 0x33, 0xd4, 0x2f, + 0x2d, 0x48, 0x2f, 0x4a, 0x4c, 0x49, 0xd5, 0x2b, 0x28, 0xca, 0x2f, 0xc9, 0x17, 0x12, 0x82, 0xa9, + 0xd0, 0x83, 0xa8, 0xd0, 0x2b, 0x33, 0x54, 0x0a, 0xe4, 0x62, 0x0f, 0x85, 0x28, 0x12, 0x92, 0xe7, + 0xe2, 0x4e, 0x2c, 0x28, 0x88, 0x2f, 0x4b, 0x2d, 0x2a, 0xce, 0xcc, 0xcf, 0x93, 0x60, 0x54, 0x60, + 0xd4, 0x60, 0x09, 0xe2, 0x4a, 0x2c, 0x28, 0x08, 0x83, 0x88, 0x08, 0xa9, 0x72, 0xf1, 0x41, 0x0d, + 0x8c, 0xcf, 0x48, 0xcd, 0x4c, 0xcf, 0x28, 0x91, 0x60, 0x52, 0x60, 0xd4, 0x60, 0x0e, 0xe2, 0x85, + 0x8a, 0x7a, 0x80, 0x05, 0x9d, 0x7c, 0x4e, 0x3c, 0x92, 0x63, 0xbc, 0xf0, 0x48, 0x8e, 0xf1, 0xc1, + 0x23, 0x39, 0xc6, 0x09, 0x8f, 0xe5, 0x18, 0x2e, 0x3c, 0x96, 0x63, 0xb8, 0xf1, 0x58, 0x8e, 0x21, + 0xca, 0x28, 0x3d, 0xb3, 0x24, 0xa3, 0x34, 0x49, 0x2f, 0x39, 0x3f, 0x57, 0x1f, 0xe6, 0x96, 0xfc, + 0xa2, 0x74, 0x38, 0x5b, 0x37, 0xb1, 0xa0, 0x40, 0xbf, 0x02, 0xe6, 0xfe, 0x92, 0xca, 0x82, 0xd4, + 0xe2, 0x24, 0x36, 0xb0, 0xdb, 0x8d, 0x01, 0x01, 0x00, 0x00, 0xff, 0xff, 0x63, 0x22, 0x2e, 0x9d, + 0xdf, 0x00, 0x00, 0x00, +} + +func (m *Upgrade) Marshal() (dAtA []byte, err error) { + size := m.Size() + dAtA = make([]byte, size) + n, err := m.MarshalToSizedBuffer(dAtA[:size]) + if err != nil { + return nil, err + } + return dAtA[:n], nil +} + +func (m *Upgrade) MarshalTo(dAtA []byte) (int, error) { + size := m.Size() + return m.MarshalToSizedBuffer(dAtA[:size]) +} + +func (m *Upgrade) MarshalToSizedBuffer(dAtA []byte) (int, error) { + i := len(dAtA) + _ = i + var l int + _ = l + if m.UpgradeHeight != 0 { + i = encodeVarintUpgrade(dAtA, i, uint64(m.UpgradeHeight)) + i-- + dAtA[i] = 0x10 + } + if m.AppVersion != 0 { + i = encodeVarintUpgrade(dAtA, i, uint64(m.AppVersion)) + i-- + dAtA[i] = 0x8 + } + return len(dAtA) - i, nil +} + +func encodeVarintUpgrade(dAtA []byte, offset int, v uint64) int { + offset -= sovUpgrade(v) + base := offset + for v >= 1<<7 { + dAtA[offset] = uint8(v&0x7f | 0x80) + v >>= 7 + offset++ + } + dAtA[offset] = uint8(v) + return base +} +func (m *Upgrade) Size() (n int) { + if m == nil { + return 0 + } + var l int + _ = l + if m.AppVersion != 0 { + n += 1 + sovUpgrade(uint64(m.AppVersion)) + } + if m.UpgradeHeight != 0 { + n += 1 + sovUpgrade(uint64(m.UpgradeHeight)) + } + return n +} + +func sovUpgrade(x uint64) (n int) { + return (math_bits.Len64(x|1) + 6) / 7 +} +func sozUpgrade(x uint64) (n int) { + return sovUpgrade(uint64((x << 1) ^ uint64((int64(x) >> 63)))) +} +func (m *Upgrade) Unmarshal(dAtA []byte) error { + l := len(dAtA) + iNdEx := 0 + for iNdEx < l { + preIndex := iNdEx + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowUpgrade + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + fieldNum := int32(wire >> 3) + wireType := int(wire & 0x7) + if wireType == 4 { + return fmt.Errorf("proto: Upgrade: wiretype end group for non-group") + } + if fieldNum <= 0 { + return fmt.Errorf("proto: Upgrade: illegal tag %d (wire type %d)", fieldNum, wire) + } + switch fieldNum { + case 1: + if wireType != 0 { + return fmt.Errorf("proto: wrong wireType = %d for field AppVersion", wireType) + } + m.AppVersion = 0 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowUpgrade + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + m.AppVersion |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + case 2: + if wireType != 0 { + return fmt.Errorf("proto: wrong wireType = %d for field UpgradeHeight", wireType) + } + m.UpgradeHeight = 0 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowUpgrade + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + m.UpgradeHeight |= int64(b&0x7F) << shift + if b < 0x80 { + break + } + } + default: + iNdEx = preIndex + skippy, err := skipUpgrade(dAtA[iNdEx:]) + if err != nil { + return err + } + if (skippy < 0) || (iNdEx+skippy) < 0 { + return ErrInvalidLengthUpgrade + } + if (iNdEx + skippy) > l { + return io.ErrUnexpectedEOF + } + iNdEx += skippy + } + } + + if iNdEx > l { + return io.ErrUnexpectedEOF + } + return nil +} +func skipUpgrade(dAtA []byte) (n int, err error) { + l := len(dAtA) + iNdEx := 0 + depth := 0 + for iNdEx < l { + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return 0, ErrIntOverflowUpgrade + } + if iNdEx >= l { + return 0, io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= (uint64(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + wireType := int(wire & 0x7) + switch wireType { + case 0: + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return 0, ErrIntOverflowUpgrade + } + if iNdEx >= l { + return 0, io.ErrUnexpectedEOF + } + iNdEx++ + if dAtA[iNdEx-1] < 0x80 { + break + } + } + case 1: + iNdEx += 8 + case 2: + var length int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return 0, ErrIntOverflowUpgrade + } + if iNdEx >= l { + return 0, io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + length |= (int(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + if length < 0 { + return 0, ErrInvalidLengthUpgrade + } + iNdEx += length + case 3: + depth++ + case 4: + if depth == 0 { + return 0, ErrUnexpectedEndOfGroupUpgrade + } + depth-- + case 5: + iNdEx += 4 + default: + return 0, fmt.Errorf("proto: illegal wireType %d", wireType) + } + if iNdEx < 0 { + return 0, ErrInvalidLengthUpgrade + } + if depth == 0 { + return iNdEx, nil + } + } + return 0, io.ErrUnexpectedEOF +} + +var ( + ErrInvalidLengthUpgrade = fmt.Errorf("proto: negative length found during unmarshaling") + ErrIntOverflowUpgrade = fmt.Errorf("proto: integer overflow") + ErrUnexpectedEndOfGroupUpgrade = fmt.Errorf("proto: unexpected end of group") +) From d812bef14ddcd637d408b306ecda8c803675388f Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Fri, 14 Jun 2024 15:09:58 -0400 Subject: [PATCH 08/22] fix: skip over upgrade key --- x/signal/keeper.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/x/signal/keeper.go b/x/signal/keeper.go index 69f60c10b4..ffa946d72a 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -1,6 +1,7 @@ package signal import ( + "bytes" "context" "encoding/binary" @@ -126,6 +127,9 @@ func (k Keeper) VersionTally(ctx context.Context, req *types.QueryVersionTallyRe iterator := store.Iterator(nil, nil) defer iterator.Close() for ; iterator.Valid(); iterator.Next() { + if bytes.Equal(iterator.Key(), types.UpgradeKey) { + continue + } valAddress := sdk.ValAddress(iterator.Key()) power := k.stakingKeeper.GetLastValidatorPower(sdkCtx, valAddress) version := VersionFromBytes(iterator.Value()) @@ -162,6 +166,9 @@ func (k Keeper) TallyVotingPower(ctx sdk.Context, threshold int64) (bool, uint64 iterator := store.Iterator(nil, nil) defer iterator.Close() for ; iterator.Valid(); iterator.Next() { + if bytes.Equal(iterator.Key(), types.UpgradeKey) { + continue + } valAddress := sdk.ValAddress(iterator.Key()) // check that the validator is still part of the bonded set val, found := k.stakingKeeper.GetValidator(ctx, valAddress) @@ -219,6 +226,9 @@ func (k *Keeper) ResetTally(ctx sdk.Context) { iterator := store.Iterator(nil, nil) defer iterator.Close() for ; iterator.Valid(); iterator.Next() { + if bytes.Equal(iterator.Key(), types.UpgradeKey) { + continue + } store.Delete(iterator.Key()) } k.setUpgrade(ctx, types.Upgrade{}) From b8fe746eaf36fd7eff742c2741b149134325b770 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Fri, 14 Jun 2024 15:12:36 -0400 Subject: [PATCH 09/22] fix: TestTallyingLogic --- x/signal/keeper.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/signal/keeper.go b/x/signal/keeper.go index ffa946d72a..d9cede5166 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -226,12 +226,13 @@ func (k *Keeper) ResetTally(ctx sdk.Context) { iterator := store.Iterator(nil, nil) defer iterator.Close() for ; iterator.Valid(); iterator.Next() { + // skip over the upgrade key if bytes.Equal(iterator.Key(), types.UpgradeKey) { continue } store.Delete(iterator.Key()) } - k.setUpgrade(ctx, types.Upgrade{}) + store.Delete(types.UpgradeKey) } func VersionToBytes(version uint64) []byte { From 44df0411f8bea5c3770856a2a8a2f71a3db160c2 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Fri, 14 Jun 2024 15:21:19 -0400 Subject: [PATCH 10/22] fix: comments and variables --- proto/celestia/signal/v1/upgrade.proto | 6 ++--- x/signal/keeper.go | 33 ++++++++++++++------------ 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/proto/celestia/signal/v1/upgrade.proto b/proto/celestia/signal/v1/upgrade.proto index 02d3a8a27c..6d5ac4e03e 100644 --- a/proto/celestia/signal/v1/upgrade.proto +++ b/proto/celestia/signal/v1/upgrade.proto @@ -5,11 +5,11 @@ option go_package = "github.com/celestiaorg/celestia-app/x/signal/types"; // Upgrade is a type that represents a network upgrade. message Upgrade { - // AppVersion is the version that has received a quorum of validators to - // signal for it. It is set to 0 if no version has reached quorum. + // AppVersion is the app version that has received a quorum of validators to + // signal for it. uint64 app_version = 1; // UpgradeHeight is the height at which the network should upgrade to the - // AppVersion. It is set to 0 if no version has reached quorum. + // AppVersion. int64 upgrade_height = 2; } diff --git a/x/signal/keeper.go b/x/signal/keeper.go index d9cede5166..335fb23ff2 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -91,10 +91,10 @@ func (k Keeper) SignalVersion(ctx context.Context, req *types.MsgSignalVersion) return &types.MsgSignalVersionResponse{}, nil } -// TryUpgrade is a method required by the MsgServer interface. -// It tallies the voting power that has voted on each version. -// If one version has quorum, it is set as the quorum version -// which the application can use as signal to upgrade to that version. +// TryUpgrade is a method required by the MsgServer interface. It tallies the +// voting power that has voted on each version. If one version has reached a +// quorum, an upgrade is persisted to the store. The upgrade is used by the +// application later when it is time to upgrade to that version. func (k *Keeper) TryUpgrade(ctx context.Context, _ *types.MsgTryUpgrade) (*types.MsgTryUpgradeResponse, error) { sdkCtx := sdk.UnwrapSDKContext(ctx) @@ -225,13 +225,15 @@ func (k *Keeper) ResetTally(ctx sdk.Context) { store := ctx.KVStore(k.storeKey) iterator := store.Iterator(nil, nil) defer iterator.Close() + // delete all signals for ; iterator.Valid(); iterator.Next() { - // skip over the upgrade key if bytes.Equal(iterator.Key(), types.UpgradeKey) { + // skip over the upgrade key continue } store.Delete(iterator.Key()) } + // delete the upgrade value store.Delete(types.UpgradeKey) } @@ -243,30 +245,31 @@ func VersionFromBytes(version []byte) uint64 { return binary.BigEndian.Uint64(version) } -// isUpgradePending returns true if a version has reached quorum and the chain -// will upgrade to the upgrade app version at the upgrade height. While the -// keeper has an upgrade pending actions like SignalVersion and TryUpgrade will +// isUpgradePending returns true if an app version has reached quorum and the +// chain should upgrade to the app version at the upgrade height. While the +// keeper has an upgrade pending the SignalVersion and TryUpgrade messages will // be rejected. func (k *Keeper) isUpgradePending(ctx sdk.Context) bool { _, ok := k.getUpgrade(ctx) return ok } -// getUpgrade returns the upgrade from the store if it exists along with a -// boolean indicating its existence. +// getUpgrade returns the current upgrade information from the store. +// If an upgrade is found, it returns the upgrade object and true. +// If no upgrade is found, it returns an empty upgrade object and false. func (k *Keeper) getUpgrade(ctx sdk.Context) (upgrade types.Upgrade, ok bool) { store := ctx.KVStore(k.storeKey) - bz := store.Get(types.UpgradeKey) - if bz == nil { + value := store.Get(types.UpgradeKey) + if value == nil { return types.Upgrade{}, false } - k.binaryCodec.MustUnmarshal(bz, &upgrade) + k.binaryCodec.MustUnmarshal(value, &upgrade) return upgrade, true } // setUpgrade sets the upgrade in the store. func (k *Keeper) setUpgrade(ctx sdk.Context, upgrade types.Upgrade) { store := ctx.KVStore(k.storeKey) - b := k.binaryCodec.MustMarshal(&upgrade) - store.Set(types.UpgradeKey, b) + value := k.binaryCodec.MustMarshal(&upgrade) + store.Set(types.UpgradeKey, value) } From a0b13d204360fa93fb0d2dcf34217415cb1c3aac Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Fri, 14 Jun 2024 16:00:56 -0400 Subject: [PATCH 11/22] feat: add query for GetUpgrade --- proto/celestia/signal/v1/query.proto | 15 ++ x/signal/keeper.go | 9 + x/signal/keeper_test.go | 31 +++ x/signal/types/query.pb.go | 388 +++++++++++++++++++++++++-- x/signal/types/query.pb.gw.go | 65 +++++ x/signal/types/upgrade.pb.go | 6 +- 6 files changed, 488 insertions(+), 26 deletions(-) diff --git a/proto/celestia/signal/v1/query.proto b/proto/celestia/signal/v1/query.proto index 3672e7f787..3999db7712 100644 --- a/proto/celestia/signal/v1/query.proto +++ b/proto/celestia/signal/v1/query.proto @@ -2,6 +2,7 @@ syntax = "proto3"; package celestia.signal.v1; import "google/api/annotations.proto"; +import "celestia/signal/v1/upgrade.proto"; option go_package = "github.com/celestiaorg/celestia-app/x/signal/types"; @@ -13,6 +14,12 @@ service Query { returns (QueryVersionTallyResponse) { option (google.api.http).get = "/upgrade/v1/tally/{version}"; } + + // GetUpgrade enables a client to query for upgrade information if an upgrade is pending. + rpc GetUpgrade(QueryGetUpgradeRequest) + returns (QueryGetUpgradeResponse) { + option (google.api.http).get = "/signal/v1/upgrade"; + } } // QueryVersionTallyRequest is the request type for the VersionTally query. @@ -24,3 +31,11 @@ message QueryVersionTallyResponse { uint64 threshold_power = 2; uint64 total_voting_power = 3; } + +// QueryGetUpgradeRequest is the request type for the GetUpgrade query. +message QueryGetUpgradeRequest {} + +// QueryGetUpgradeResponse is the response type for the GetUpgrade query. +message QueryGetUpgradeResponse { + Upgrade upgrade = 1; +} diff --git a/x/signal/keeper.go b/x/signal/keeper.go index f00ce7dbdb..061db89e16 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -244,6 +244,15 @@ func VersionFromBytes(version []byte) uint64 { return binary.BigEndian.Uint64(version) } +// GetUpgrade returns the current upgrade information. +func (k Keeper) GetUpgrade(ctx context.Context, request *types.QueryGetUpgradeRequest) (*types.QueryGetUpgradeResponse, error) { + upgrade, ok := k.getUpgrade(sdk.UnwrapSDKContext(ctx)) + if !ok { + return &types.QueryGetUpgradeResponse{}, nil + } + return &types.QueryGetUpgradeResponse{Upgrade: &upgrade}, nil +} + // IsUpgradePending returns true if an app version has reached quorum and the // chain should upgrade to the app version at the upgrade height. While the // keeper has an upgrade pending the SignalVersion and TryUpgrade messages will diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index daec3e6411..62e09f2cc8 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -386,6 +386,37 @@ func TestTryUpgrade(t *testing.T) { }) } +func TestGetUpgrade(t *testing.T) { + upgradeKeeper, ctx, _ := setup(t) + goCtx := sdk.WrapSDKContext(ctx) + + t.Run("should return an empty upgrade if no upgrade is pending", func(t *testing.T) { + got, err := upgradeKeeper.GetUpgrade(ctx, &types.QueryGetUpgradeRequest{}) + require.NoError(t, err) + assert.Nil(t, got.Upgrade) + }) + + t.Run("should return an upgrade if an upgrade is pending", func(t *testing.T) { + _, err := upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[0].String(), Version: 2}) + require.NoError(t, err) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[1].String(), Version: 2}) + require.NoError(t, err) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[2].String(), Version: 2}) + require.NoError(t, err) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[3].String(), Version: 2}) + require.NoError(t, err) + + // This TryUpgrade should succeed. + _, err = upgradeKeeper.TryUpgrade(goCtx, &types.MsgTryUpgrade{}) + require.NoError(t, err) + + got, err := upgradeKeeper.GetUpgrade(ctx, &types.QueryGetUpgradeRequest{}) + require.NoError(t, err) + assert.Equal(t, uint64(2), got.Upgrade.AppVersion) + assert.Equal(t, int64(defaultUpgradeHeightDelay), got.Upgrade.UpgradeHeight) + }) +} + func setup(t *testing.T) (signal.Keeper, sdk.Context, *mockStakingKeeper) { signalStore := sdk.NewKVStoreKey(types.StoreKey) db := tmdb.NewMemDB() diff --git a/x/signal/types/query.pb.go b/x/signal/types/query.pb.go index 1f2998df99..63d48752ad 100644 --- a/x/signal/types/query.pb.go +++ b/x/signal/types/query.pb.go @@ -134,37 +134,125 @@ func (m *QueryVersionTallyResponse) GetTotalVotingPower() uint64 { return 0 } +// QueryGetUpgradeRequest is the request type for the GetUpgrade query. +type QueryGetUpgradeRequest struct { +} + +func (m *QueryGetUpgradeRequest) Reset() { *m = QueryGetUpgradeRequest{} } +func (m *QueryGetUpgradeRequest) String() string { return proto.CompactTextString(m) } +func (*QueryGetUpgradeRequest) ProtoMessage() {} +func (*QueryGetUpgradeRequest) Descriptor() ([]byte, []int) { + return fileDescriptor_7af24246367e432c, []int{2} +} +func (m *QueryGetUpgradeRequest) XXX_Unmarshal(b []byte) error { + return m.Unmarshal(b) +} +func (m *QueryGetUpgradeRequest) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { + if deterministic { + return xxx_messageInfo_QueryGetUpgradeRequest.Marshal(b, m, deterministic) + } else { + b = b[:cap(b)] + n, err := m.MarshalToSizedBuffer(b) + if err != nil { + return nil, err + } + return b[:n], nil + } +} +func (m *QueryGetUpgradeRequest) XXX_Merge(src proto.Message) { + xxx_messageInfo_QueryGetUpgradeRequest.Merge(m, src) +} +func (m *QueryGetUpgradeRequest) XXX_Size() int { + return m.Size() +} +func (m *QueryGetUpgradeRequest) XXX_DiscardUnknown() { + xxx_messageInfo_QueryGetUpgradeRequest.DiscardUnknown(m) +} + +var xxx_messageInfo_QueryGetUpgradeRequest proto.InternalMessageInfo + +// QueryGetUpgradeResponse is the response type for the GetUpgrade query. +type QueryGetUpgradeResponse struct { + Upgrade *Upgrade `protobuf:"bytes,1,opt,name=upgrade,proto3" json:"upgrade,omitempty"` +} + +func (m *QueryGetUpgradeResponse) Reset() { *m = QueryGetUpgradeResponse{} } +func (m *QueryGetUpgradeResponse) String() string { return proto.CompactTextString(m) } +func (*QueryGetUpgradeResponse) ProtoMessage() {} +func (*QueryGetUpgradeResponse) Descriptor() ([]byte, []int) { + return fileDescriptor_7af24246367e432c, []int{3} +} +func (m *QueryGetUpgradeResponse) XXX_Unmarshal(b []byte) error { + return m.Unmarshal(b) +} +func (m *QueryGetUpgradeResponse) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { + if deterministic { + return xxx_messageInfo_QueryGetUpgradeResponse.Marshal(b, m, deterministic) + } else { + b = b[:cap(b)] + n, err := m.MarshalToSizedBuffer(b) + if err != nil { + return nil, err + } + return b[:n], nil + } +} +func (m *QueryGetUpgradeResponse) XXX_Merge(src proto.Message) { + xxx_messageInfo_QueryGetUpgradeResponse.Merge(m, src) +} +func (m *QueryGetUpgradeResponse) XXX_Size() int { + return m.Size() +} +func (m *QueryGetUpgradeResponse) XXX_DiscardUnknown() { + xxx_messageInfo_QueryGetUpgradeResponse.DiscardUnknown(m) +} + +var xxx_messageInfo_QueryGetUpgradeResponse proto.InternalMessageInfo + +func (m *QueryGetUpgradeResponse) GetUpgrade() *Upgrade { + if m != nil { + return m.Upgrade + } + return nil +} + func init() { proto.RegisterType((*QueryVersionTallyRequest)(nil), "celestia.signal.v1.QueryVersionTallyRequest") proto.RegisterType((*QueryVersionTallyResponse)(nil), "celestia.signal.v1.QueryVersionTallyResponse") + proto.RegisterType((*QueryGetUpgradeRequest)(nil), "celestia.signal.v1.QueryGetUpgradeRequest") + proto.RegisterType((*QueryGetUpgradeResponse)(nil), "celestia.signal.v1.QueryGetUpgradeResponse") } func init() { proto.RegisterFile("celestia/signal/v1/query.proto", fileDescriptor_7af24246367e432c) } var fileDescriptor_7af24246367e432c = []byte{ - // 337 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x8c, 0x91, 0xcd, 0x4e, 0x3a, 0x31, - 0x14, 0xc5, 0x29, 0xff, 0x0f, 0x93, 0x4a, 0xd4, 0x74, 0x85, 0xa8, 0x13, 0xc5, 0x85, 0x2e, 0x60, - 0x1a, 0xd0, 0x27, 0x70, 0xed, 0x42, 0x89, 0x61, 0xe1, 0x86, 0x14, 0x68, 0x4a, 0x93, 0xda, 0x5b, - 0xda, 0xce, 0x28, 0x31, 0x6e, 0x7c, 0x02, 0x12, 0xe3, 0xc6, 0x27, 0x72, 0x49, 0xe2, 0xc6, 0xa5, - 0x01, 0x1f, 0xc4, 0xcc, 0x0c, 0x83, 0x1a, 0x35, 0x71, 0x37, 0x73, 0xee, 0xef, 0x9e, 0xde, 0x7b, - 0x0f, 0x0e, 0x7a, 0x5c, 0x71, 0xe7, 0x25, 0xa3, 0x4e, 0x0a, 0xcd, 0x14, 0x8d, 0x1b, 0x74, 0x18, - 0x71, 0x3b, 0x0a, 0x8d, 0x05, 0x0f, 0x84, 0xe4, 0xf5, 0x30, 0xab, 0x87, 0x71, 0xa3, 0xb2, 0x29, - 0x00, 0x84, 0xe2, 0x94, 0x19, 0x49, 0x99, 0xd6, 0xe0, 0x99, 0x97, 0xa0, 0x5d, 0xd6, 0x51, 0x3d, - 0xc4, 0xe5, 0xd3, 0xc4, 0xa0, 0xcd, 0xad, 0x93, 0xa0, 0xcf, 0x98, 0x52, 0xa3, 0x16, 0x1f, 0x46, - 0xdc, 0x79, 0x52, 0xc6, 0x4b, 0x71, 0x26, 0x97, 0xd1, 0x36, 0xda, 0xff, 0xdb, 0xca, 0x7f, 0xab, - 0xf7, 0x08, 0xaf, 0x7f, 0xd3, 0xe6, 0x0c, 0x68, 0xc7, 0xc9, 0x0e, 0x2e, 0xc5, 0xe0, 0xa5, 0x16, - 0x1d, 0x03, 0x97, 0xdc, 0xce, 0x9b, 0x97, 0x33, 0xed, 0x24, 0x91, 0xc8, 0x1e, 0x5e, 0xf5, 0x03, - 0xcb, 0xdd, 0x00, 0x54, 0x7f, 0x4e, 0x15, 0x53, 0x6a, 0x65, 0x21, 0x67, 0x60, 0x0d, 0x13, 0x0f, - 0x9e, 0xa9, 0xce, 0x27, 0xc7, 0x3f, 0x29, 0xbb, 0x96, 0x56, 0xda, 0xef, 0xb6, 0xcd, 0x07, 0x84, - 0xff, 0xa5, 0x73, 0x91, 0x31, 0xc2, 0xa5, 0x8f, 0xc3, 0x91, 0x5a, 0xf8, 0xf5, 0x36, 0xe1, 0x4f, - 0xab, 0x57, 0xea, 0xbf, 0xa4, 0xb3, 0x8d, 0xab, 0xbb, 0xb7, 0x4f, 0xaf, 0x77, 0xc5, 0x2d, 0xb2, - 0x41, 0x23, 0x23, 0x2c, 0xeb, 0xf3, 0x24, 0x18, 0x9f, 0x20, 0xf4, 0x7a, 0x7e, 0xb3, 0x9b, 0xa3, - 0xe3, 0xc7, 0x69, 0x80, 0x26, 0xd3, 0x00, 0xbd, 0x4c, 0x03, 0x34, 0x9e, 0x05, 0x85, 0xc9, 0x2c, - 0x28, 0x3c, 0xcf, 0x82, 0xc2, 0x79, 0x53, 0x48, 0x3f, 0x88, 0xba, 0x61, 0x0f, 0x2e, 0x68, 0xfe, - 0x2e, 0x58, 0xb1, 0xf8, 0xae, 0x33, 0x63, 0xe8, 0x55, 0x9e, 0xb9, 0x1f, 0x19, 0xee, 0xba, 0xff, - 0xd3, 0xfc, 0x0e, 0xde, 0x02, 0x00, 0x00, 0xff, 0xff, 0xe7, 0x5f, 0x33, 0x08, 0x13, 0x02, 0x00, - 0x00, + // 414 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x8c, 0x92, 0xcf, 0x6e, 0xda, 0x40, + 0x10, 0xc6, 0x59, 0xfa, 0x07, 0x69, 0x41, 0x6d, 0xb5, 0xaa, 0x5a, 0xd7, 0xb4, 0x16, 0x75, 0x0f, + 0xad, 0x5a, 0xf0, 0x0a, 0xda, 0xbe, 0x40, 0x2e, 0xb9, 0xe4, 0x40, 0x50, 0xc2, 0x21, 0x17, 0xb4, + 0xc0, 0xca, 0x58, 0x72, 0xbc, 0x8b, 0x77, 0xed, 0x04, 0x45, 0x39, 0x24, 0x4f, 0x80, 0x14, 0xe5, + 0x9a, 0xe7, 0xc9, 0x11, 0x29, 0x97, 0x1c, 0x23, 0xc8, 0x83, 0x44, 0xde, 0xb5, 0x21, 0x91, 0x41, + 0xe2, 0x66, 0xcf, 0xfc, 0xe6, 0x9b, 0x6f, 0x66, 0x16, 0x5a, 0x03, 0xea, 0x53, 0x21, 0x3d, 0x82, + 0x85, 0xe7, 0x06, 0xc4, 0xc7, 0x71, 0x13, 0x8f, 0x23, 0x1a, 0x4e, 0x1c, 0x1e, 0x32, 0xc9, 0x10, + 0xca, 0xf2, 0x8e, 0xce, 0x3b, 0x71, 0xd3, 0xfc, 0xea, 0x32, 0xe6, 0xfa, 0x14, 0x13, 0xee, 0x61, + 0x12, 0x04, 0x4c, 0x12, 0xe9, 0xb1, 0x40, 0xe8, 0x0a, 0xb3, 0xb6, 0x46, 0x31, 0xe2, 0x6e, 0x48, + 0x86, 0x54, 0x13, 0xf6, 0x3f, 0x68, 0xec, 0x27, 0x2d, 0xba, 0x34, 0x14, 0x1e, 0x0b, 0x0e, 0x88, + 0xef, 0x4f, 0x3a, 0x74, 0x1c, 0x51, 0x21, 0x91, 0x01, 0x4b, 0xb1, 0x0e, 0x1b, 0xa0, 0x06, 0x7e, + 0xbd, 0xee, 0x64, 0xbf, 0xf6, 0x35, 0x80, 0x5f, 0xd6, 0x94, 0x09, 0xce, 0x02, 0x41, 0xd1, 0x77, + 0x58, 0x89, 0x99, 0xf4, 0x02, 0xb7, 0xc7, 0xd9, 0x09, 0x0d, 0xd3, 0xe2, 0xb2, 0x8e, 0xb5, 0x93, + 0x10, 0xfa, 0x09, 0xdf, 0xcb, 0x51, 0x48, 0xc5, 0x88, 0xf9, 0xc3, 0x94, 0x2a, 0x2a, 0xea, 0xdd, + 0x32, 0xac, 0xc1, 0x3a, 0x44, 0x92, 0x49, 0xe2, 0xf7, 0x5e, 0x28, 0xbe, 0x52, 0xec, 0x07, 0x95, + 0xe9, 0xae, 0x64, 0x6d, 0x03, 0x7e, 0x52, 0xb6, 0x76, 0xa9, 0x3c, 0xd4, 0x63, 0xa6, 0xb3, 0xd8, + 0x6d, 0xf8, 0x39, 0x97, 0x49, 0xed, 0xfe, 0x87, 0xa5, 0x74, 0x27, 0xca, 0x69, 0xb9, 0x55, 0x75, + 0xf2, 0x8b, 0x76, 0xb2, 0xaa, 0x8c, 0x6d, 0xdd, 0x14, 0xe1, 0x1b, 0x25, 0x89, 0xa6, 0x00, 0x56, + 0x9e, 0x2f, 0x02, 0xd5, 0xd7, 0x09, 0x6c, 0x5a, 0xb3, 0xd9, 0xd8, 0x92, 0xd6, 0x76, 0xed, 0x1f, + 0x97, 0x77, 0x8f, 0x57, 0xc5, 0x6f, 0xa8, 0x9a, 0x5d, 0x32, 0x39, 0xaa, 0x4c, 0x10, 0x7c, 0x96, + 0xde, 0xe7, 0x1c, 0x5d, 0x00, 0x08, 0x57, 0xa3, 0xa2, 0xdf, 0x1b, 0x5b, 0xe4, 0x36, 0x65, 0xfe, + 0xd9, 0x8a, 0x4d, 0xcd, 0x98, 0xca, 0xcc, 0x47, 0x84, 0xf2, 0x0f, 0x6c, 0x67, 0xef, 0x76, 0x6e, + 0x81, 0xd9, 0xdc, 0x02, 0x0f, 0x73, 0x0b, 0x4c, 0x17, 0x56, 0x61, 0xb6, 0xb0, 0x0a, 0xf7, 0x0b, + 0xab, 0x70, 0xd4, 0x72, 0x3d, 0x39, 0x8a, 0xfa, 0xce, 0x80, 0x1d, 0xe3, 0xac, 0x19, 0x0b, 0xdd, + 0xe5, 0x77, 0x83, 0x70, 0x8e, 0x4f, 0x33, 0x49, 0x39, 0xe1, 0x54, 0xf4, 0xdf, 0xaa, 0xf7, 0xfa, + 0xf7, 0x29, 0x00, 0x00, 0xff, 0xff, 0x3d, 0x57, 0x0e, 0xab, 0x25, 0x03, 0x00, 0x00, } // Reference imports to suppress errors if they are not otherwise used. @@ -182,6 +270,8 @@ type QueryClient interface { // VersionTally enables a client to query for the tally of voting power has // signalled for a particular version. VersionTally(ctx context.Context, in *QueryVersionTallyRequest, opts ...grpc.CallOption) (*QueryVersionTallyResponse, error) + // GetUpgrade enables a client to query for upgrade information if an upgrade is pending. + GetUpgrade(ctx context.Context, in *QueryGetUpgradeRequest, opts ...grpc.CallOption) (*QueryGetUpgradeResponse, error) } type queryClient struct { @@ -201,11 +291,22 @@ func (c *queryClient) VersionTally(ctx context.Context, in *QueryVersionTallyReq return out, nil } +func (c *queryClient) GetUpgrade(ctx context.Context, in *QueryGetUpgradeRequest, opts ...grpc.CallOption) (*QueryGetUpgradeResponse, error) { + out := new(QueryGetUpgradeResponse) + err := c.cc.Invoke(ctx, "/celestia.signal.v1.Query/GetUpgrade", in, out, opts...) + if err != nil { + return nil, err + } + return out, nil +} + // QueryServer is the server API for Query service. type QueryServer interface { // VersionTally enables a client to query for the tally of voting power has // signalled for a particular version. VersionTally(context.Context, *QueryVersionTallyRequest) (*QueryVersionTallyResponse, error) + // GetUpgrade enables a client to query for upgrade information if an upgrade is pending. + GetUpgrade(context.Context, *QueryGetUpgradeRequest) (*QueryGetUpgradeResponse, error) } // UnimplementedQueryServer can be embedded to have forward compatible implementations. @@ -215,6 +316,9 @@ type UnimplementedQueryServer struct { func (*UnimplementedQueryServer) VersionTally(ctx context.Context, req *QueryVersionTallyRequest) (*QueryVersionTallyResponse, error) { return nil, status.Errorf(codes.Unimplemented, "method VersionTally not implemented") } +func (*UnimplementedQueryServer) GetUpgrade(ctx context.Context, req *QueryGetUpgradeRequest) (*QueryGetUpgradeResponse, error) { + return nil, status.Errorf(codes.Unimplemented, "method GetUpgrade not implemented") +} func RegisterQueryServer(s grpc1.Server, srv QueryServer) { s.RegisterService(&_Query_serviceDesc, srv) @@ -238,6 +342,24 @@ func _Query_VersionTally_Handler(srv interface{}, ctx context.Context, dec func( return interceptor(ctx, in, info, handler) } +func _Query_GetUpgrade_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { + in := new(QueryGetUpgradeRequest) + if err := dec(in); err != nil { + return nil, err + } + if interceptor == nil { + return srv.(QueryServer).GetUpgrade(ctx, in) + } + info := &grpc.UnaryServerInfo{ + Server: srv, + FullMethod: "/celestia.signal.v1.Query/GetUpgrade", + } + handler := func(ctx context.Context, req interface{}) (interface{}, error) { + return srv.(QueryServer).GetUpgrade(ctx, req.(*QueryGetUpgradeRequest)) + } + return interceptor(ctx, in, info, handler) +} + var _Query_serviceDesc = grpc.ServiceDesc{ ServiceName: "celestia.signal.v1.Query", HandlerType: (*QueryServer)(nil), @@ -246,6 +368,10 @@ var _Query_serviceDesc = grpc.ServiceDesc{ MethodName: "VersionTally", Handler: _Query_VersionTally_Handler, }, + { + MethodName: "GetUpgrade", + Handler: _Query_GetUpgrade_Handler, + }, }, Streams: []grpc.StreamDesc{}, Metadata: "celestia/signal/v1/query.proto", @@ -317,6 +443,64 @@ func (m *QueryVersionTallyResponse) MarshalToSizedBuffer(dAtA []byte) (int, erro return len(dAtA) - i, nil } +func (m *QueryGetUpgradeRequest) Marshal() (dAtA []byte, err error) { + size := m.Size() + dAtA = make([]byte, size) + n, err := m.MarshalToSizedBuffer(dAtA[:size]) + if err != nil { + return nil, err + } + return dAtA[:n], nil +} + +func (m *QueryGetUpgradeRequest) MarshalTo(dAtA []byte) (int, error) { + size := m.Size() + return m.MarshalToSizedBuffer(dAtA[:size]) +} + +func (m *QueryGetUpgradeRequest) MarshalToSizedBuffer(dAtA []byte) (int, error) { + i := len(dAtA) + _ = i + var l int + _ = l + return len(dAtA) - i, nil +} + +func (m *QueryGetUpgradeResponse) Marshal() (dAtA []byte, err error) { + size := m.Size() + dAtA = make([]byte, size) + n, err := m.MarshalToSizedBuffer(dAtA[:size]) + if err != nil { + return nil, err + } + return dAtA[:n], nil +} + +func (m *QueryGetUpgradeResponse) MarshalTo(dAtA []byte) (int, error) { + size := m.Size() + return m.MarshalToSizedBuffer(dAtA[:size]) +} + +func (m *QueryGetUpgradeResponse) MarshalToSizedBuffer(dAtA []byte) (int, error) { + i := len(dAtA) + _ = i + var l int + _ = l + if m.Upgrade != nil { + { + size, err := m.Upgrade.MarshalToSizedBuffer(dAtA[:i]) + if err != nil { + return 0, err + } + i -= size + i = encodeVarintQuery(dAtA, i, uint64(size)) + } + i-- + dAtA[i] = 0xa + } + return len(dAtA) - i, nil +} + func encodeVarintQuery(dAtA []byte, offset int, v uint64) int { offset -= sovQuery(v) base := offset @@ -358,6 +542,28 @@ func (m *QueryVersionTallyResponse) Size() (n int) { return n } +func (m *QueryGetUpgradeRequest) Size() (n int) { + if m == nil { + return 0 + } + var l int + _ = l + return n +} + +func (m *QueryGetUpgradeResponse) Size() (n int) { + if m == nil { + return 0 + } + var l int + _ = l + if m.Upgrade != nil { + l = m.Upgrade.Size() + n += 1 + l + sovQuery(uint64(l)) + } + return n +} + func sovQuery(x uint64) (n int) { return (math_bits.Len64(x|1) + 6) / 7 } @@ -540,6 +746,142 @@ func (m *QueryVersionTallyResponse) Unmarshal(dAtA []byte) error { } return nil } +func (m *QueryGetUpgradeRequest) Unmarshal(dAtA []byte) error { + l := len(dAtA) + iNdEx := 0 + for iNdEx < l { + preIndex := iNdEx + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowQuery + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + fieldNum := int32(wire >> 3) + wireType := int(wire & 0x7) + if wireType == 4 { + return fmt.Errorf("proto: QueryGetUpgradeRequest: wiretype end group for non-group") + } + if fieldNum <= 0 { + return fmt.Errorf("proto: QueryGetUpgradeRequest: illegal tag %d (wire type %d)", fieldNum, wire) + } + switch fieldNum { + default: + iNdEx = preIndex + skippy, err := skipQuery(dAtA[iNdEx:]) + if err != nil { + return err + } + if (skippy < 0) || (iNdEx+skippy) < 0 { + return ErrInvalidLengthQuery + } + if (iNdEx + skippy) > l { + return io.ErrUnexpectedEOF + } + iNdEx += skippy + } + } + + if iNdEx > l { + return io.ErrUnexpectedEOF + } + return nil +} +func (m *QueryGetUpgradeResponse) Unmarshal(dAtA []byte) error { + l := len(dAtA) + iNdEx := 0 + for iNdEx < l { + preIndex := iNdEx + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowQuery + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + fieldNum := int32(wire >> 3) + wireType := int(wire & 0x7) + if wireType == 4 { + return fmt.Errorf("proto: QueryGetUpgradeResponse: wiretype end group for non-group") + } + if fieldNum <= 0 { + return fmt.Errorf("proto: QueryGetUpgradeResponse: illegal tag %d (wire type %d)", fieldNum, wire) + } + switch fieldNum { + case 1: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Upgrade", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowQuery + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= int(b&0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return ErrInvalidLengthQuery + } + postIndex := iNdEx + msglen + if postIndex < 0 { + return ErrInvalidLengthQuery + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + if m.Upgrade == nil { + m.Upgrade = &Upgrade{} + } + if err := m.Upgrade.Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } + iNdEx = postIndex + default: + iNdEx = preIndex + skippy, err := skipQuery(dAtA[iNdEx:]) + if err != nil { + return err + } + if (skippy < 0) || (iNdEx+skippy) < 0 { + return ErrInvalidLengthQuery + } + if (iNdEx + skippy) > l { + return io.ErrUnexpectedEOF + } + iNdEx += skippy + } + } + + if iNdEx > l { + return io.ErrUnexpectedEOF + } + return nil +} func skipQuery(dAtA []byte) (n int, err error) { l := len(dAtA) iNdEx := 0 diff --git a/x/signal/types/query.pb.gw.go b/x/signal/types/query.pb.gw.go index 0fe22f5631..6c9e54066a 100644 --- a/x/signal/types/query.pb.gw.go +++ b/x/signal/types/query.pb.gw.go @@ -87,6 +87,24 @@ func local_request_Query_VersionTally_0(ctx context.Context, marshaler runtime.M } +func request_Query_GetUpgrade_0(ctx context.Context, marshaler runtime.Marshaler, client QueryClient, req *http.Request, pathParams map[string]string) (proto.Message, runtime.ServerMetadata, error) { + var protoReq QueryGetUpgradeRequest + var metadata runtime.ServerMetadata + + msg, err := client.GetUpgrade(ctx, &protoReq, grpc.Header(&metadata.HeaderMD), grpc.Trailer(&metadata.TrailerMD)) + return msg, metadata, err + +} + +func local_request_Query_GetUpgrade_0(ctx context.Context, marshaler runtime.Marshaler, server QueryServer, req *http.Request, pathParams map[string]string) (proto.Message, runtime.ServerMetadata, error) { + var protoReq QueryGetUpgradeRequest + var metadata runtime.ServerMetadata + + msg, err := server.GetUpgrade(ctx, &protoReq) + return msg, metadata, err + +} + // RegisterQueryHandlerServer registers the http handlers for service Query to "mux". // UnaryRPC :call QueryServer directly. // StreamingRPC :currently unsupported pending https://github.com/grpc/grpc-go/issues/906. @@ -116,6 +134,29 @@ func RegisterQueryHandlerServer(ctx context.Context, mux *runtime.ServeMux, serv }) + mux.Handle("GET", pattern_Query_GetUpgrade_0, func(w http.ResponseWriter, req *http.Request, pathParams map[string]string) { + ctx, cancel := context.WithCancel(req.Context()) + defer cancel() + var stream runtime.ServerTransportStream + ctx = grpc.NewContextWithServerTransportStream(ctx, &stream) + inboundMarshaler, outboundMarshaler := runtime.MarshalerForRequest(mux, req) + rctx, err := runtime.AnnotateIncomingContext(ctx, mux, req) + if err != nil { + runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err) + return + } + resp, md, err := local_request_Query_GetUpgrade_0(rctx, inboundMarshaler, server, req, pathParams) + md.HeaderMD, md.TrailerMD = metadata.Join(md.HeaderMD, stream.Header()), metadata.Join(md.TrailerMD, stream.Trailer()) + ctx = runtime.NewServerMetadataContext(ctx, md) + if err != nil { + runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err) + return + } + + forward_Query_GetUpgrade_0(ctx, mux, outboundMarshaler, w, req, resp, mux.GetForwardResponseOptions()...) + + }) + return nil } @@ -177,13 +218,37 @@ func RegisterQueryHandlerClient(ctx context.Context, mux *runtime.ServeMux, clie }) + mux.Handle("GET", pattern_Query_GetUpgrade_0, func(w http.ResponseWriter, req *http.Request, pathParams map[string]string) { + ctx, cancel := context.WithCancel(req.Context()) + defer cancel() + inboundMarshaler, outboundMarshaler := runtime.MarshalerForRequest(mux, req) + rctx, err := runtime.AnnotateContext(ctx, mux, req) + if err != nil { + runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err) + return + } + resp, md, err := request_Query_GetUpgrade_0(rctx, inboundMarshaler, client, req, pathParams) + ctx = runtime.NewServerMetadataContext(ctx, md) + if err != nil { + runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err) + return + } + + forward_Query_GetUpgrade_0(ctx, mux, outboundMarshaler, w, req, resp, mux.GetForwardResponseOptions()...) + + }) + return nil } var ( pattern_Query_VersionTally_0 = runtime.MustPattern(runtime.NewPattern(1, []int{2, 0, 2, 1, 2, 2, 1, 0, 4, 1, 5, 3}, []string{"upgrade", "v1", "tally", "version"}, "", runtime.AssumeColonVerbOpt(false))) + + pattern_Query_GetUpgrade_0 = runtime.MustPattern(runtime.NewPattern(1, []int{2, 0, 2, 1, 2, 2}, []string{"signal", "v1", "upgrade"}, "", runtime.AssumeColonVerbOpt(false))) ) var ( forward_Query_VersionTally_0 = runtime.ForwardResponseMessage + + forward_Query_GetUpgrade_0 = runtime.ForwardResponseMessage ) diff --git a/x/signal/types/upgrade.pb.go b/x/signal/types/upgrade.pb.go index 24aaff5a44..53a2a9590e 100644 --- a/x/signal/types/upgrade.pb.go +++ b/x/signal/types/upgrade.pb.go @@ -24,11 +24,11 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // Upgrade is a type that represents a network upgrade. type Upgrade struct { - // AppVersion is the version that has received a quorum of validators to - // signal for it. It is set to 0 if no version has reached quorum. + // AppVersion is the app version that has received a quorum of validators to + // signal for it. AppVersion uint64 `protobuf:"varint,1,opt,name=app_version,json=appVersion,proto3" json:"app_version,omitempty"` // UpgradeHeight is the height at which the network should upgrade to the - // AppVersion. It is set to 0 if no version has reached quorum. + // AppVersion. UpgradeHeight int64 `protobuf:"varint,2,opt,name=upgrade_height,json=upgradeHeight,proto3" json:"upgrade_height,omitempty"` } From 16e8355d7dba37b639e391553e3ee9ae16e2d8da Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Fri, 14 Jun 2024 16:08:59 -0400 Subject: [PATCH 12/22] feat: CLI command to get upgrade --- x/signal/cli/cli_test.go | 7 +++++++ x/signal/cli/query.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/x/signal/cli/cli_test.go b/x/signal/cli/cli_test.go index 82d6dd0500..3d881dc83d 100644 --- a/x/signal/cli/cli_test.go +++ b/x/signal/cli/cli_test.go @@ -38,3 +38,10 @@ func (s *CLITestSuite) TestCmdQueryTally() { s.Require().Contains(output.String(), "threshold_power") s.Require().Contains(output.String(), "total_voting_power") } + +func (s *CLITestSuite) TestCmdGetUpgrade() { + cmd := cli.CmdGetUpgrade() + output, err := testutil.ExecTestCLICmd(s.ctx.Context, cmd, []string{}) + s.Require().NoError(err) + s.Require().Contains(output.String(), "no upgrade is pending.") +} diff --git a/x/signal/cli/query.go b/x/signal/cli/query.go index 565f808a26..89a15be6f9 100644 --- a/x/signal/cli/query.go +++ b/x/signal/cli/query.go @@ -54,3 +54,34 @@ func CmdQueryTally() *cobra.Command { flags.AddQueryFlagsToCmd(cmd) return cmd } + +func CmdGetUpgrade() *cobra.Command { + cmd := &cobra.Command{ + Use: "get-upgrade", + Short: "Query for the upgrade information if an upgrade is pending", + Args: cobra.NoArgs, + Example: "get-upgrade", + RunE: func(cmd *cobra.Command, args []string) error { + clientCtx, err := client.GetClientQueryContext(cmd) + if err != nil { + return err + } + + queryClient := types.NewQueryClient(clientCtx) + resp, err := queryClient.GetUpgrade(cmd.Context(), &types.QueryGetUpgradeRequest{}) + if err != nil { + return err + } + + if resp.Upgrade != nil { + clientCtx.PrintString(fmt.Sprintf("upgrade to app version %d at height %d is pending.\n", resp.Upgrade.AppVersion, resp.Upgrade.UpgradeHeight)) + return nil + } + clientCtx.PrintString("no upgrade is pending.") + return nil + }, + } + + flags.AddQueryFlagsToCmd(cmd) + return cmd +} From b17e8d4764d0072bf53099f6574373d5be76ffe8 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Fri, 14 Jun 2024 16:11:54 -0400 Subject: [PATCH 13/22] fix: add command --- x/signal/cli/query.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/x/signal/cli/query.go b/x/signal/cli/query.go index 89a15be6f9..fd3a9f110f 100644 --- a/x/signal/cli/query.go +++ b/x/signal/cli/query.go @@ -21,6 +21,7 @@ func GetQueryCmd() *cobra.Command { } cmd.AddCommand(CmdQueryTally()) + cmd.AddCommand(CmdGetUpgrade()) return cmd } @@ -57,10 +58,10 @@ func CmdQueryTally() *cobra.Command { func CmdGetUpgrade() *cobra.Command { cmd := &cobra.Command{ - Use: "get-upgrade", + Use: "upgrade", Short: "Query for the upgrade information if an upgrade is pending", Args: cobra.NoArgs, - Example: "get-upgrade", + Example: "upgrade", RunE: func(cmd *cobra.Command, args []string) error { clientCtx, err := client.GetClientQueryContext(cmd) if err != nil { @@ -74,10 +75,10 @@ func CmdGetUpgrade() *cobra.Command { } if resp.Upgrade != nil { - clientCtx.PrintString(fmt.Sprintf("upgrade to app version %d at height %d is pending.\n", resp.Upgrade.AppVersion, resp.Upgrade.UpgradeHeight)) + clientCtx.PrintString(fmt.Sprintf("An upgrade is pending to app version %d at height %d.\n", resp.Upgrade.AppVersion, resp.Upgrade.UpgradeHeight)) return nil } - clientCtx.PrintString("no upgrade is pending.") + clientCtx.PrintString("No upgrade is pending.\n") return nil }, } From 9dbec24a42b3cd446eb3b0c2e16f2040eda68c63 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Fri, 14 Jun 2024 16:40:24 -0400 Subject: [PATCH 14/22] fix: lint --- x/signal/cli/query.go | 8 +++----- x/signal/keeper.go | 2 +- x/signal/keeper_test.go | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/x/signal/cli/query.go b/x/signal/cli/query.go index fd3a9f110f..f703e8b3c4 100644 --- a/x/signal/cli/query.go +++ b/x/signal/cli/query.go @@ -62,7 +62,7 @@ func CmdGetUpgrade() *cobra.Command { Short: "Query for the upgrade information if an upgrade is pending", Args: cobra.NoArgs, Example: "upgrade", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { clientCtx, err := client.GetClientQueryContext(cmd) if err != nil { return err @@ -75,11 +75,9 @@ func CmdGetUpgrade() *cobra.Command { } if resp.Upgrade != nil { - clientCtx.PrintString(fmt.Sprintf("An upgrade is pending to app version %d at height %d.\n", resp.Upgrade.AppVersion, resp.Upgrade.UpgradeHeight)) - return nil + return clientCtx.PrintString(fmt.Sprintf("An upgrade is pending to app version %d at height %d.\n", resp.Upgrade.AppVersion, resp.Upgrade.UpgradeHeight)) } - clientCtx.PrintString("No upgrade is pending.\n") - return nil + return clientCtx.PrintString("No upgrade is pending.\n") }, } diff --git a/x/signal/keeper.go b/x/signal/keeper.go index 061db89e16..fb915a5e25 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -245,7 +245,7 @@ func VersionFromBytes(version []byte) uint64 { } // GetUpgrade returns the current upgrade information. -func (k Keeper) GetUpgrade(ctx context.Context, request *types.QueryGetUpgradeRequest) (*types.QueryGetUpgradeResponse, error) { +func (k Keeper) GetUpgrade(ctx context.Context, _ *types.QueryGetUpgradeRequest) (*types.QueryGetUpgradeResponse, error) { upgrade, ok := k.getUpgrade(sdk.UnwrapSDKContext(ctx)) if !ok { return &types.QueryGetUpgradeResponse{}, nil diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index 62e09f2cc8..6356a80ec8 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -413,7 +413,7 @@ func TestGetUpgrade(t *testing.T) { got, err := upgradeKeeper.GetUpgrade(ctx, &types.QueryGetUpgradeRequest{}) require.NoError(t, err) assert.Equal(t, uint64(2), got.Upgrade.AppVersion) - assert.Equal(t, int64(defaultUpgradeHeightDelay), got.Upgrade.UpgradeHeight) + assert.Equal(t, defaultUpgradeHeightDelay, got.Upgrade.UpgradeHeight) }) } From ff2d564ac984e88d0a6ee2cdc9934b6f35835b9d Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Fri, 14 Jun 2024 16:41:21 -0400 Subject: [PATCH 15/22] fix: test --- x/signal/cli/cli_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/signal/cli/cli_test.go b/x/signal/cli/cli_test.go index 3d881dc83d..36471c6eb4 100644 --- a/x/signal/cli/cli_test.go +++ b/x/signal/cli/cli_test.go @@ -43,5 +43,5 @@ func (s *CLITestSuite) TestCmdGetUpgrade() { cmd := cli.CmdGetUpgrade() output, err := testutil.ExecTestCLICmd(s.ctx.Context, cmd, []string{}) s.Require().NoError(err) - s.Require().Contains(output.String(), "no upgrade is pending.") + s.Require().Contains(output.String(), "No upgrade is pending.") } From 464e40b04c866179ff12248cfa44438d8171bfcd Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Mon, 17 Jun 2024 10:13:57 -0400 Subject: [PATCH 16/22] reafctor: make const public --- x/signal/integration_test.go | 3 ++- x/signal/keeper.go | 12 ++++++------ x/signal/keeper_test.go | 6 ++---- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/x/signal/integration_test.go b/x/signal/integration_test.go index 47cd434b47..edd26c3fb5 100644 --- a/x/signal/integration_test.go +++ b/x/signal/integration_test.go @@ -5,6 +5,7 @@ import ( "github.com/celestiaorg/celestia-app/v2/app" testutil "github.com/celestiaorg/celestia-app/v2/test/util" + "github.com/celestiaorg/celestia-app/v2/x/signal" "github.com/celestiaorg/celestia-app/v2/x/signal/types" "github.com/stretchr/testify/require" @@ -73,7 +74,7 @@ func TestUpgradeIntegration(t *testing.T) { require.False(t, shouldUpgrade) require.EqualValues(t, 0, version) - ctx = ctx.WithBlockHeight(ctx.BlockHeight() + defaultUpgradeHeightDelay) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + signal.DefaultUpgradeHeightDelay) shouldUpgrade, version = app.SignalKeeper.ShouldUpgrade(ctx) require.True(t, shouldUpgrade) diff --git a/x/signal/keeper.go b/x/signal/keeper.go index fb915a5e25..302adb4057 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -13,6 +13,11 @@ import ( stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) +// DefaultUpgradeHeightDelay is the number of blocks after a quorum has been +// reached that the chain should upgrade to the new version. Assuming a +// block interval of 12 seconds, this is 48 hours. +const DefaultUpgradeHeightDelay = int64(3 * 7 * 24 * 60 * 60 / 12) // 3 weeks * 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 151,200 blocks. + // Keeper implements the MsgServer and QueryServer interfaces var ( _ types.MsgServer = &Keeper{} @@ -20,11 +25,6 @@ var ( // defaultSignalThreshold is 5/6 or approximately 83.33% defaultSignalThreshold = sdk.NewDec(5).Quo(sdk.NewDec(6)) - - // defaultUpgradeHeightDelay is the number of blocks after a quorum has been - // reached that the chain should upgrade to the new version. Assuming a - // block interval of 12 seconds, this is 48 hours. - defaultUpgradeHeightDelay = int64(3 * 7 * 24 * 60 * 60 / 12) // 3 weeks * 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 151,200 blocks. ) // Threshold is the fraction of voting power that is required @@ -109,7 +109,7 @@ func (k *Keeper) TryUpgrade(ctx context.Context, _ *types.MsgTryUpgrade) (*types } upgrade := types.Upgrade{ AppVersion: version, - UpgradeHeight: sdkCtx.BlockHeader().Height + defaultUpgradeHeightDelay, + UpgradeHeight: sdkCtx.BlockHeader().Height + DefaultUpgradeHeightDelay, } k.setUpgrade(sdkCtx, upgrade) } diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index 6356a80ec8..3cdac5a00a 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -26,8 +26,6 @@ import ( tmdb "github.com/tendermint/tm-db" ) -const defaultUpgradeHeightDelay = int64(3 * 7 * 24 * 60 * 60 / 12) // 3 weeks * 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 151,200 blocks. - func TestGetVotingPowerThreshold(t *testing.T) { bigInt := big.NewInt(0) bigInt.SetString("23058430092136939509", 10) @@ -182,7 +180,7 @@ func TestTallyingLogic(t *testing.T) { require.False(t, shouldUpgrade) // should be false because upgrade height hasn't been reached. require.Equal(t, uint64(0), version) - ctx = ctx.WithBlockHeight(ctx.BlockHeight() + defaultUpgradeHeightDelay) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + signal.DefaultUpgradeHeightDelay) shouldUpgrade, version = upgradeKeeper.ShouldUpgrade(ctx) require.True(t, shouldUpgrade) // should be true because upgrade height has been reached. @@ -413,7 +411,7 @@ func TestGetUpgrade(t *testing.T) { got, err := upgradeKeeper.GetUpgrade(ctx, &types.QueryGetUpgradeRequest{}) require.NoError(t, err) assert.Equal(t, uint64(2), got.Upgrade.AppVersion) - assert.Equal(t, defaultUpgradeHeightDelay, got.Upgrade.UpgradeHeight) + assert.Equal(t, signal.DefaultUpgradeHeightDelay, got.Upgrade.UpgradeHeight) }) } From 01ec9f3d0e8089483f7e5d887b121622e5088719 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Mon, 17 Jun 2024 10:16:14 -0400 Subject: [PATCH 17/22] docs: response will be empty --- proto/celestia/signal/v1/query.proto | 1 + x/signal/types/query.pb.go | 2 ++ 2 files changed, 3 insertions(+) diff --git a/proto/celestia/signal/v1/query.proto b/proto/celestia/signal/v1/query.proto index 3999db7712..2717e9c244 100644 --- a/proto/celestia/signal/v1/query.proto +++ b/proto/celestia/signal/v1/query.proto @@ -16,6 +16,7 @@ service Query { } // GetUpgrade enables a client to query for upgrade information if an upgrade is pending. + // The response will be empty if no upgrade is pending. rpc GetUpgrade(QueryGetUpgradeRequest) returns (QueryGetUpgradeResponse) { option (google.api.http).get = "/signal/v1/upgrade"; diff --git a/x/signal/types/query.pb.go b/x/signal/types/query.pb.go index 63d48752ad..4abc8f93dc 100644 --- a/x/signal/types/query.pb.go +++ b/x/signal/types/query.pb.go @@ -271,6 +271,7 @@ type QueryClient interface { // signalled for a particular version. VersionTally(ctx context.Context, in *QueryVersionTallyRequest, opts ...grpc.CallOption) (*QueryVersionTallyResponse, error) // GetUpgrade enables a client to query for upgrade information if an upgrade is pending. + // The response will be empty if no upgrade is pending. GetUpgrade(ctx context.Context, in *QueryGetUpgradeRequest, opts ...grpc.CallOption) (*QueryGetUpgradeResponse, error) } @@ -306,6 +307,7 @@ type QueryServer interface { // signalled for a particular version. VersionTally(context.Context, *QueryVersionTallyRequest) (*QueryVersionTallyResponse, error) // GetUpgrade enables a client to query for upgrade information if an upgrade is pending. + // The response will be empty if no upgrade is pending. GetUpgrade(context.Context, *QueryGetUpgradeRequest) (*QueryGetUpgradeResponse, error) } From c7a9f7d43646cb168750d4709ea93646be226b3c Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Mon, 17 Jun 2024 10:22:37 -0400 Subject: [PATCH 18/22] refactor: ResetTally --- x/signal/keeper.go | 8 +------- x/signal/keeper_test.go | 28 ++++++++++++++++++++-------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/x/signal/keeper.go b/x/signal/keeper.go index 302adb4057..385ee08ffa 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -224,16 +224,10 @@ func (k *Keeper) ResetTally(ctx sdk.Context) { store := ctx.KVStore(k.storeKey) iterator := store.Iterator(nil, nil) defer iterator.Close() - // delete all signals + // delete the value in the upgrade key and all signals. for ; iterator.Valid(); iterator.Next() { - if bytes.Equal(iterator.Key(), types.UpgradeKey) { - // skip over the upgrade key - continue - } store.Delete(iterator.Key()) } - // delete the upgrade value - store.Delete(types.UpgradeKey) } func VersionToBytes(version uint64) []byte { diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index 3cdac5a00a..e477f0c394 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -313,32 +313,44 @@ func TestThresholdVotingPower(t *testing.T) { } } -// TestResetTally verifies that the VotingPower for all versions is reset to -// zero after calling ResetTally. +// TestResetTally verifies that ResetTally resets the VotingPower for all +// versions to 0 and any pending upgrade is cleared. func TestResetTally(t *testing.T) { upgradeKeeper, ctx, _ := setup(t) - _, err := upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[0].String(), Version: 1}) + _, err := upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[0].String(), Version: 2}) require.NoError(t, err) - resp, err := upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 1}) + resp, err := upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 2}) require.NoError(t, err) assert.Equal(t, uint64(40), resp.VotingPower) - _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[1].String(), Version: 2}) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[1].String(), Version: 3}) require.NoError(t, err) - resp, err = upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 2}) + resp, err = upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 3}) require.NoError(t, err) assert.Equal(t, uint64(1), resp.VotingPower) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[2].String(), Version: 2}) + require.NoError(t, err) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[3].String(), Version: 2}) + require.NoError(t, err) + + _, err = upgradeKeeper.TryUpgrade(ctx, &types.MsgTryUpgrade{}) + require.NoError(t, err) + + assert.True(t, upgradeKeeper.IsUpgradePending(ctx)) + upgradeKeeper.ResetTally(ctx) - resp, err = upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 1}) + resp, err = upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 2}) require.NoError(t, err) assert.Equal(t, uint64(0), resp.VotingPower) - resp, err = upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 2}) + resp, err = upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 3}) require.NoError(t, err) assert.Equal(t, uint64(0), resp.VotingPower) + + assert.False(t, upgradeKeeper.IsUpgradePending(ctx)) } func TestTryUpgrade(t *testing.T) { From 3e791ddd25518cfa34cc5068f0f06026e478babd Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Mon, 17 Jun 2024 11:44:28 -0400 Subject: [PATCH 19/22] refactor: FirstSignalKey --- x/signal/keeper.go | 11 ++--------- x/signal/types/keys.go | 13 ++++++++++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/x/signal/keeper.go b/x/signal/keeper.go index 385ee08ffa..ec8b17d8d1 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -1,7 +1,6 @@ package signal import ( - "bytes" "context" "encoding/binary" @@ -123,12 +122,9 @@ func (k Keeper) VersionTally(ctx context.Context, req *types.QueryVersionTallyRe totalVotingPower := k.stakingKeeper.GetLastTotalPower(sdkCtx) currentVotingPower := sdk.NewInt(0) store := sdkCtx.KVStore(k.storeKey) - iterator := store.Iterator(nil, nil) + iterator := store.Iterator(types.FirstSignalKey, nil) defer iterator.Close() for ; iterator.Valid(); iterator.Next() { - if bytes.Equal(iterator.Key(), types.UpgradeKey) { - continue - } valAddress := sdk.ValAddress(iterator.Key()) power := k.stakingKeeper.GetLastValidatorPower(sdkCtx, valAddress) version := VersionFromBytes(iterator.Value()) @@ -162,12 +158,9 @@ func (k Keeper) DeleteValidatorVersion(ctx sdk.Context, valAddress sdk.ValAddres func (k Keeper) TallyVotingPower(ctx sdk.Context, threshold int64) (bool, uint64) { versionToPower := make(map[uint64]int64) store := ctx.KVStore(k.storeKey) - iterator := store.Iterator(nil, nil) + iterator := store.Iterator(types.FirstSignalKey, nil) defer iterator.Close() for ; iterator.Valid(); iterator.Next() { - if bytes.Equal(iterator.Key(), types.UpgradeKey) { - continue - } valAddress := sdk.ValAddress(iterator.Key()) // check that the validator is still part of the bonded set val, found := k.stakingKeeper.GetValidator(ctx, valAddress) diff --git a/x/signal/types/keys.go b/x/signal/types/keys.go index 19f8f852e3..4568e8a794 100644 --- a/x/signal/types/keys.go +++ b/x/signal/types/keys.go @@ -1,5 +1,12 @@ package types -// UpgradeKey is the key in the signal store used to persist a upgrade if one is -// pending. -var UpgradeKey = []byte{0x00} +var ( + // UpgradeKey is the key in the signal store used to persist a upgrade if one is + // pending. + UpgradeKey = []byte{0x00} + + // FirstSignalKey is used as a divier to separate the UpgradeKey from all + // the keys associated with signals from validators. In practice, this key + // isn't expected to be set or retrieved. + FirstSignalKey = []byte{0x000} +) From 9b81eb3d4f89d5b5d9107b82fcc6e496cd5cd693 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Mon, 17 Jun 2024 15:10:57 -0400 Subject: [PATCH 20/22] feat: decrease DefaultUpgradeHeightDelay to one week --- x/signal/keeper.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/signal/keeper.go b/x/signal/keeper.go index ec8b17d8d1..a8a6632ac8 100644 --- a/x/signal/keeper.go +++ b/x/signal/keeper.go @@ -13,9 +13,9 @@ import ( ) // DefaultUpgradeHeightDelay is the number of blocks after a quorum has been -// reached that the chain should upgrade to the new version. Assuming a -// block interval of 12 seconds, this is 48 hours. -const DefaultUpgradeHeightDelay = int64(3 * 7 * 24 * 60 * 60 / 12) // 3 weeks * 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 151,200 blocks. +// reached that the chain should upgrade to the new version. Assuming a block +// interval of 12 seconds, this is 7 days. +const DefaultUpgradeHeightDelay = int64(7 * 24 * 60 * 60 / 12) // 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 50,400 blocks. // Keeper implements the MsgServer and QueryServer interfaces var ( From 64eb47ba142976bd4bd7133054a17d8582b34ea3 Mon Sep 17 00:00:00 2001 From: Rootul P Date: Wed, 19 Jun 2024 21:41:47 +0200 Subject: [PATCH 21/22] Update x/signal/types/keys.go Co-authored-by: Sanaz Taheri <35961250+staheri14@users.noreply.github.com> --- x/signal/types/keys.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/signal/types/keys.go b/x/signal/types/keys.go index 4568e8a794..c6925cd994 100644 --- a/x/signal/types/keys.go +++ b/x/signal/types/keys.go @@ -5,7 +5,7 @@ var ( // pending. UpgradeKey = []byte{0x00} - // FirstSignalKey is used as a divier to separate the UpgradeKey from all + // FirstSignalKey is used as a divider to separate the UpgradeKey from all // the keys associated with signals from validators. In practice, this key // isn't expected to be set or retrieved. FirstSignalKey = []byte{0x000} From 8e9c37d86a8c5efd3522f4298d3802c124e3c4a2 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Wed, 19 Jun 2024 15:47:12 -0400 Subject: [PATCH 22/22] chore: use version consts --- x/signal/keeper_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index fadbe9cd1e..c63dbdbc0e 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -12,6 +12,8 @@ import ( "github.com/celestiaorg/celestia-app/v2/app" "github.com/celestiaorg/celestia-app/v2/app/encoding" + v1 "github.com/celestiaorg/celestia-app/v2/pkg/appconsts/v1" + v2 "github.com/celestiaorg/celestia-app/v2/pkg/appconsts/v2" "github.com/celestiaorg/celestia-app/v2/x/signal" "github.com/celestiaorg/celestia-app/v2/x/signal/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" @@ -184,7 +186,7 @@ func TestTallyingLogic(t *testing.T) { shouldUpgrade, version = upgradeKeeper.ShouldUpgrade(ctx) require.True(t, shouldUpgrade) // should be true because upgrade height has been reached. - require.Equal(t, uint64(2), version) + require.Equal(t, v2.Version, version) upgradeKeeper.ResetTally(ctx) @@ -257,7 +259,7 @@ func TestCanSkipVersion(t *testing.T) { upgradeKeeper, ctx, _ := setup(t) goCtx := sdk.WrapSDKContext(ctx) - require.Equal(t, uint64(1), ctx.BlockHeader().Version.App) + require.Equal(t, v1.Version, ctx.BlockHeader().Version.App) validators := []sdk.ValAddress{ testutil.ValAddrs[0], @@ -422,7 +424,7 @@ func TestGetUpgrade(t *testing.T) { got, err := upgradeKeeper.GetUpgrade(ctx, &types.QueryGetUpgradeRequest{}) require.NoError(t, err) - assert.Equal(t, uint64(2), got.Upgrade.AppVersion) + assert.Equal(t, v2.Version, got.Upgrade.AppVersion) assert.Equal(t, signal.DefaultUpgradeHeightDelay, got.Upgrade.UpgradeHeight) }) }