Fix max vesting cancellation (#1188)

* fix max vesting cancellation

* add cancelled amount as event attributes

---------

Co-authored-by: Abhinav Kumar <57705190+avkr003@users.noreply.github.com>

Author: 99adarsh

x/commitment/keeper/msg_server_cancel_vest.go

@@ -37,6 +37,7 @@ func (k msgServer) CancelVest(goCtx context.Context, msg *types.MsgCancelVest) (
 	commitments := k.GetCommitments(ctx, creator)
 
 	remainingToCancel := msg.Amount
+	totalCancelled := sdkmath.ZeroInt()
 
 	for i := len(commitments.VestingTokens) - 1; i >= 0; i-- {
 		vesting := commitments.VestingTokens[i]
@@ -58,6 +59,7 @@ func (k msgServer) CancelVest(goCtx context.Context, msg *types.MsgCancelVest) (
 		commitments.VestingTokens[i] = vesting
 
 		remainingToCancel = remainingToCancel.Sub(cancelAmount)
+		totalCancelled = totalCancelled.Add(cancelAmount)
 	}
 
 	newVestingTokens := []*types.VestingTokens{}
@@ -70,15 +72,19 @@ func (k msgServer) CancelVest(goCtx context.Context, msg *types.MsgCancelVest) (
 
 	commitments.VestingTokens = newVestingTokens
 
-	if !remainingToCancel.IsZero() {
+	if totalCancelled.IsZero() {
 		return nil, errorsmod.Wrapf(types.ErrInsufficientVestingTokens, "denom: %s, amount: %s", ptypes.Eden, msg.Amount)
 	}
+	ctx.Logger().Info("Successfully Cancelled vesting token",
+		"creator", msg.Creator,
+		"amount", totalCancelled.String(),
+		"denom", msg.Denom)
 
 	// Update the unclaimed tokens amount
-	commitments.AddClaimed(sdk.NewCoin(ptypes.Eden, msg.Amount))
+	commitments.AddClaimed(sdk.NewCoin(ptypes.Eden, totalCancelled))
 
 	prev := k.GetTotalSupply(ctx)
-	prev.TotalEdenSupply = prev.TotalEdenSupply.Add(msg.Amount)
+	prev.TotalEdenSupply = prev.TotalEdenSupply.Add(totalCancelled)
 	k.SetTotalSupply(ctx, prev)
 	k.SetCommitments(ctx, commitments)
 
@@ -88,6 +94,7 @@ func (k msgServer) CancelVest(goCtx context.Context, msg *types.MsgCancelVest) (
 			types.EventTypeCommitmentChanged,
 			sdk.NewAttribute(types.AttributeCreator, msg.Creator),
 			sdk.NewAttribute(types.AttributeAmount, msg.Amount.String()),
+			sdk.NewAttribute(types.AttributeCancelledAmount, totalCancelled.String()),
 			sdk.NewAttribute(types.AttributeDenom, ptypes.Eden),
 		),
 	)

x/commitment/keeper/msg_server_cancel_vest_test.go

@@ -14,6 +14,78 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestCancelVest_CompleteAmount(t *testing.T) {
+	app := app.InitElysTestApp(true, t)
+
+	ctx := app.BaseApp.NewContext(true)
+	// Create a test context and keeper
+	keeper := app.CommitmentKeeper
+
+	msgServer := commitmentkeeper.NewMsgServerImpl(*keeper)
+
+	vestingInfos := []types.VestingInfo{
+		{
+			BaseDenom:      ptypes.Eden,
+			VestingDenom:   ptypes.Elys,
+			NumBlocks:      10,
+			VestNowFactor:  sdkmath.NewInt(90),
+			NumMaxVestings: 10,
+		},
+	}
+
+	params := types.Params{
+		VestingInfos: vestingInfos,
+	}
+
+	keeper.SetParams(ctx, params)
+
+	// Create a new account
+	creator, _ := sdk.AccAddressFromBech32("cosmos1xv9tklw7d82sezh9haa573wufgy59vmwe6xxe5")
+	acc := app.AccountKeeper.GetAccount(ctx, creator)
+	if acc == nil {
+		acc = app.AccountKeeper.NewAccountWithAddress(ctx, creator)
+		app.AccountKeeper.SetAccount(ctx, acc)
+	}
+	// Create a cancel vesting message
+	cancelVestMsg := &types.MsgCancelVest{
+		Creator: creator.String(),
+		Denom:   ptypes.Eden,
+		Amount:  sdkmath.NewInt(100),
+	}
+
+	// Set up the commitments for the creator
+	commitments := types.Commitments{
+		Creator: creator.String(),
+		VestingTokens: []*types.VestingTokens{
+			{
+				Denom:         ptypes.Elys,
+				TotalAmount:   sdkmath.NewInt(100),
+				ClaimedAmount: sdkmath.NewInt(0),
+				NumBlocks:     100,
+				StartBlock:    0,
+			},
+		},
+	}
+	keeper.SetCommitments(ctx, commitments)
+
+	// Increase the block height
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 25)
+
+	// From 100 tokens, some of tokens are already vested and that will be claimed,
+	// so CancelVest should cancel the remaining amount without any issue
+	// Execute the CancelVest function
+	_, err := msgServer.CancelVest(ctx, cancelVestMsg)
+	require.NoError(t, err)
+
+	newCommitments := keeper.GetCommitments(ctx, creator)
+	require.Len(t, newCommitments.VestingTokens, 0, "vesting tokens should be empty after cancelling all remaining amount")
+
+	// No vesting tokens, so should throw an error
+	_, err = msgServer.CancelVest(ctx, cancelVestMsg)
+	require.Error(t, err)
+	require.True(t, types.ErrInsufficientVestingTokens.Is(err), "Error should be insufficient vesting tokens")
+}
+
 func TestCancelVest(t *testing.T) {
 	app := app.InitElysTestApp(true, t)
 
@@ -83,11 +155,12 @@ func TestCancelVest(t *testing.T) {
 	// check if the unclaimed tokens were updated correctly
 	require.Equal(t, sdkmath.NewInt(25), newCommitments.GetClaimedForDenom(ptypes.Eden))
 
-	// Try to cancel an amount that exceeds the unvested amount
+	// Try to cancel an amount that exceeds the unvested amount, should cancel all the remaining amount
 	cancelVestMsg.Amount = sdkmath.NewInt(100)
 	_, err = msgServer.CancelVest(ctx, cancelVestMsg)
-	require.Error(t, err, "should throw an error when trying to cancel more tokens than available")
-	require.True(t, types.ErrInsufficientVestingTokens.Is(err), "error should be insufficient vesting tokens")
+	require.NoError(t, err)
+	newCommitments = keeper.GetCommitments(ctx, creator)
+	require.Len(t, newCommitments.VestingTokens, 0, "vesting tokens should be empty after cancelling all remaining amount")
 }
 
 func TestCancelVest_WithPreviousClaimed(t *testing.T) {
@@ -246,5 +319,5 @@ func TestCancelVestNoVestingInfo(t *testing.T) {
 	_, err := msgServer.CancelVest(ctx, cancelVestMsg)
 	require.Error(t, err, "should throw an error when trying to cancel tokens with no vesting info")
 	fmt.Println(err.Error())
-	require.True(t, types.ErrInsufficientVestingTokens.Is(err), "error should be invalid denom")
+	require.True(t, types.ErrInsufficientVestingTokens.Is(err), "Error should be insufficient vesting tokens")
 }

x/commitment/types/events.go

@@ -8,7 +8,8 @@ const (
 	EventTypeBurnCoins         = "burn_coins"
 	EventTypeSendCoins         = "send_coins"
 
-	AttributeCreator = "creator"
-	AttributeAmount  = "token_amount"
-	AttributeDenom   = "token_denom"
+	AttributeCreator         = "creator"
+	AttributeAmount          = "token_amount"
+	AttributeCancelledAmount = "cancelled_amount"
+	AttributeDenom           = "token_denom"
 )