fix: fix bugs on aborting x/foundation proposals (#687)

* Fix bugs on abortOldProposals()

* Fix the misleading naming of the variable in the test

* Update CHANGELOG.md

* Fix copy-pasted return

CHANGELOG.md

@@ -88,6 +88,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

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,
 		},
 	}
 

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,16 +147,16 @@ 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(),
 			Amount:   sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, s.balance)),
 		},
 	})
 	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

x/foundation/keeper/msg_server_test.go

@@ -294,7 +294,7 @@ func (s *KeeperTestSuite) TestMsgWithdrawProposal() {
 			address: s.stranger,
 		},
 		"inactive proposal": {
-			proposalID: s.abortedProposal,
+			proposalID: s.withdrawnProposal,
 			address:    s.members[0],
 		},
 	}

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
 		}
 

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"
@@ -75,7 +82,7 @@ func (s *KeeperTestSuite) TestWithdrawProposal() {
 			valid: true,
 		},
 		"not active": {
-			id: s.abortedProposal,
+			id: s.withdrawnProposal,
 		},
 	}
 
@@ -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)
+	}
+}

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,
 		},

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)