fix: challangee rewards stay in dao module if challenge is not sucessful (#286)

* fix: challangee rewards stay in dao module if challenge is not sucessful

* refactor: split up coin conversion functions

* fix: add dao module coin conversion

* test: adjust test case for updated behavior

* chore: fix typo

* fix: do not mint claims for dao

* refactor: remove redundant statement

* fix: error handling on claim converstion fail

* fix: linter error

* chore: add comment to test case
---------

Signed-off-by: Lorenz Herzberger <lorenzherzberger@gmail.com>

tests/e2e/dao/suite.go

@@ -268,7 +268,7 @@ func (s *E2ETestSuite) TestPoPResult() {
 		challenges[i].Initiator = val.Address.String()
 		challenges[i].Challenger = aliceAddr.String()
 		challenges[i].Challengee = bobAddr.String()
-		challenges[i].Success = true
+		challenges[i].Success = blockHeight%2 == 0 // Need some successful and unsuccessful challenges for assertion of correct behavior
 		challenges[i].Finished = true
 
 		msg := daotypes.NewMsgReportPopResult(val.Address.String(), &challenges[i])
@@ -287,7 +287,7 @@ func (s *E2ETestSuite) TestPoPResult() {
 	})
 	s.Require().NoError(err)
 	assert.Contains(s.T(), out.String(), conf.StagedDenom)
-	assert.Contains(s.T(), out.String(), "39954337890") // Total supply 5 * 7990867578 = 39954337890
+	assert.Equal(s.T(), "amount: \"21974885838\"\ndenom: stagedcrddl\n", out.String()) // Total supply 5 * 1997716894 + 2 * 5993150684 = 21974885838
 
 	out, err = clitestutil.ExecTestCLICmd(val.ClientCtx, bank.GetBalancesCmd(), []string{
 		aliceAddr.String(),
@@ -295,7 +295,7 @@ func (s *E2ETestSuite) TestPoPResult() {
 	})
 	s.Require().NoError(err)
 	assert.Contains(s.T(), out.String(), conf.StagedDenom)
-	assert.Contains(s.T(), out.String(), "9988584470") // 5 * 1997716894 = 9988584470
+	assert.Equal(s.T(), "amount: \"9988584470\"\ndenom: stagedcrddl\n", out.String()) // 5 * 1997716894 = 9988584470
 
 	out, err = clitestutil.ExecTestCLICmd(val.ClientCtx, bank.GetBalancesCmd(), []string{
 		bobAddr.String(),
@@ -303,7 +303,7 @@ func (s *E2ETestSuite) TestPoPResult() {
 	})
 	s.Require().NoError(err)
 	assert.Contains(s.T(), out.String(), conf.StagedDenom)
-	assert.Contains(s.T(), out.String(), "29965753420") // 5 * 5993150684 = 29965753420
+	assert.Equal(s.T(), "amount: \"11986301368\"\ndenom: stagedcrddl\n", out.String()) // 2 * 5993150684 = 11986301368
 
 	// send ReissuanceProposal
 	msg1 := daotypes.NewMsgReissueRDDLProposal(val.Address.String(), hex.EncodeToString(val.PubKey.Address()),
@@ -345,23 +345,16 @@ func (s *E2ETestSuite) TestPoPResult() {
 	s.Require().NoError(err)
 	assert.Equal(s.T(), "[]", txResponse.RawLog)
 
-	// send DistributionResult
+	// send DistributionResult implicitly
-	msg4 := daotypes.NewMsgDistributionResult(val.Address.String(), challenges[2].Height, "DaoTxID", "InvestorTxID", "PoPTxID")
-	output, err = e2etestutil.BuildSignBroadcastTx(s.T(), val.Address, msg4)
-	s.Require().NoError(err)
 	s.Require().NoError(s.network.WaitForNextBlock())
 
-	txResponse, err = lib.GetTxResponseFromOut(output)
-	s.Require().NoError(err)
-	assert.Equal(s.T(), "[]", txResponse.RawLog)
-
 	// check balance for crddl
 	out, err = clitestutil.ExecTestCLICmd(val.ClientCtx, bank.GetCmdQueryTotalSupply(), []string{
 		fmt.Sprintf("--%s=%s", bank.FlagDenom, conf.ClaimDenom),
 	})
 	s.Require().NoError(err)
 	assert.Contains(s.T(), out.String(), conf.ClaimDenom)
-	assert.Equal(s.T(), "amount: \"23972602734\"\ndenom: crddl\n", out.String()) // Total supply 3 * 5993150684 + 3 * 1997716894 = 23972602734
+	assert.Equal(s.T(), "amount: \"11986301366\"\ndenom: crddl\n", out.String()) // Total supply 1 * 5993150684 + 3 * 1997716894 = 11986301366
 
 	out, err = clitestutil.ExecTestCLICmd(val.ClientCtx, bank.GetBalancesCmd(), []string{
 		aliceAddr.String(),
@@ -377,5 +370,5 @@ func (s *E2ETestSuite) TestPoPResult() {
 	})
 	s.Require().NoError(err)
 	assert.Contains(s.T(), out.String(), conf.ClaimDenom)
-	assert.Equal(s.T(), "amount: \"17979452052\"\ndenom: crddl\n", out.String()) // 3 * 5993150684 = 17979452052
+	assert.Equal(s.T(), "amount: \"5993150684\"\ndenom: crddl\n", out.String()) // 1 * 5993150684 = 5993150684
 }

x/dao/keeper/msg_server_distribution_result.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"strconv"
 
+	errorsmod "cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/planetmint/planetmint-go/config"
 	"github.com/planetmint/planetmint-go/util"
@@ -21,9 +22,9 @@ func (k msgServer) DistributionResult(goCtx context.Context, msg *types.MsgDistr
 		err := k.resolveStagedClaims(ctx, distribution.FirstPop, distribution.LastPop)
 		if err != nil {
 			util.GetAppLogger().Error(ctx, "%s for provided PoP heights: %d %d", types.ErrResolvingStagedClaims.Error(), distribution.FirstPop, distribution.LastPop)
-		} else {
+			return nil, errorsmod.Wrap(types.ErrConvertClaims, err.Error())
-			util.GetAppLogger().Info(ctx, "staged claims successfully for provided PoP heights: %d %d", distribution.FirstPop, distribution.LastPop)
 		}
+		util.GetAppLogger().Info(ctx, "staged claims successfully for provided PoP heights: %d %d", distribution.FirstPop, distribution.LastPop)
 		k.StoreDistributionOrder(ctx, distribution)
 	} else {
 		util.GetAppLogger().Error(ctx, "%s for provided block height %s", types.ErrDistributionNotFound.Error(), strconv.FormatInt(msg.GetLastPop(), 10))
@@ -42,9 +43,15 @@ func (k msgServer) resolveStagedClaims(ctx sdk.Context, start int64, end int64)
 	popParticipants := make(map[string]uint64)
 
 	for _, challenge := range challenges {
-		challengerAmt, challengeeAmt := getAmountsForChallenge(challenge)
+		// if challenge not finished nobody has claims
+		if !challenge.GetFinished() {
+			continue
+		}
+		_, challengerAmt, challengeeAmt := util.GetPopReward(challenge.Height)
 		popParticipants[challenge.Challenger] += challengerAmt
-		popParticipants[challenge.Challengee] += challengeeAmt
+		if challenge.GetSuccess() {
+			popParticipants[challenge.Challengee] += challengeeAmt
+		}
 	}
 
 	// second data structure because map iteration order is not guaranteed in GO
@@ -53,7 +60,7 @@ func (k msgServer) resolveStagedClaims(ctx sdk.Context, start int64, end int64)
 		keys = append(keys, p)
 	}
 	for _, p := range keys {
-		err = k.convertClaim(ctx, p, popParticipants[p])
+		err = k.convertAccountClaim(ctx, p, popParticipants[p])
 		if err != nil {
 			return err
 		}
@@ -63,7 +70,7 @@ func (k msgServer) resolveStagedClaims(ctx sdk.Context, start int64, end int64)
 }
 
 // convert per account
-func (k msgServer) convertClaim(ctx sdk.Context, participant string, amount uint64) (err error) {
+func (k msgServer) convertAccountClaim(ctx sdk.Context, participant string, amount uint64) (err error) {
 	conf := config.GetConfig()
 	accAddr, err := sdk.AccAddressFromBech32(participant)
 	if err != nil {
@@ -73,21 +80,17 @@ func (k msgServer) convertClaim(ctx sdk.Context, participant string, amount uint
 	accStagedClaim := k.bankKeeper.GetBalance(ctx, accAddr, conf.StagedDenom)
 
 	if accStagedClaim.Amount.GTE(sdk.NewIntFromUint64(amount)) {
-		burnCoins := sdk.NewCoins(sdk.NewCoin(conf.StagedDenom, sdk.NewIntFromUint64(amount)))
+		burnCoins, mintCoins := getConvertCoins(amount)
-		mintCoins := sdk.NewCoins(sdk.NewCoin(conf.ClaimDenom, sdk.NewIntFromUint64(amount)))
-
 		err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, accAddr, types.ModuleName, burnCoins)
 		if err != nil {
 			return err
 		}
-		err = k.bankKeeper.BurnCoins(ctx, types.ModuleName, burnCoins)
+
-		if err != nil {
+		err = k.convertCoins(ctx, burnCoins, mintCoins)
-			return err
-		}
-		err = k.bankKeeper.MintCoins(ctx, types.ModuleName, mintCoins)
 		if err != nil {
 			return err
 		}
+
 		err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, accAddr, mintCoins)
 		if err != nil {
 			return err
@@ -97,11 +100,17 @@ func (k msgServer) convertClaim(ctx sdk.Context, participant string, amount uint
 	return
 }
 
-// gather amounts for accounts
+func (k msgServer) convertCoins(ctx sdk.Context, burnCoins sdk.Coins, mintCoins sdk.Coins) (err error) {
-func getAmountsForChallenge(challenge types.Challenge) (challenger uint64, challengee uint64) {
+	err = k.bankKeeper.BurnCoins(ctx, types.ModuleName, burnCoins)
-	totalAmt, challengerAmt, challengeeAmt := util.GetPopReward(challenge.Height)
+	if err != nil {
-	if challenge.Success {
+		return err
-		return challengerAmt, challengeeAmt
 	}
-	return totalAmt, 0
+	return k.bankKeeper.MintCoins(ctx, types.ModuleName, mintCoins)
+}
+
+func getConvertCoins(amount uint64) (burnCoins sdk.Coins, mintCoins sdk.Coins) {
+	conf := config.GetConfig()
+	burnCoins = sdk.NewCoins(sdk.NewCoin(conf.StagedDenom, sdk.NewIntFromUint64(amount)))
+	mintCoins = sdk.NewCoins(sdk.NewCoin(conf.ClaimDenom, sdk.NewIntFromUint64(amount)))
+	return
 }

x/dao/keeper/msg_server_report_pop_result.go

@@ -37,50 +37,44 @@ func (k msgServer) ReportPopResult(goCtx context.Context, msg *types.MsgReportPo
 
 func (k msgServer) issuePoPRewards(ctx sdk.Context, challenge types.Challenge) (err error) {
 	conf := config.GetConfig()
-	total, _, _ := util.GetPopReward(challenge.Height)
+	total, challengerAmt, _ := util.GetPopReward(challenge.Height)
+
+	stagedCRDDL := sdk.NewCoin(conf.StagedDenom, sdk.ZeroInt())
+	if challenge.GetSuccess() {
+		stagedCRDDL = stagedCRDDL.AddAmount(sdk.NewIntFromUint64(total))
+	} else {
+		stagedCRDDL = stagedCRDDL.AddAmount(sdk.NewIntFromUint64(challengerAmt))
+	}
 
-	stagedCRDDL := sdk.NewCoin(conf.StagedDenom, sdk.NewIntFromUint64(total))
 	err = k.bankKeeper.MintCoins(ctx, types.ModuleName, sdk.NewCoins(stagedCRDDL))
 	if err != nil {
-		return err
+		return
 	}
 
-	if challenge.Success {
+	return k.handlePoP(ctx, challenge)
-		err = k.handlePoPSuccess(ctx, challenge)
-		if err != nil {
-			return err
-		}
-	} else {
-		err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, sdk.AccAddress(challenge.Challenger), sdk.NewCoins(stagedCRDDL))
-		if err != nil {
-			return err
-		}
-	}
-
-	return err
 }
 
-func (k msgServer) handlePoPSuccess(ctx sdk.Context, challenge types.Challenge) (err error) {
+func (k msgServer) handlePoP(ctx sdk.Context, challenge types.Challenge) (err error) {
-	conf := config.GetConfig()
 	_, challengerAmt, challengeeAmt := util.GetPopReward(challenge.Height)
 
-	challengerCoin := sdk.NewCoin(conf.StagedDenom, sdk.NewIntFromUint64(challengerAmt))
+	err = k.sendRewards(ctx, challenge.GetChallenger(), challengerAmt)
-	challengeeCoin := sdk.NewCoin(conf.StagedDenom, sdk.NewIntFromUint64(challengeeAmt))
-	challengee, err := sdk.AccAddressFromBech32(challenge.Challengee)
 	if err != nil {
-		return err
+		return
 	}
-	err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, challengee, sdk.NewCoins(challengeeCoin))
+
-	if err != nil {
+	if !challenge.GetSuccess() {
-		return err
+		return
 	}
-	challenger, err := sdk.AccAddressFromBech32(challenge.Challenger)
+
-	if err != nil {
+	return k.sendRewards(ctx, challenge.GetChallengee(), challengeeAmt)
-		return err
+}
-	}
+
-	err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, challenger, sdk.NewCoins(challengerCoin))
+func (k msgServer) sendRewards(ctx sdk.Context, receiver string, amt uint64) (err error) {
-	if err != nil {
+	conf := config.GetConfig()
-		return err
+	coins := sdk.NewCoins(sdk.NewCoin(conf.StagedDenom, sdk.NewIntFromUint64(amt)))
-	}
+	receiverAddr, err := sdk.AccAddressFromBech32(receiver)
-	return
+	if err != nil {
+		return err
+	}
+	return k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, receiverAddr, coins)
 }

x/dao/types/errors.go

@@ -26,4 +26,5 @@ var (
 	ErrReissuanceTxIDMissing    = errorsmod.Register(ModuleName, 17, "reissuance has no transaction id")
 	ErrRestrictedMsg            = errorsmod.Register(ModuleName, 18, "restricted validator msg")
 	ErrDistributionWrongHeight  = errorsmod.Register(ModuleName, 19, "distribution wrong height")
+	ErrConvertClaims            = errorsmod.Register(ModuleName, 20, "convert claim failed")
 )