Skip to content

Commit 52a0f44

Browse files
MSalopekMSalopek
authored
fix!: check v1 and v1beta1 MsgVote in VoteSpamDecorator (#2923)
* fix!: check v1 and v1beta1 MsgVote in VoteSpamDecorator #2912 introuced the VoteSpamDecorator. The implementation was not complete and only v1beta1 gov types were handled. The CLI supports v1 gov types so the validation was not sufficient. This commit expands the antehandler validation to allow correct processing of both v1 and v1beta1 messages. * appease linter * test: stop app from failing non-determinism test on gov vote * ante: handle edgecase with 0 minStakedTokens * appease linter
1 parent 382301c commit 52a0f44

File tree

3 files changed

+190
-32
lines changed

3 files changed

+190
-32
lines changed

‎ante/gov_vote_ante.go

+50-25
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"github.com/cosmos/cosmos-sdk/codec"
77
sdk "github.com/cosmos/cosmos-sdk/types"
88
"github.com/cosmos/cosmos-sdk/x/authz"
9+
govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
910
govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
1011
stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
1112
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
@@ -18,6 +19,12 @@ var (
1819
maxDelegationsChecked = 100 // number of delegation to check for the minStakedTokens
1920
)
2021

22+
// SetMinStakedTokens sets the minimum amount of staked tokens required to vote
23+
// Should only be used in testing
24+
func SetMinStakedTokens(tokens sdk.Dec) {
25+
minStakedTokens = tokens
26+
}
27+
2128
type GovVoteDecorator struct {
2229
stakingKeeper *stakingkeeper.Keeper
2330
cdc codec.BinaryCodec
@@ -50,36 +57,54 @@ func (g GovVoteDecorator) AnteHandle(
5057
// ValidateVoteMsgs checks if a voter has enough stake to vote
5158
func (g GovVoteDecorator) ValidateVoteMsgs(ctx sdk.Context, msgs []sdk.Msg) error {
5259
validMsg := func(m sdk.Msg) error {
53-
if msg, ok := m.(*govv1beta1.MsgVote); ok {
54-
accAddr, err := sdk.AccAddressFromBech32(msg.Voter)
60+
var accAddr sdk.AccAddress
61+
var err error
62+
63+
switch msg := m.(type) {
64+
case *govv1beta1.MsgVote:
65+
accAddr, err = sdk.AccAddressFromBech32(msg.Voter)
5566
if err != nil {
5667
return err
5768
}
58-
enoughStake := false
59-
delegationCount := 0
60-
stakedTokens := sdk.NewDec(0)
61-
g.stakingKeeper.IterateDelegatorDelegations(ctx, accAddr, func(delegation stakingtypes.Delegation) bool {
62-
validatorAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress)
63-
if err != nil {
64-
panic(err) // shouldn't happen
65-
}
66-
validator, found := g.stakingKeeper.GetValidator(ctx, validatorAddr)
67-
if found {
68-
shares := delegation.Shares
69-
tokens := validator.TokensFromSharesTruncated(shares)
70-
stakedTokens = stakedTokens.Add(tokens)
71-
if stakedTokens.GTE(minStakedTokens) {
72-
enoughStake = true
73-
return true // break the iteration
74-
}
69+
case *govv1.MsgVote:
70+
accAddr, err = sdk.AccAddressFromBech32(msg.Voter)
71+
if err != nil {
72+
return err
73+
}
74+
default:
75+
// not a vote message - nothing to validate
76+
return nil
77+
}
78+
79+
if minStakedTokens.IsZero() {
80+
return nil
81+
}
82+
83+
enoughStake := false
84+
delegationCount := 0
85+
stakedTokens := sdk.NewDec(0)
86+
g.stakingKeeper.IterateDelegatorDelegations(ctx, accAddr, func(delegation stakingtypes.Delegation) bool {
87+
validatorAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress)
88+
if err != nil {
89+
panic(err) // shouldn't happen
90+
}
91+
validator, found := g.stakingKeeper.GetValidator(ctx, validatorAddr)
92+
if found {
93+
shares := delegation.Shares
94+
tokens := validator.TokensFromSharesTruncated(shares)
95+
stakedTokens = stakedTokens.Add(tokens)
96+
if stakedTokens.GTE(minStakedTokens) {
97+
enoughStake = true
98+
return true // break the iteration
7599
}
76-
delegationCount++
77-
// break the iteration if maxDelegationsChecked were already checked
78-
return delegationCount >= maxDelegationsChecked
79-
})
80-
if !enoughStake {
81-
return errorsmod.Wrapf(gaiaerrors.ErrInsufficientStake, "insufficient stake for voting - min required %v", minStakedTokens)
82100
}
101+
delegationCount++
102+
// break the iteration if maxDelegationsChecked were already checked
103+
return delegationCount >= maxDelegationsChecked
104+
})
105+
106+
if !enoughStake {
107+
return errorsmod.Wrapf(gaiaerrors.ErrInsufficientStake, "insufficient stake for voting - min required %v", minStakedTokens)
83108
}
84109

85110
return nil

‎ante/gov_vote_ante_test.go

+133-7
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,17 @@ import (
1111

1212
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
1313
sdk "github.com/cosmos/cosmos-sdk/types"
14+
govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
1415
govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
1516
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
1617

1718
"github.com/cosmos/gaia/v15/ante"
1819
"github.com/cosmos/gaia/v15/app/helpers"
1920
)
2021

21-
func TestVoteSpamDecorator(t *testing.T) {
22+
// Test that the GovVoteDecorator rejects v1beta1 vote messages from accounts with less than 1 atom staked
23+
// Submitting v1beta1.VoteMsg should not be possible through the CLI, but it's still possible to craft a transaction
24+
func TestVoteSpamDecoratorGovV1Beta1(t *testing.T) {
2225
gaiaApp := helpers.Setup(t)
2326
ctx := gaiaApp.NewUncachedContext(true, tmproto.Header{})
2427
decorator := ante.NewGovVoteDecorator(gaiaApp.AppCodec(), gaiaApp.StakingKeeper)
@@ -56,6 +59,12 @@ func TestVoteSpamDecorator(t *testing.T) {
5659
validators []sdk.ValAddress
5760
expectPass bool
5861
}{
62+
{
63+
name: "delegate 0 atom",
64+
bondAmt: sdk.ZeroInt(),
65+
validators: []sdk.ValAddress{valAddr1},
66+
expectPass: false,
67+
},
5968
{
6069
name: "delegate 0.1 atom",
6170
bondAmt: sdk.NewInt(100000),
@@ -97,12 +106,14 @@ func TestVoteSpamDecorator(t *testing.T) {
97106
}
98107

99108
// Delegate tokens
100-
amt := tc.bondAmt.Quo(sdk.NewInt(int64(len(tc.validators))))
101-
for _, valAddr := range tc.validators {
102-
val, found := stakingKeeper.GetValidator(ctx, valAddr)
103-
require.True(t, found)
104-
_, err := stakingKeeper.Delegate(ctx, delegator, amt, stakingtypes.Unbonded, val, true)
105-
require.NoError(t, err)
109+
if !tc.bondAmt.IsZero() {
110+
amt := tc.bondAmt.Quo(sdk.NewInt(int64(len(tc.validators))))
111+
for _, valAddr := range tc.validators {
112+
val, found := stakingKeeper.GetValidator(ctx, valAddr)
113+
require.True(t, found)
114+
_, err := stakingKeeper.Delegate(ctx, delegator, amt, stakingtypes.Unbonded, val, true)
115+
require.NoError(t, err)
116+
}
106117
}
107118

108119
// Create vote message
@@ -121,3 +132,118 @@ func TestVoteSpamDecorator(t *testing.T) {
121132
}
122133
}
123134
}
135+
136+
// Test that the GovVoteDecorator rejects v1 vote messages from accounts with less than 1 atom staked
137+
// Usually, only v1.VoteMsg can be submitted using the CLI.
138+
func TestVoteSpamDecoratorGovV1(t *testing.T) {
139+
gaiaApp := helpers.Setup(t)
140+
ctx := gaiaApp.NewUncachedContext(true, tmproto.Header{})
141+
decorator := ante.NewGovVoteDecorator(gaiaApp.AppCodec(), gaiaApp.StakingKeeper)
142+
stakingKeeper := gaiaApp.StakingKeeper
143+
144+
// Get validator
145+
valAddr1 := stakingKeeper.GetAllValidators(ctx)[0].GetOperator()
146+
147+
// Create one more validator
148+
pk := ed25519.GenPrivKeyFromSecret([]byte{uint8(13)}).PubKey()
149+
validator2, err := stakingtypes.NewValidator(
150+
sdk.ValAddress(pk.Address()),
151+
pk,
152+
stakingtypes.Description{},
153+
)
154+
valAddr2 := validator2.GetOperator()
155+
require.NoError(t, err)
156+
// Make sure the validator is bonded so it's not removed on Undelegate
157+
validator2.Status = stakingtypes.Bonded
158+
stakingKeeper.SetValidator(ctx, validator2)
159+
err = stakingKeeper.SetValidatorByConsAddr(ctx, validator2)
160+
require.NoError(t, err)
161+
stakingKeeper.SetNewValidatorByPowerIndex(ctx, validator2)
162+
err = stakingKeeper.Hooks().AfterValidatorCreated(ctx, validator2.GetOperator())
163+
require.NoError(t, err)
164+
165+
// Get delegator (this account was created during setup)
166+
addr := gaiaApp.AccountKeeper.GetAccountAddressByID(ctx, 0)
167+
delegator, err := sdk.AccAddressFromBech32(addr)
168+
require.NoError(t, err)
169+
170+
tests := []struct {
171+
name string
172+
bondAmt math.Int
173+
validators []sdk.ValAddress
174+
expectPass bool
175+
}{
176+
{
177+
name: "delegate 0 atom",
178+
bondAmt: sdk.ZeroInt(),
179+
validators: []sdk.ValAddress{valAddr1},
180+
expectPass: false,
181+
},
182+
{
183+
name: "delegate 0.1 atom",
184+
bondAmt: sdk.NewInt(100000),
185+
validators: []sdk.ValAddress{valAddr1},
186+
expectPass: false,
187+
},
188+
{
189+
name: "delegate 1 atom",
190+
bondAmt: sdk.NewInt(1000000),
191+
validators: []sdk.ValAddress{valAddr1},
192+
expectPass: true,
193+
},
194+
{
195+
name: "delegate 1 atom to two validators",
196+
bondAmt: sdk.NewInt(1000000),
197+
validators: []sdk.ValAddress{valAddr1, valAddr2},
198+
expectPass: true,
199+
},
200+
{
201+
name: "delegate 0.9 atom to two validators",
202+
bondAmt: sdk.NewInt(900000),
203+
validators: []sdk.ValAddress{valAddr1, valAddr2},
204+
expectPass: false,
205+
},
206+
{
207+
name: "delegate 10 atom",
208+
bondAmt: sdk.NewInt(10000000),
209+
validators: []sdk.ValAddress{valAddr1},
210+
expectPass: true,
211+
},
212+
}
213+
214+
for _, tc := range tests {
215+
// Unbond all tokens for this delegator
216+
delegations := stakingKeeper.GetAllDelegatorDelegations(ctx, delegator)
217+
for _, del := range delegations {
218+
_, err := stakingKeeper.Undelegate(ctx, delegator, del.GetValidatorAddr(), del.GetShares())
219+
require.NoError(t, err)
220+
}
221+
222+
// Delegate tokens
223+
if !tc.bondAmt.IsZero() {
224+
amt := tc.bondAmt.Quo(sdk.NewInt(int64(len(tc.validators))))
225+
for _, valAddr := range tc.validators {
226+
val, found := stakingKeeper.GetValidator(ctx, valAddr)
227+
require.True(t, found)
228+
_, err := stakingKeeper.Delegate(ctx, delegator, amt, stakingtypes.Unbonded, val, true)
229+
require.NoError(t, err)
230+
}
231+
}
232+
233+
// Create vote message
234+
msg := govv1.NewMsgVote(
235+
delegator,
236+
0,
237+
govv1.VoteOption_VOTE_OPTION_YES,
238+
"new-v1-vote-message-test",
239+
)
240+
241+
// Validate vote message
242+
err := decorator.ValidateVoteMsgs(ctx, []sdk.Msg{msg})
243+
if tc.expectPass {
244+
require.NoError(t, err, "expected %v to pass", tc.name)
245+
} else {
246+
require.Error(t, err, "expected %v to fail", tc.name)
247+
}
248+
}
249+
}

‎app/sim_test.go

+7
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
dbm "github.com/cometbft/cometbft-db"
1313
"github.com/cometbft/cometbft/libs/log"
1414

15+
"cosmossdk.io/math"
16+
1517
"github.com/cosmos/cosmos-sdk/baseapp"
1618
"github.com/cosmos/cosmos-sdk/client/flags"
1719
"github.com/cosmos/cosmos-sdk/server"
@@ -21,6 +23,7 @@ import (
2123
"github.com/cosmos/cosmos-sdk/x/simulation"
2224
simcli "github.com/cosmos/cosmos-sdk/x/simulation/client/cli"
2325

26+
"github.com/cosmos/gaia/v15/ante"
2427
gaia "github.com/cosmos/gaia/v15/app"
2528
// "github.com/cosmos/gaia/v11/app/helpers"
2629
// "github.com/cosmos/gaia/v11/app/params"
@@ -97,6 +100,10 @@ func TestAppStateDeterminism(t *testing.T) {
97100
baseapp.SetChainID(AppChainID),
98101
)
99102

103+
// NOTE: setting to zero to avoid failing the simulation
104+
// due to the minimum staked tokens required to submit a vote
105+
ante.SetMinStakedTokens(math.LegacyZeroDec())
106+
100107
fmt.Printf(
101108
"running non-determinism simulation; seed %d: %d/%d, attempt: %d/%d\n",
102109
config.Seed, i+1, numSeeds, j+1, numTimesToRunPerSeed,

0 commit comments

Comments
 (0)