Skip to content

Commit

Permalink
x/gov/simulation: fix wrong NewDecodeStore Proposal decoding + proper…
Browse files Browse the repository at this point in the history
… tests (#8603)

The code in NewDecodeStore decoded the wrong proposal due
to a typographical error, but the tests used the exact same
value for the key value pairs hence the typo could never be caught.
I noticed it during an audit of the code, and I've fixed the
tests to pass in varying values for the various key value pairs.

Fixes #8570

Co-authored-by: Alessio Treglia <alessio@tendermint.com>
  • Loading branch information
odeke-em and Alessio Treglia authored Feb 17, 2021
1 parent f970056 commit d56de85
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 25 deletions.
2 changes: 1 addition & 1 deletion x/gov/simulation/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func NewDecodeStore(cdc codec.Marshaler) func(kvA, kvB kv.Pair) string {
panic(err)
}
var proposalB types.Proposal
err = cdc.UnmarshalBinaryBare(kvA.Value, &proposalB)
err = cdc.UnmarshalBinaryBare(kvB.Value, &proposalB)
if err != nil {
panic(err)
}
Expand Down
68 changes: 44 additions & 24 deletions x/gov/simulation/decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,46 +27,66 @@ func TestDecodeStore(t *testing.T) {

endTime := time.Now().UTC()
content := types.ContentFromProposalType("test", "test", types.ProposalTypeText)
proposal, err := types.NewProposal(content, 1, endTime, endTime.Add(24*time.Hour))
proposalA, err := types.NewProposal(content, 1, endTime, endTime.Add(24*time.Hour))
require.NoError(t, err)
proposalB, err := types.NewProposal(content, 2, endTime, endTime.Add(24*time.Hour))
require.NoError(t, err)

proposalIDBz := make([]byte, 8)
binary.LittleEndian.PutUint64(proposalIDBz, 1)
deposit := types.NewDeposit(1, delAddr1, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.OneInt())))
vote := types.NewVote(1, delAddr1, types.NewNonSplitVoteOption(types.OptionYes))

proposalBz, err := cdc.MarshalBinaryBare(&proposal)
proposalBzA, err := cdc.MarshalBinaryBare(&proposalA)
require.NoError(t, err)
proposalBzB, err := cdc.MarshalBinaryBare(&proposalB)
require.NoError(t, err)

kvPairs := kv.Pairs{
Pairs: []kv.Pair{
{Key: types.ProposalKey(1), Value: proposalBz},
{Key: types.InactiveProposalQueueKey(1, endTime), Value: proposalIDBz},
{Key: types.DepositKey(1, delAddr1), Value: cdc.MustMarshalBinaryBare(&deposit)},
{Key: types.VoteKey(1, delAddr1), Value: cdc.MustMarshalBinaryBare(&vote)},
{Key: []byte{0x99}, Value: []byte{0x99}},
},
}

tests := []struct {
name string
kvA, kvB kv.Pair
expectedLog string
wantPanic bool
}{
{"proposals", fmt.Sprintf("%v\n%v", proposal, proposal)},
{"proposal IDs", "proposalIDA: 1\nProposalIDB: 1"},
{"deposits", fmt.Sprintf("%v\n%v", deposit, deposit)},
{"votes", fmt.Sprintf("%v\n%v", vote, vote)},
{"other", ""},
{
"proposals",
kv.Pair{Key: types.ProposalKey(1), Value: proposalBzA},
kv.Pair{Key: types.ProposalKey(2), Value: proposalBzB},
fmt.Sprintf("%v\n%v", proposalA, proposalB), false,
},
{
"proposal IDs",
kv.Pair{Key: types.InactiveProposalQueueKey(1, endTime), Value: proposalIDBz},
kv.Pair{Key: types.InactiveProposalQueueKey(1, endTime), Value: proposalIDBz},
"proposalIDA: 1\nProposalIDB: 1", false,
},
{
"deposits",
kv.Pair{Key: types.DepositKey(1, delAddr1), Value: cdc.MustMarshalBinaryBare(&deposit)},
kv.Pair{Key: types.DepositKey(1, delAddr1), Value: cdc.MustMarshalBinaryBare(&deposit)},
fmt.Sprintf("%v\n%v", deposit, deposit), false,
},
{
"votes",
kv.Pair{Key: types.VoteKey(1, delAddr1), Value: cdc.MustMarshalBinaryBare(&vote)},
kv.Pair{Key: types.VoteKey(1, delAddr1), Value: cdc.MustMarshalBinaryBare(&vote)},
fmt.Sprintf("%v\n%v", vote, vote), false,
},
{
"other",
kv.Pair{Key: []byte{0x99}, Value: []byte{0x99}},
kv.Pair{Key: []byte{0x99}, Value: []byte{0x99}},
"", true,
},
}

for i, tt := range tests {
i, tt := i, tt
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
switch i {
case len(tests) - 1:
require.Panics(t, func() { dec(kvPairs.Pairs[i], kvPairs.Pairs[i]) }, tt.name)
default:
require.Equal(t, tt.expectedLog, dec(kvPairs.Pairs[i], kvPairs.Pairs[i]), tt.name)
if tt.wantPanic {
require.Panics(t, func() { dec(tt.kvA, tt.kvB) }, tt.name)
} else {
require.Equal(t, tt.expectedLog, dec(tt.kvA, tt.kvB), tt.name)
}
})
}
Expand Down

0 comments on commit d56de85

Please sign in to comment.