From 28f6bc378871b685a5685f53b5aac6dfdbecf0ce Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Mon, 8 Apr 2024 16:33:36 -0400 Subject: [PATCH 1/8] refactor(x/upgrade): reset all versions via ResetTally --- x/upgrade/keeper.go | 12 ++++-------- x/upgrade/keeper_test.go | 29 +++++++++++++++++++++++++++++ x/upgrade/tally_test.go | 3 +-- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/x/upgrade/keeper.go b/x/upgrade/keeper.go index 0852eb14f3..95128d51c9 100644 --- a/x/upgrade/keeper.go +++ b/x/upgrade/keeper.go @@ -180,18 +180,14 @@ func (k *Keeper) ShouldUpgrade() (bool, uint64) { return k.quorumVersion != 0, k.quorumVersion } -// ResetTally resets the tally after a version change. It iterates over the store, -// and deletes any versions that are less than the provided version. It also -// resets the quorumVersion to 0. -func (k *Keeper) ResetTally(ctx sdk.Context, version uint64) { +// ResetTally resets the tally after a version change. It iterates over the +// store and deletes all versions. It also resets the quorumVersion to 0. +func (k *Keeper) ResetTally(ctx sdk.Context) { store := ctx.KVStore(k.storeKey) iterator := store.Iterator(nil, nil) defer iterator.Close() for ; iterator.Valid(); iterator.Next() { - v := VersionFromBytes(iterator.Value()) - if v <= version { - store.Delete(iterator.Key()) - } + store.Delete(iterator.Key()) } k.quorumVersion = 0 } diff --git a/x/upgrade/keeper_test.go b/x/upgrade/keeper_test.go index 44964e950b..6181d9124f 100644 --- a/x/upgrade/keeper_test.go +++ b/x/upgrade/keeper_test.go @@ -7,9 +7,12 @@ import ( "testing" sdkmath "cosmossdk.io/math" + testutil "github.com/celestiaorg/celestia-app/v2/test/util" "github.com/celestiaorg/celestia-app/v2/x/upgrade" + "github.com/celestiaorg/celestia-app/v2/x/upgrade/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetVotingPowerThreshold(t *testing.T) { @@ -57,3 +60,29 @@ func TestGetVotingPowerThreshold(t *testing.T) { }) } } + +// TestResetTally verifies that the VotingPower for all versions is reset to +// zero after calling ResetTally. +func TestResetTally(t *testing.T) { + upgradeKeeper, ctx, _ := setup(t) + + upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[0].String(), Version: 2}) + resp, err := upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 2}) + require.NoError(t, err) + assert.Equal(t, uint64(40), resp.VotingPower) + + upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[1].String(), Version: 3}) + resp, err = upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 3}) + require.NoError(t, err) + assert.Equal(t, uint64(0), resp.VotingPower) + + upgradeKeeper.ResetTally(ctx) + + 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: 3}) + require.NoError(t, err) + assert.Equal(t, uint64(0), resp.VotingPower) +} diff --git a/x/upgrade/tally_test.go b/x/upgrade/tally_test.go index e9289d2f4e..ee555399a2 100644 --- a/x/upgrade/tally_test.go +++ b/x/upgrade/tally_test.go @@ -167,8 +167,7 @@ func TestTallyingLogic(t *testing.T) { }) require.Error(t, err) - // resetting the tally should clear other votes - upgradeKeeper.ResetTally(ctx, 2) + upgradeKeeper.ResetTally(ctx) res, err = upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ Version: 2, }) From 38387a5fa47dd98c5a14a71a20e9f064ae5fc9e3 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Mon, 8 Apr 2024 16:33:52 -0400 Subject: [PATCH 2/8] test: shorten test --- x/upgrade/keeper_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/x/upgrade/keeper_test.go b/x/upgrade/keeper_test.go index 6181d9124f..88e3d339cf 100644 --- a/x/upgrade/keeper_test.go +++ b/x/upgrade/keeper_test.go @@ -71,18 +71,9 @@ func TestResetTally(t *testing.T) { require.NoError(t, err) assert.Equal(t, uint64(40), resp.VotingPower) - upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[1].String(), Version: 3}) - resp, err = upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 3}) - require.NoError(t, err) - assert.Equal(t, uint64(0), resp.VotingPower) - upgradeKeeper.ResetTally(ctx) 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: 3}) - require.NoError(t, err) - assert.Equal(t, uint64(0), resp.VotingPower) } From 5d37f3e7dc1216613a4c34bf07c46543b7d76832 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Mon, 8 Apr 2024 16:37:06 -0400 Subject: [PATCH 3/8] chore(x/upgrade): move tally_test to keeper_test --- x/upgrade/keeper_test.go | 257 +++++++++++++++++++++++++++++++++++++ x/upgrade/tally_test.go | 271 --------------------------------------- 2 files changed, 257 insertions(+), 271 deletions(-) delete mode 100644 x/upgrade/tally_test.go diff --git a/x/upgrade/keeper_test.go b/x/upgrade/keeper_test.go index 88e3d339cf..76eafb412a 100644 --- a/x/upgrade/keeper_test.go +++ b/x/upgrade/keeper_test.go @@ -10,9 +10,16 @@ import ( testutil "github.com/celestiaorg/celestia-app/v2/test/util" "github.com/celestiaorg/celestia-app/v2/x/upgrade" "github.com/celestiaorg/celestia-app/v2/x/upgrade/types" + "github.com/cosmos/cosmos-sdk/store" + storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/libs/log" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + tmversion "github.com/tendermint/tendermint/proto/tendermint/version" + tmdb "github.com/tendermint/tm-db" ) func TestGetVotingPowerThreshold(t *testing.T) { @@ -77,3 +84,253 @@ func TestResetTally(t *testing.T) { require.NoError(t, err) assert.Equal(t, uint64(0), resp.VotingPower) } + +func TestSignalVersion(t *testing.T) { + upgradeKeeper, ctx, _ := setup(t) + goCtx := sdk.WrapSDKContext(ctx) + t.Run("should return an error if the signal version is less than the current version", func(t *testing.T) { + _, err := upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ + ValidatorAddress: testutil.ValAddrs[0].String(), + Version: 0, + }) + assert.Error(t, err) + assert.ErrorIs(t, err, types.ErrInvalidVersion) + }) + t.Run("should return an error if the signal version is greater than the next version", func(t *testing.T) { + _, err := upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ + ValidatorAddress: testutil.ValAddrs[0].String(), + Version: 3, + }) + assert.Error(t, err) + assert.ErrorIs(t, err, types.ErrInvalidVersion) + }) + t.Run("should return an error if the validator was not found", func(t *testing.T) { + _, err := upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ + ValidatorAddress: testutil.ValAddrs[4].String(), + Version: 2, + }) + require.Error(t, err) + require.ErrorIs(t, err, stakingtypes.ErrNoValidatorFound) + }) + t.Run("should not return an error if the signal version and validator are valid", func(t *testing.T) { + _, err := upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ + ValidatorAddress: testutil.ValAddrs[0].String(), + Version: 2, + }) + require.NoError(t, err) + + res, err := upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ + Version: 2, + }) + require.NoError(t, err) + require.EqualValues(t, 40, res.VotingPower) + require.EqualValues(t, 100, res.ThresholdPower) + require.EqualValues(t, 120, res.TotalVotingPower) + }) +} + +func TestTallyingLogic(t *testing.T) { + upgradeKeeper, ctx, mockStakingKeeper := setup(t) + goCtx := sdk.WrapSDKContext(ctx) + _, err := upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ + ValidatorAddress: testutil.ValAddrs[0].String(), + Version: 0, + }) + require.Error(t, err) + _, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ + ValidatorAddress: testutil.ValAddrs[0].String(), + Version: 3, + }) + require.Error(t, err) + + _, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ + ValidatorAddress: testutil.ValAddrs[0].String(), + Version: 2, + }) + require.NoError(t, err) + + res, err := upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ + Version: 2, + }) + require.NoError(t, err) + require.EqualValues(t, 40, res.VotingPower) + require.EqualValues(t, 100, res.ThresholdPower) + require.EqualValues(t, 120, res.TotalVotingPower) + + _, 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, + }) + require.NoError(t, err) + require.EqualValues(t, 99, res.VotingPower) + require.EqualValues(t, 100, res.ThresholdPower) + require.EqualValues(t, 120, res.TotalVotingPower) + + _, err = upgradeKeeper.TryUpgrade(goCtx, &types.MsgTryUpgrade{}) + require.NoError(t, err) + shouldUpgrade, version := upgradeKeeper.ShouldUpgrade() + require.False(t, shouldUpgrade) + require.Equal(t, uint64(0), version) + + // we now have 101/120 + _, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ + ValidatorAddress: testutil.ValAddrs[1].String(), + Version: 2, + }) + require.NoError(t, err) + + _, err = upgradeKeeper.TryUpgrade(goCtx, &types.MsgTryUpgrade{}) + require.NoError(t, err) + shouldUpgrade, version = upgradeKeeper.ShouldUpgrade() + require.True(t, shouldUpgrade) + require.Equal(t, uint64(2), version) + // update the version to 2 + ctx = ctx.WithBlockHeader(tmproto.Header{ + Version: tmversion.Consensus{ + Block: 1, + App: 2, + }, + }) + goCtx = sdk.WrapSDKContext(ctx) + + _, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ + ValidatorAddress: testutil.ValAddrs[0].String(), + Version: 3, + }) + require.NoError(t, err) + + res, err = upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ + Version: 2, + }) + require.NoError(t, err) + require.EqualValues(t, 60, res.VotingPower) + require.EqualValues(t, 100, res.ThresholdPower) + require.EqualValues(t, 120, res.TotalVotingPower) + + // remove one of the validators from the set + delete(mockStakingKeeper.validators, testutil.ValAddrs[1].String()) + // the validator had 1 voting power, so we deduct it from the total + mockStakingKeeper.totalVotingPower = mockStakingKeeper.totalVotingPower.SubRaw(1) + + res, err = upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ + Version: 2, + }) + require.NoError(t, err) + require.EqualValues(t, 59, res.VotingPower) + require.EqualValues(t, 100, res.ThresholdPower) + require.EqualValues(t, 119, res.TotalVotingPower) + + // That validator should not be able to signal a version + _, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ + ValidatorAddress: testutil.ValAddrs[1].String(), + Version: 2, + }) + require.Error(t, err) + + upgradeKeeper.ResetTally(ctx) + res, err = upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ + Version: 2, + }) + require.NoError(t, err) + require.EqualValues(t, 0, res.VotingPower) +} + +func TestEmptyStore(t *testing.T) { + upgradeKeeper, ctx, _ := setup(t) + goCtx := sdk.WrapSDKContext(ctx) + + res, err := upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ + Version: 2, + }) + require.NoError(t, err) + require.EqualValues(t, 0, res.VotingPower) + // 120 is the summation in voting power of the four validators + require.EqualValues(t, 120, res.TotalVotingPower) +} + +func TestThresholdVotingPower(t *testing.T) { + upgradeKeeper, ctx, mockStakingKeeper := setup(t) + + for _, tc := range []struct { + total int64 + threshold int64 + }{ + {total: 1, threshold: 1}, + {total: 2, threshold: 2}, + {total: 3, threshold: 3}, + {total: 6, threshold: 5}, + {total: 59, threshold: 50}, + } { + mockStakingKeeper.totalVotingPower = sdkmath.NewInt(tc.total) + threshold := upgradeKeeper.GetVotingPowerThreshold(ctx) + require.EqualValues(t, tc.threshold, threshold.Int64()) + } +} + +func setup(t *testing.T) (upgrade.Keeper, sdk.Context, *mockStakingKeeper) { + upgradeStore := sdk.NewKVStoreKey(types.StoreKey) + db := tmdb.NewMemDB() + stateStore := store.NewCommitMultiStore(db) + stateStore.MountStoreWithDB(upgradeStore, storetypes.StoreTypeIAVL, nil) + require.NoError(t, stateStore.LoadLatestVersion()) + mockCtx := sdk.NewContext(stateStore, tmproto.Header{ + Version: tmversion.Consensus{ + Block: 1, + App: 1, + }, + }, false, log.NewNopLogger()) + mockStakingKeeper := newMockStakingKeeper( + map[string]int64{ + testutil.ValAddrs[0].String(): 40, + testutil.ValAddrs[1].String(): 1, + testutil.ValAddrs[2].String(): 59, + testutil.ValAddrs[3].String(): 20, + }, + ) + + upgradeKeeper := upgrade.NewKeeper(upgradeStore, mockStakingKeeper) + return upgradeKeeper, mockCtx, mockStakingKeeper +} + +var _ upgrade.StakingKeeper = (*mockStakingKeeper)(nil) + +type mockStakingKeeper struct { + totalVotingPower sdkmath.Int + validators map[string]int64 +} + +func newMockStakingKeeper(validators map[string]int64) *mockStakingKeeper { + totalVotingPower := sdkmath.NewInt(0) + for _, power := range validators { + totalVotingPower = totalVotingPower.AddRaw(power) + } + return &mockStakingKeeper{ + totalVotingPower: totalVotingPower, + validators: validators, + } +} + +func (m *mockStakingKeeper) GetLastTotalPower(_ sdk.Context) sdkmath.Int { + return m.totalVotingPower +} + +func (m *mockStakingKeeper) GetLastValidatorPower(_ sdk.Context, addr sdk.ValAddress) int64 { + addrStr := addr.String() + if power, ok := m.validators[addrStr]; ok { + return power + } + return 0 +} + +func (m *mockStakingKeeper) GetValidator(_ sdk.Context, addr sdk.ValAddress) (validator stakingtypes.Validator, found bool) { + addrStr := addr.String() + if _, ok := m.validators[addrStr]; ok { + return stakingtypes.Validator{Status: stakingtypes.Bonded}, true + } + return stakingtypes.Validator{}, false +} diff --git a/x/upgrade/tally_test.go b/x/upgrade/tally_test.go deleted file mode 100644 index ee555399a2..0000000000 --- a/x/upgrade/tally_test.go +++ /dev/null @@ -1,271 +0,0 @@ -package upgrade_test - -import ( - "testing" - - "cosmossdk.io/math" - "github.com/celestiaorg/celestia-app/v2/x/upgrade" - "github.com/celestiaorg/celestia-app/v2/x/upgrade/types" - "github.com/cosmos/cosmos-sdk/store" - sdk "github.com/cosmos/cosmos-sdk/types" - stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - testutil "github.com/celestiaorg/celestia-app/v2/test/util" - storetypes "github.com/cosmos/cosmos-sdk/store/types" - "github.com/tendermint/tendermint/libs/log" - tmproto "github.com/tendermint/tendermint/proto/tendermint/types" - tmversion "github.com/tendermint/tendermint/proto/tendermint/version" - tmdb "github.com/tendermint/tm-db" -) - -func TestSignalVersion(t *testing.T) { - upgradeKeeper, ctx, _ := setup(t) - goCtx := sdk.WrapSDKContext(ctx) - t.Run("should return an error if the signal version is less than the current version", func(t *testing.T) { - _, err := upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ - ValidatorAddress: testutil.ValAddrs[0].String(), - Version: 0, - }) - assert.Error(t, err) - assert.ErrorIs(t, err, types.ErrInvalidVersion) - }) - t.Run("should return an error if the signal version is greater than the next version", func(t *testing.T) { - _, err := upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ - ValidatorAddress: testutil.ValAddrs[0].String(), - Version: 3, - }) - assert.Error(t, err) - assert.ErrorIs(t, err, types.ErrInvalidVersion) - }) - t.Run("should return an error if the validator was not found", func(t *testing.T) { - _, err := upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ - ValidatorAddress: testutil.ValAddrs[4].String(), - Version: 2, - }) - require.Error(t, err) - require.ErrorIs(t, err, stakingtypes.ErrNoValidatorFound) - }) - t.Run("should not return an error if the signal version and validator are valid", func(t *testing.T) { - _, err := upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ - ValidatorAddress: testutil.ValAddrs[0].String(), - Version: 2, - }) - require.NoError(t, err) - - res, err := upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ - Version: 2, - }) - require.NoError(t, err) - require.EqualValues(t, 40, res.VotingPower) - require.EqualValues(t, 100, res.ThresholdPower) - require.EqualValues(t, 120, res.TotalVotingPower) - }) -} - -func TestTallyingLogic(t *testing.T) { - upgradeKeeper, ctx, mockStakingKeeper := setup(t) - goCtx := sdk.WrapSDKContext(ctx) - _, err := upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ - ValidatorAddress: testutil.ValAddrs[0].String(), - Version: 0, - }) - require.Error(t, err) - _, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ - ValidatorAddress: testutil.ValAddrs[0].String(), - Version: 3, - }) - require.Error(t, err) - - _, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ - ValidatorAddress: testutil.ValAddrs[0].String(), - Version: 2, - }) - require.NoError(t, err) - - res, err := upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ - Version: 2, - }) - require.NoError(t, err) - require.EqualValues(t, 40, res.VotingPower) - require.EqualValues(t, 100, res.ThresholdPower) - require.EqualValues(t, 120, res.TotalVotingPower) - - _, 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, - }) - require.NoError(t, err) - require.EqualValues(t, 99, res.VotingPower) - require.EqualValues(t, 100, res.ThresholdPower) - require.EqualValues(t, 120, res.TotalVotingPower) - - _, err = upgradeKeeper.TryUpgrade(goCtx, &types.MsgTryUpgrade{}) - require.NoError(t, err) - shouldUpgrade, version := upgradeKeeper.ShouldUpgrade() - require.False(t, shouldUpgrade) - require.Equal(t, uint64(0), version) - - // we now have 101/120 - _, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ - ValidatorAddress: testutil.ValAddrs[1].String(), - Version: 2, - }) - require.NoError(t, err) - - _, err = upgradeKeeper.TryUpgrade(goCtx, &types.MsgTryUpgrade{}) - require.NoError(t, err) - shouldUpgrade, version = upgradeKeeper.ShouldUpgrade() - require.True(t, shouldUpgrade) - require.Equal(t, uint64(2), version) - // update the version to 2 - ctx = ctx.WithBlockHeader(tmproto.Header{ - Version: tmversion.Consensus{ - Block: 1, - App: 2, - }, - }) - goCtx = sdk.WrapSDKContext(ctx) - - _, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ - ValidatorAddress: testutil.ValAddrs[0].String(), - Version: 3, - }) - require.NoError(t, err) - - res, err = upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ - Version: 2, - }) - require.NoError(t, err) - require.EqualValues(t, 60, res.VotingPower) - require.EqualValues(t, 100, res.ThresholdPower) - require.EqualValues(t, 120, res.TotalVotingPower) - - // remove one of the validators from the set - delete(mockStakingKeeper.validators, testutil.ValAddrs[1].String()) - // the validator had 1 voting power, so we deduct it from the total - mockStakingKeeper.totalVotingPower = mockStakingKeeper.totalVotingPower.SubRaw(1) - - res, err = upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ - Version: 2, - }) - require.NoError(t, err) - require.EqualValues(t, 59, res.VotingPower) - require.EqualValues(t, 100, res.ThresholdPower) - require.EqualValues(t, 119, res.TotalVotingPower) - - // That validator should not be able to signal a version - _, err = upgradeKeeper.SignalVersion(goCtx, &types.MsgSignalVersion{ - ValidatorAddress: testutil.ValAddrs[1].String(), - Version: 2, - }) - require.Error(t, err) - - upgradeKeeper.ResetTally(ctx) - res, err = upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ - Version: 2, - }) - require.NoError(t, err) - require.EqualValues(t, 0, res.VotingPower) -} - -func TestEmptyStore(t *testing.T) { - upgradeKeeper, ctx, _ := setup(t) - goCtx := sdk.WrapSDKContext(ctx) - - res, err := upgradeKeeper.VersionTally(goCtx, &types.QueryVersionTallyRequest{ - Version: 2, - }) - require.NoError(t, err) - require.EqualValues(t, 0, res.VotingPower) - // 120 is the summation in voting power of the four validators - require.EqualValues(t, 120, res.TotalVotingPower) -} - -func TestThresholdVotingPower(t *testing.T) { - upgradeKeeper, ctx, mockStakingKeeper := setup(t) - - for _, tc := range []struct { - total int64 - threshold int64 - }{ - {total: 1, threshold: 1}, - {total: 2, threshold: 2}, - {total: 3, threshold: 3}, - {total: 6, threshold: 5}, - {total: 59, threshold: 50}, - } { - mockStakingKeeper.totalVotingPower = math.NewInt(tc.total) - threshold := upgradeKeeper.GetVotingPowerThreshold(ctx) - require.EqualValues(t, tc.threshold, threshold.Int64()) - } -} - -func setup(t *testing.T) (upgrade.Keeper, sdk.Context, *mockStakingKeeper) { - upgradeStore := sdk.NewKVStoreKey(types.StoreKey) - db := tmdb.NewMemDB() - stateStore := store.NewCommitMultiStore(db) - stateStore.MountStoreWithDB(upgradeStore, storetypes.StoreTypeIAVL, nil) - require.NoError(t, stateStore.LoadLatestVersion()) - mockCtx := sdk.NewContext(stateStore, tmproto.Header{ - Version: tmversion.Consensus{ - Block: 1, - App: 1, - }, - }, false, log.NewNopLogger()) - mockStakingKeeper := newMockStakingKeeper( - map[string]int64{ - testutil.ValAddrs[0].String(): 40, - testutil.ValAddrs[1].String(): 1, - testutil.ValAddrs[2].String(): 59, - testutil.ValAddrs[3].String(): 20, - }, - ) - - upgradeKeeper := upgrade.NewKeeper(upgradeStore, mockStakingKeeper) - return upgradeKeeper, mockCtx, mockStakingKeeper -} - -var _ upgrade.StakingKeeper = (*mockStakingKeeper)(nil) - -type mockStakingKeeper struct { - totalVotingPower math.Int - validators map[string]int64 -} - -func newMockStakingKeeper(validators map[string]int64) *mockStakingKeeper { - totalVotingPower := math.NewInt(0) - for _, power := range validators { - totalVotingPower = totalVotingPower.AddRaw(power) - } - return &mockStakingKeeper{ - totalVotingPower: totalVotingPower, - validators: validators, - } -} - -func (m *mockStakingKeeper) GetLastTotalPower(_ sdk.Context) math.Int { - return m.totalVotingPower -} - -func (m *mockStakingKeeper) GetLastValidatorPower(_ sdk.Context, addr sdk.ValAddress) int64 { - addrStr := addr.String() - if power, ok := m.validators[addrStr]; ok { - return power - } - return 0 -} - -func (m *mockStakingKeeper) GetValidator(_ sdk.Context, addr sdk.ValAddress) (validator stakingtypes.Validator, found bool) { - addrStr := addr.String() - if _, ok := m.validators[addrStr]; ok { - return stakingtypes.Validator{Status: stakingtypes.Bonded}, true - } - return stakingtypes.Validator{}, false -} From 92a9832b48cd386944c4aee23833420f16f3cb0c Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Thu, 11 Apr 2024 13:34:03 -0400 Subject: [PATCH 4/8] test: add back test --- x/signal/keeper_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index 1a0ba8d87f..cf4c0bd257 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -258,6 +258,32 @@ func TestThresholdVotingPower(t *testing.T) { } } +// TestResetTally verifies that the VotingPower for all versions is reset to +// zero after calling ResetTally. +func TestResetTally(t *testing.T) { + upgradeKeeper, ctx, _ := setup(t) + + upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[0].String(), Version: 1}) + resp, err := upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 1}) + require.NoError(t, err) + assert.Equal(t, uint64(40), resp.VotingPower) + + upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[1].String(), Version: 2}) + resp, err = upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 2}) + require.NoError(t, err) + assert.Equal(t, uint64(1), resp.VotingPower) + + upgradeKeeper.ResetTally(ctx) + + resp, err = upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 1}) + require.NoError(t, err) + assert.Equal(t, uint64(0), resp.VotingPower) + + resp, err = upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 2}) + require.NoError(t, err) + assert.Equal(t, uint64(0), resp.VotingPower) +} + func setup(t *testing.T) (signal.Keeper, sdk.Context, *mockStakingKeeper) { signalStore := sdk.NewKVStoreKey(types.StoreKey) db := tmdb.NewMemDB() From 268f88bf453dccf7a102de49560529bd8228ea13 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Thu, 11 Apr 2024 13:35:58 -0400 Subject: [PATCH 5/8] fix: invoke ResetTally --- app/app.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app/app.go b/app/app.go index d22ac03f97..c86cdeb471 100644 --- a/app/app.go +++ b/app/app.go @@ -711,6 +711,7 @@ func (app *App) Upgrade(ctx sdk.Context, fromVersion, toVersion uint64) error { app.SetInitialAppVersionInConsensusParams(ctx, toVersion) } app.SetAppVersion(ctx, toVersion) + app.SignalKeeper.ResetTally(ctx) return nil } From bd20b72166f80d602402f620526151cc7f05abd2 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Thu, 11 Apr 2024 21:11:58 -0400 Subject: [PATCH 6/8] fix: markdownlint link check --- specs/src/README.md | 2 +- specs/src/specs/state_machine_modules.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/src/README.md b/specs/src/README.md index a2bde6d614..67a2354fa4 100644 --- a/specs/src/README.md +++ b/specs/src/README.md @@ -20,6 +20,6 @@ - [blobstream](https://github.com/celestiaorg/celestia-app/blob/main/x/blobstream/README.md) - [mint](https://github.com/celestiaorg/celestia-app/blob/main/x/mint/README.md) - [paramfilter](https://github.com/celestiaorg/celestia-app/blob/main/x/paramfilter/README.md) - - [upgrade](https://github.com/celestiaorg/celestia-app/blob/main/x/upgrade/README.md) + - [signal](https://github.com/celestiaorg/celestia-app/blob/main/x/signal/README.md) - [tokenfilter](https://github.com/celestiaorg/celestia-app/blob/main/x/tokenfilter/README.md) - [Mainnet Parameters](./specs/params.md) diff --git a/specs/src/specs/state_machine_modules.md b/specs/src/specs/state_machine_modules.md index 4966add7e7..ae6551e131 100644 --- a/specs/src/specs/state_machine_modules.md +++ b/specs/src/specs/state_machine_modules.md @@ -9,7 +9,7 @@ Celestia app is built using the cosmos-sdk, and follows standard cosmos-sdk modu - [minfee](https://github.com/celestiaorg/celestia-app/blob/main/x/minfee/README.md) - [mint](https://github.com/celestiaorg/celestia-app/blob/main/x/mint/README.md) - [paramfilter](https://github.com/celestiaorg/celestia-app/blob/main/x/paramfilter/README.md) -- [upgrade](https://github.com/celestiaorg/celestia-app/blob/main/x/upgrade/README.md) +- [signal](https://github.com/celestiaorg/celestia-app/blob/main/x/signal/README.md) - [tokenfilter](https://github.com/celestiaorg/celestia-app/blob/main/x/tokenfilter/README.md) ## Standard `cosmos-sdk` Modules From b7a5491eec2cb791e43315ffa116f846fd2d00a6 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Fri, 12 Apr 2024 14:45:04 -0400 Subject: [PATCH 7/8] ci: skip pkg cache --- .github/workflows/lint.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index b551b21336..1e9cfad305 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -25,6 +25,7 @@ jobs: version: v1.57.0 args: --timeout 10m github-token: ${{ secrets.github_token }} + skip-pkg-cache: true if: env.GIT_DIFF # hadolint lints the Dockerfile From 41e6ce3eaea9bad2365c08bea7be9a6ca79437d1 Mon Sep 17 00:00:00 2001 From: Rootul Patel Date: Fri, 12 Apr 2024 14:48:58 -0400 Subject: [PATCH 8/8] fix: lint --- x/signal/keeper_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x/signal/keeper_test.go b/x/signal/keeper_test.go index cf4c0bd257..bf2693f09f 100644 --- a/x/signal/keeper_test.go +++ b/x/signal/keeper_test.go @@ -263,12 +263,14 @@ func TestThresholdVotingPower(t *testing.T) { func TestResetTally(t *testing.T) { upgradeKeeper, ctx, _ := setup(t) - upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[0].String(), Version: 1}) + _, err := upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[0].String(), Version: 1}) + require.NoError(t, err) resp, err := upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 1}) require.NoError(t, err) assert.Equal(t, uint64(40), resp.VotingPower) - upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[1].String(), Version: 2}) + _, err = upgradeKeeper.SignalVersion(ctx, &types.MsgSignalVersion{ValidatorAddress: testutil.ValAddrs[1].String(), Version: 2}) + require.NoError(t, err) resp, err = upgradeKeeper.VersionTally(ctx, &types.QueryVersionTallyRequest{Version: 2}) require.NoError(t, err) assert.Equal(t, uint64(1), resp.VotingPower)