diff --git a/x/gov/keeper/common_test.go b/x/gov/keeper/common_test.go index 85f4b198919e..2d667dc8e841 100644 --- a/x/gov/keeper/common_test.go +++ b/x/gov/keeper/common_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" + corestoretypes "cosmossdk.io/core/store" "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/baseapp" @@ -57,6 +58,7 @@ func setupGovKeeper(t *testing.T) ( *govtestutil.MockBankKeeper, *govtestutil.MockStakingKeeper, *govtestutil.MockDistributionKeeper, + corestoretypes.KVStoreService, moduletestutil.TestEncodingConfig, sdk.Context, ) { @@ -115,7 +117,7 @@ func setupGovKeeper(t *testing.T) ( v1.RegisterMsgServer(msr, keeper.NewMsgServerImpl(govKeeper)) banktypes.RegisterMsgServer(msr, nil) // Nil is fine here as long as we never execute the proposal's Msgs. - return govKeeper, acctKeeper, bankKeeper, stakingKeeper, distributionKeeper, encCfg, ctx + return govKeeper, acctKeeper, bankKeeper, stakingKeeper, distributionKeeper, storeService, encCfg, ctx } // trackMockBalances sets up expected calls on the Mock BankKeeper, and also diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index 2ccc09954ca0..c0c75d220651 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -38,7 +38,7 @@ func TestDeposits(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - govKeeper, authKeeper, bankKeeper, stakingKeeper, distKeeper, _, ctx := setupGovKeeper(t) + govKeeper, authKeeper, bankKeeper, stakingKeeper, distKeeper, _, _, ctx := setupGovKeeper(t) trackMockBalances(bankKeeper, distKeeper) // With expedited proposals the minimum deposit is higher, so we must @@ -207,7 +207,7 @@ func TestDepositAmount(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - govKeeper, authKeeper, bankKeeper, stakingKeeper, distrKeeper, _, ctx := setupGovKeeper(t) + govKeeper, authKeeper, bankKeeper, stakingKeeper, distrKeeper, _, _, ctx := setupGovKeeper(t) trackMockBalances(bankKeeper, distrKeeper) testAddrs := simtestutil.AddTestAddrsIncremental(bankKeeper, stakingKeeper, ctx, 2, sdkmath.NewInt(1000000000000000)) @@ -324,7 +324,7 @@ func TestValidateInitialDeposit(t *testing.T) { for name, tc := range testcases { t.Run(name, func(t *testing.T) { - govKeeper, _, _, _, _, _, ctx := setupGovKeeper(t) + govKeeper, _, _, _, _, _, _, ctx := setupGovKeeper(t) params := v1.DefaultParams() if tc.expedited { @@ -387,7 +387,7 @@ func TestChargeDeposit(t *testing.T) { } t.Run(testName(i), func(t *testing.T) { - govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, ctx := setupGovKeeper(t) + govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, _, ctx := setupGovKeeper(t) params := v1.DefaultParams() params.ProposalCancelRatio = tc.proposalCancelRatio TestAddrs := simtestutil.AddTestAddrsIncremental(bankKeeper, stakingKeeper, ctx, 2, sdkmath.NewInt(10000000000)) diff --git a/x/gov/keeper/genesis_test.go b/x/gov/keeper/genesis_test.go index c39048819cc7..d1978617f5d8 100644 --- a/x/gov/keeper/genesis_test.go +++ b/x/gov/keeper/genesis_test.go @@ -1,12 +1,19 @@ package keeper_test import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" sdkmath "cosmossdk.io/math" + "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov/keeper" + "github.com/cosmos/cosmos-sdk/x/gov/types" v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" ) @@ -34,3 +41,113 @@ func (suite *KeeperTestSuite) TestImportExportQueues_ErrorInconsistentState() { suite.Require().NoError(err) suite.Require().Equal(genState, v1.DefaultGenesisState()) } + +// TestInitGenesis_PanicsWhenDistrCancelDestSetWithoutDistrKeeper covers the +// guard added in #25616 that requires a non-nil distribution keeper whenever +// the distribution module address is configured as the proposal cancel +// destination. +func TestInitGenesis_PanicsWhenDistrCancelDestSetWithoutDistrKeeper(t *testing.T) { + _, acctKeeper, bankKeeper, stakingKeeper, _, storeService, encCfg, ctx := setupGovKeeper(t) + + // Build a separate keeper sharing the same dependencies but with a nil + // distribution keeper, so we can hit the guard without disturbing the + // suite-wide keeper. + govKeeperNoDistr := keeper.NewKeeper( + encCfg.Codec, + storeService, + acctKeeper, + bankKeeper, + nil, + baseapp.NewMsgServiceRouter(), + types.DefaultConfig(), + govAcct.String(), + keeper.NewDefaultCalculateVoteResultsAndVotingPower(stakingKeeper), + ) + + params := v1.DefaultParams() + params.ProposalCancelDest = distAcct.String() + + expected := fmt.Sprintf( + "must set DistrKeeper first if using distribution module (%s) as proposal cancel destination", + distAcct.String(), + ) + require.PanicsWithValue(t, expected, func() { + keeper.InitGenesis(ctx, acctKeeper, bankKeeper, govKeeperNoDistr, &v1.GenesisState{ + StartingProposalId: 1, + Params: ¶ms, + }) + }) +} + +// TestInitGenesis_RoundTripProposalsAndVotes verifies that proposals in both +// deposit and voting period statuses, plus a vote, survive an InitGenesis → +// ExportGenesis round-trip. +func (suite *KeeperTestSuite) TestInitGenesis_RoundTripProposalsAndVotes() { + suite.reset() + suite.acctKeeper.EXPECT().SetModuleAccount(suite.ctx, gomock.Any()).AnyTimes() + + blockTime := suite.ctx.BlockHeader().Time + depositEnd := blockTime.Add(time.Hour) + votingEnd := blockTime.Add(2 * time.Hour) + + proposalA := &v1.Proposal{ + Id: 1, + Status: v1.StatusDepositPeriod, + SubmitTime: &blockTime, + DepositEndTime: &depositEnd, + Title: "proposal A", + Summary: "summary A", + Proposer: suite.addrs[0].String(), + } + proposalB := &v1.Proposal{ + Id: 2, + Status: v1.StatusVotingPeriod, + SubmitTime: &blockTime, + DepositEndTime: &depositEnd, + VotingStartTime: &blockTime, + VotingEndTime: &votingEnd, + Title: "proposal B", + Summary: "summary B", + Proposer: suite.addrs[1].String(), + } + vote := &v1.Vote{ + ProposalId: 2, + Voter: suite.addrs[0].String(), + Options: v1.NewNonSplitVoteOption(v1.OptionYes), + } + + params := v1.DefaultParams() + state := &v1.GenesisState{ + StartingProposalId: 3, + Params: ¶ms, + Constitution: "test constitution", + Proposals: v1.Proposals{proposalA, proposalB}, + Votes: v1.Votes{vote}, + } + + suite.Require().NotPanics(func() { + keeper.InitGenesis(suite.ctx, suite.acctKeeper, suite.bankKeeper, suite.govKeeper, state) + }) + + exported, err := keeper.ExportGenesis(suite.ctx, suite.govKeeper) + suite.Require().NoError(err) + + suite.Require().Equal("test constitution", exported.Constitution) + suite.Require().Equal(uint64(3), exported.StartingProposalId) + + // Both proposals should be exported, preserving their IDs and statuses. + suite.Require().Len(exported.Proposals, 2) + exportedByID := make(map[uint64]*v1.Proposal, len(exported.Proposals)) + for _, p := range exported.Proposals { + exportedByID[p.Id] = p + } + suite.Require().Contains(exportedByID, uint64(1)) + suite.Require().Equal(v1.StatusDepositPeriod, exportedByID[1].Status) + suite.Require().Contains(exportedByID, uint64(2)) + suite.Require().Equal(v1.StatusVotingPeriod, exportedByID[2].Status) + + // The single vote should round-trip. + suite.Require().Len(exported.Votes, 1) + suite.Require().Equal(uint64(2), exported.Votes[0].ProposalId) + suite.Require().Equal(suite.addrs[0].String(), exported.Votes[0].Voter) +} diff --git a/x/gov/keeper/hooks_test.go b/x/gov/keeper/hooks_test.go index 17ebf5e47ced..e1ff3face1e7 100644 --- a/x/gov/keeper/hooks_test.go +++ b/x/gov/keeper/hooks_test.go @@ -54,7 +54,7 @@ func (h *MockGovHooksReceiver) AfterProposalVotingPeriodEnded(ctx context.Contex func TestHooks(t *testing.T) { minDeposit := v1.DefaultParams().MinDeposit - govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, ctx := setupGovKeeper(t) + govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, _, ctx := setupGovKeeper(t) addrs := simtestutil.AddTestAddrs(bankKeeper, stakingKeeper, ctx, 1, minDeposit[0].Amount) authKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() diff --git a/x/gov/keeper/keeper_test.go b/x/gov/keeper/keeper_test.go index 748ad89de796..1ef1452b7cb4 100644 --- a/x/gov/keeper/keeper_test.go +++ b/x/gov/keeper/keeper_test.go @@ -46,7 +46,7 @@ func (suite *KeeperTestSuite) SetupSuite() { } func (suite *KeeperTestSuite) reset() { - govKeeper, acctKeeper, bankKeeper, stakingKeeper, distKeeper, encCfg, ctx := setupGovKeeper(suite.T()) + govKeeper, acctKeeper, bankKeeper, stakingKeeper, distKeeper, _, encCfg, ctx := setupGovKeeper(suite.T()) // Populate the gov account with some coins, as the TestProposal we have // is a MsgSend from the gov account. @@ -81,7 +81,7 @@ func (suite *KeeperTestSuite) reset() { } func TestIncrementProposalNumber(t *testing.T) { - govKeeper, authKeeper, _, _, _, _, ctx := setupGovKeeper(t) + govKeeper, authKeeper, _, _, _, _, _, ctx := setupGovKeeper(t) authKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() @@ -107,7 +107,7 @@ func TestIncrementProposalNumber(t *testing.T) { } func TestProposalQueues(t *testing.T) { - govKeeper, authKeeper, _, _, _, _, ctx := setupGovKeeper(t) + govKeeper, authKeeper, _, _, _, _, _, ctx := setupGovKeeper(t) ac := address.NewBech32Codec("cosmos") addrBz, err := ac.StringToBytes(address1) @@ -134,7 +134,7 @@ func TestProposalQueues(t *testing.T) { } func TestSetHooks(t *testing.T) { - govKeeper, _, _, _, _, _, _ := setupGovKeeper(t) + govKeeper, _, _, _, _, _, _, _ := setupGovKeeper(t) require.Empty(t, govKeeper.Hooks()) govHooksReceiver := MockGovHooksReceiver{} @@ -146,7 +146,7 @@ func TestSetHooks(t *testing.T) { } func TestGetGovGovernanceAndModuleAccountAddress(t *testing.T) { - govKeeper, authKeeper, _, _, _, _, ctx := setupGovKeeper(t) + govKeeper, authKeeper, _, _, _, _, _, ctx := setupGovKeeper(t) mAcc := authKeeper.GetModuleAccount(ctx, "gov") require.Equal(t, mAcc, govKeeper.GetGovernanceAccount(ctx)) mAddr := authKeeper.GetModuleAddress("gov") diff --git a/x/gov/keeper/tally_test.go b/x/gov/keeper/tally_test.go index dd2600882ee0..47df539e379e 100644 --- a/x/gov/keeper/tally_test.go +++ b/x/gov/keeper/tally_test.go @@ -14,7 +14,7 @@ import ( ) func TestVoteRemovalAfterTally(t *testing.T) { - govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, ctx := setupGovKeeper(t) + govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, _, ctx := setupGovKeeper(t) authKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() stakingKeeper.EXPECT().ValidatorAddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() @@ -59,7 +59,7 @@ func TestVoteRemovalAfterTally(t *testing.T) { // TestMultipleProposalsVoteRemoval verifies that votes for one proposal are removed // while votes for another proposal are preserved during tallying func TestMultipleProposalsVoteRemoval(t *testing.T) { - govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, ctx := setupGovKeeper(t) + govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, _, ctx := setupGovKeeper(t) authKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() stakingKeeper.EXPECT().ValidatorAddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() diff --git a/x/gov/keeper/vote_test.go b/x/gov/keeper/vote_test.go index 21186dd679bf..5f3fecb3d8b2 100644 --- a/x/gov/keeper/vote_test.go +++ b/x/gov/keeper/vote_test.go @@ -15,7 +15,7 @@ import ( ) func TestVotes(t *testing.T) { - govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, ctx := setupGovKeeper(t) + govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, _, ctx := setupGovKeeper(t) addrs := simtestutil.AddTestAddrsIncremental(bankKeeper, stakingKeeper, ctx, 2, sdkmath.NewInt(10000000)) authKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes()