From 4a4ba8f442c2ee8c1659230eca1726ede5295a6d Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Thu, 29 Sep 2022 02:24:54 +0000 Subject: [PATCH 1/4] Fix bugs on abortOldProposals() --- x/foundation/keeper/proposal.go | 2 +- x/foundation/keeper/proposal_test.go | 58 ++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/x/foundation/keeper/proposal.go b/x/foundation/keeper/proposal.go index 7fb93cdc1f..71529a7c44 100644 --- a/x/foundation/keeper/proposal.go +++ b/x/foundation/keeper/proposal.go @@ -130,7 +130,7 @@ func (k Keeper) abortOldProposals(ctx sdk.Context) { latestVersion := k.GetFoundationInfo(ctx).Version k.iterateProposals(ctx, func(proposal foundation.Proposal) (stop bool) { - if proposal.FoundationVersion != latestVersion-1 { + if proposal.FoundationVersion == latestVersion { return true } diff --git a/x/foundation/keeper/proposal_test.go b/x/foundation/keeper/proposal_test.go index 3ea8dc12d3..4be1dce60f 100644 --- a/x/foundation/keeper/proposal_test.go +++ b/x/foundation/keeper/proposal_test.go @@ -1,6 +1,13 @@ package keeper_test import ( + "testing" + + ocproto "github.com/line/ostracon/proto/ostracon/types" + "github.com/stretchr/testify/require" + + "github.com/line/lbm-sdk/crypto/keys/secp256k1" + "github.com/line/lbm-sdk/simapp" sdk "github.com/line/lbm-sdk/types" "github.com/line/lbm-sdk/x/foundation" govtypes "github.com/line/lbm-sdk/x/gov/types" @@ -92,3 +99,54 @@ func (s *KeeperTestSuite) TestWithdrawProposal() { }) } } + +func TestAbortProposal(t *testing.T) { + checkTx := false + app := simapp.Setup(checkTx) + ctx := app.BaseApp.NewContext(checkTx, ocproto.Header{}) + keeper := app.FoundationKeeper + + createAddress := func() sdk.AccAddress { + return sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + } + + operator := keeper.GetOperator(ctx) + + members := make([]sdk.AccAddress, 10) + for i := range members { + members[i] = createAddress() + } + err := keeper.UpdateMembers(ctx, []foundation.Member{ + { + Address: members[0].String(), + Participating: true, + }, + }) + require.NoError(t, err) + + stranger := createAddress() + + // create proposals of different versions and abort them + for _, newMember := range members[1:] { + _, err := keeper.SubmitProposal(ctx, []string{members[0].String()}, "", []sdk.Msg{ + &foundation.MsgWithdrawFromTreasury{ + Operator: operator.String(), + To: stranger.String(), + Amount: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.OneInt())), + }, + }) + require.NoError(t, err) + + err = keeper.UpdateMembers(ctx, []foundation.Member{ + { + Address: newMember.String(), + Participating: true, + }, + }) + require.NoError(t, err) + } + + for _, proposal := range keeper.GetProposals(ctx) { + require.Equal(t, foundation.PROPOSAL_STATUS_ABORTED, proposal.Status) + } +} From 52ab94fbe1d961a1e70ffbafe19326b7de3e95a3 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Thu, 29 Sep 2022 02:31:46 +0000 Subject: [PATCH 2/4] Fix the misleading naming of the variable in the test --- x/foundation/keeper/exec_test.go | 4 ++-- x/foundation/keeper/keeper_test.go | 14 +++++++------- x/foundation/keeper/msg_server_test.go | 2 +- x/foundation/keeper/proposal_test.go | 2 +- x/foundation/keeper/vote_test.go | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/x/foundation/keeper/exec_test.go b/x/foundation/keeper/exec_test.go index 7a0e54c036..d2883c0fdf 100644 --- a/x/foundation/keeper/exec_test.go +++ b/x/foundation/keeper/exec_test.go @@ -17,8 +17,8 @@ func (s *KeeperTestSuite) TestExec() { proposalID: s.invalidProposal, valid: true, }, - "aborted proposal": { - proposalID: s.abortedProposal, + "withdrawn proposal": { + proposalID: s.withdrawnProposal, }, } diff --git a/x/foundation/keeper/keeper_test.go b/x/foundation/keeper/keeper_test.go index 024e9f7ae9..7f1421a8c5 100644 --- a/x/foundation/keeper/keeper_test.go +++ b/x/foundation/keeper/keeper_test.go @@ -51,10 +51,10 @@ type KeeperTestSuite struct { members []sdk.AccAddress stranger sdk.AccAddress - activeProposal uint64 - votedProposal uint64 - abortedProposal uint64 - invalidProposal uint64 + activeProposal uint64 + votedProposal uint64 + withdrawnProposal uint64 + invalidProposal uint64 balance sdk.Int } @@ -147,8 +147,8 @@ func (s *KeeperTestSuite) SetupTest() { s.Require().NoError(err) } - // create an aborted proposal - s.abortedProposal, err = s.keeper.SubmitProposal(s.ctx, []string{s.members[0].String()}, "", []sdk.Msg{ + // create an withdrawn proposal + s.withdrawnProposal, err = s.keeper.SubmitProposal(s.ctx, []string{s.members[0].String()}, "", []sdk.Msg{ &foundation.MsgWithdrawFromTreasury{ Operator: s.operator.String(), To: s.stranger.String(), @@ -156,7 +156,7 @@ func (s *KeeperTestSuite) SetupTest() { }, }) s.Require().NoError(err) - err = s.keeper.WithdrawProposal(s.ctx, s.abortedProposal) + err = s.keeper.WithdrawProposal(s.ctx, s.withdrawnProposal) s.Require().NoError(err) // create an invalid proposal which contains invalid message diff --git a/x/foundation/keeper/msg_server_test.go b/x/foundation/keeper/msg_server_test.go index 262b7ced64..2eca009a54 100644 --- a/x/foundation/keeper/msg_server_test.go +++ b/x/foundation/keeper/msg_server_test.go @@ -289,7 +289,7 @@ func (s *KeeperTestSuite) TestMsgWithdrawProposal() { address: s.stranger, }, "inactive proposal": { - proposalID: s.abortedProposal, + proposalID: s.withdrawnProposal, address: s.members[0], }, } diff --git a/x/foundation/keeper/proposal_test.go b/x/foundation/keeper/proposal_test.go index 4be1dce60f..6daafd24a8 100644 --- a/x/foundation/keeper/proposal_test.go +++ b/x/foundation/keeper/proposal_test.go @@ -82,7 +82,7 @@ func (s *KeeperTestSuite) TestWithdrawProposal() { valid: true, }, "not active": { - id: s.abortedProposal, + id: s.withdrawnProposal, }, } diff --git a/x/foundation/keeper/vote_test.go b/x/foundation/keeper/vote_test.go index dd0cfdd7ef..f35d0d1854 100644 --- a/x/foundation/keeper/vote_test.go +++ b/x/foundation/keeper/vote_test.go @@ -34,7 +34,7 @@ func (s *KeeperTestSuite) TestVote() { option: foundation.VOTE_OPTION_YES, }, "inactive proposal": { - proposalID: s.abortedProposal, + proposalID: s.withdrawnProposal, voter: s.members[0], option: foundation.VOTE_OPTION_YES, }, From f3a5dda3b570fe709a8ea8bc7ce68c56a8d6205a Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Thu, 29 Sep 2022 02:45:45 +0000 Subject: [PATCH 3/4] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4a2887f6a..e0f4799cb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (amino) [\#635](https://github.com/line/lbm-sdk/pull/635) change some minor things that haven't been fixed in #549 * (store) [\#666](https://github.com/line/lbm-sdk/pull/666) change default `iavl-cache-size` and description * (simapp) [\#679](https://github.com/line/lbm-sdk/pull/679) fix the bug not setting `iavl-cache-size` value of `app.toml` +* (x/foundation) [\#687](https://github.com/line/lbm-sdk/pull/687) fix bugs on aborting x/foundation proposals ### Breaking Changes * (proto) [\#564](https://github.com/line/lbm-sdk/pull/564) change gRPC path to original cosmos path From afd6097fa4e933a0dca07301a71d569a8faef20e Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Tue, 4 Oct 2022 02:16:57 +0000 Subject: [PATCH 4/4] Fix copy-pasted return --- x/foundation/msgs_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/x/foundation/msgs_test.go b/x/foundation/msgs_test.go index 7ffd77513f..97d28772a4 100644 --- a/x/foundation/msgs_test.go +++ b/x/foundation/msgs_test.go @@ -45,7 +45,7 @@ func TestMsgFundTreasury(t *testing.T) { err := msg.ValidateBasic() if !tc.valid { require.Error(t, err, name) - return + continue } require.NoError(t, err, name) @@ -96,7 +96,7 @@ func TestMsgWithdrawFromTreasury(t *testing.T) { err := msg.ValidateBasic() if !tc.valid { require.Error(t, err, name) - return + continue } require.NoError(t, err, name) @@ -161,7 +161,7 @@ func TestMsgUpdateMembers(t *testing.T) { err := msg.ValidateBasic() if !tc.valid { require.Error(t, err, name) - return + continue } require.NoError(t, err, name) @@ -250,7 +250,7 @@ func TestMsgUpdateDecisionPolicy(t *testing.T) { err := msg.ValidateBasic() if !tc.valid { require.Error(t, err, name) - return + continue } require.NoError(t, err, name) @@ -336,7 +336,7 @@ func TestMsgSubmitProposal(t *testing.T) { err = msg.ValidateBasic() if !tc.valid { require.Error(t, err, name) - return + continue } require.NoError(t, err, name) @@ -377,7 +377,7 @@ func TestMsgWithdrawProposal(t *testing.T) { err := msg.ValidateBasic() if !tc.valid { require.Error(t, err, name) - return + continue } require.NoError(t, err, name) @@ -440,7 +440,7 @@ func TestMsgVote(t *testing.T) { err := msg.ValidateBasic() if !tc.valid { require.Error(t, err, name) - return + continue } require.NoError(t, err, name) @@ -481,7 +481,7 @@ func TestMsgExec(t *testing.T) { err := msg.ValidateBasic() if !tc.valid { require.Error(t, err, name) - return + continue } require.NoError(t, err, name) @@ -514,7 +514,7 @@ func TestMsgLeaveFoundation(t *testing.T) { err := msg.ValidateBasic() if !tc.valid { require.Error(t, err, name) - return + continue } require.NoError(t, err, name) @@ -566,7 +566,7 @@ func TestMsgGrant(t *testing.T) { err := msg.ValidateBasic() if !tc.valid { require.Error(t, err, name) - return + continue } require.NoError(t, err, name) @@ -616,7 +616,7 @@ func TestMsgRevoke(t *testing.T) { err := msg.ValidateBasic() if !tc.valid { require.Error(t, err, name) - return + continue } require.NoError(t, err, name)