fix!(manifest): only poa is allowed to burn tokens (#66)

interchaintest/helpers/manifest.go

@@ -34,3 +34,10 @@ func ManifestQueryParams(ctx context.Context, node *cosmos.ChainNode) (*manifest
 	res, err := manifesttypes.NewQueryClient(node.GrpcConn).Params(ctx, &manifesttypes.QueryParamsRequest{})
 	return res.GetParams(), err
 }
+
+func ManifestBurnTokens(t *testing.T, ctx context.Context, chain *cosmos.CosmosChain, keyName string, amount string, flags ...string) (sdk.TxResponse, error) {
+	txCmd := []string{"tx", "manifest", "burn-coins", amount}
+	fmt.Println("ManifestBurnTokens", txCmd)
+	cmd := TxCommandBuilder(ctx, chain, txCmd, keyName, flags...)
+	return ExecuteTransaction(ctx, chain, cmd)
+}

interchaintest/mainfest_test.go

@@ -148,6 +148,61 @@ func TestManifestModule(t *testing.T) {
 		require.Error(t, err)
 	})
 
+	t.Run("fail: invalid burn authority", func(t *testing.T) {
+		accBal, err := appChain.GetBalance(ctx, uaddr, Denom)
+		require.NoError(t, err)
+		o, err := helpers.ManifestBurnTokens(t, ctx, appChain, uaddr, "1"+Denom)
+		require.NoError(t, err) // The tx is successful but the burn fails
+		tx, err := appChain.GetTransaction(o.TxHash)
+		require.NoError(t, err)
+		require.NotEqual(t, tx.Code, uint32(0x0)) // The burn failed
+		require.Contains(t, tx.RawLog, "invalid authority")
+		accBal2, err := appChain.GetBalance(ctx, uaddr, Denom)
+		require.NoError(t, err)
+		require.EqualValues(t, accBal, accBal2)
+	})
+
+	t.Run("success: burn tokens as poa admin", func(t *testing.T) {
+		poaAdminAddr := poaAdmin.FormattedAddress()
+		accBal, err := appChain.GetBalance(ctx, poaAdminAddr, Denom)
+		require.NoError(t, err)
+		o, err := helpers.ManifestBurnTokens(t, ctx, appChain, poaAdminAddr, "1"+Denom)
+		require.NoError(t, err)
+		tx, err := appChain.GetTransaction(o.TxHash)
+		require.NoError(t, err)
+		require.Equal(t, tx.Code, uint32(0x0))
+		accBal2, err := appChain.GetBalance(ctx, poaAdminAddr, Denom)
+		require.NoError(t, err)
+		require.EqualValues(t, accBal2, accBal.Sub(sdkmath.OneInt()))
+	})
+
+	t.Run("fail: burn unknown denom as poa admin", func(t *testing.T) {
+		poaAdminAddr := poaAdmin.FormattedAddress()
+		accBal, err := appChain.GetBalance(ctx, poaAdminAddr, Denom)
+		require.NoError(t, err)
+		o, err := helpers.ManifestBurnTokens(t, ctx, appChain, poaAdminAddr, "1foobar")
+		require.NoError(t, err) // The tx is successful but the burn fails
+		tx, err := appChain.GetTransaction(o.TxHash)
+		require.NoError(t, err)
+		require.NotEqual(t, tx.Code, uint32(0x0)) // The burn failed
+		require.Contains(t, tx.RawLog, "insufficient funds ")
+		accBal2, err := appChain.GetBalance(ctx, poaAdminAddr, Denom)
+		require.NoError(t, err)
+		require.EqualValues(t, accBal, accBal2)
+	})
+
+	t.Run("fail: burn invalid coin expression as poa admin", func(t *testing.T) {
+		poaAdminAddr := poaAdmin.FormattedAddress()
+		accBal, err := appChain.GetBalance(ctx, poaAdminAddr, Denom)
+		require.NoError(t, err)
+		_, err = helpers.ManifestBurnTokens(t, ctx, appChain, poaAdminAddr, "foobar")
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "invalid decimal coin expression")
+		accBal2, err := appChain.GetBalance(ctx, poaAdminAddr, Denom)
+		require.NoError(t, err)
+		require.EqualValues(t, accBal, accBal2)
+	})
+
 	t.Cleanup(func() {
 		CopyCoverageFromContainer(ctx, t, client, appChain.GetNode().ContainerID(), appChain.HomeDir())
 		_ = ic.Close()

interchaintest/poa_group_test.go

@@ -491,7 +491,7 @@ func createManifestPayoutProposal(authority string, payouts []manifesttypes.Payo
 
 func createManifestBurnProposal(sender string, amounts sdk.Coins) manifesttypes.MsgBurnHeldBalance {
 	return manifesttypes.MsgBurnHeldBalance{
-		Sender:    sender,
+		Authority: sender,
 		BurnCoins: amounts,
 	}
 }

proto/manifest/v1/tx.proto

@@ -78,11 +78,11 @@ message MsgPayoutResponse {}
 
 // MsgPayout is the Msg/BurnHeldBalance request type.
 message MsgBurnHeldBalance {
-  option (cosmos.msg.v1.signer) = "sender";
+  option (cosmos.msg.v1.signer) = "authority";
   option (gogoproto.equal) = false;
 
   // sender is the address of the tokenholder.
-  string sender = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+  string authority = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
 
   // burn_coins are the coins to be burned by the tokenholder.
   repeated cosmos.base.v1beta1.Coin burn_coins = 2 [

x/manifest/keeper/msg_server.go

@@ -44,15 +44,18 @@ func (ms msgServer) Payout(ctx context.Context, req *types.MsgPayout) (*types.Ms
 	return nil, ms.k.Payout(ctx, req.PayoutPairs)
 }
 
-func (ms msgServer) BurnHeldBalance(ctx context.Context, msg *types.MsgBurnHeldBalance) (*types.MsgBurnHeldBalanceResponse, error) {
-	addr, err := sdk.AccAddressFromBech32(msg.Sender)
+func (ms msgServer) BurnHeldBalance(ctx context.Context, req *types.MsgBurnHeldBalance) (*types.MsgBurnHeldBalanceResponse, error) {
+	if ms.k.authority != req.Authority {
+		return nil, fmt.Errorf("invalid authority; expected %s, got %s", ms.k.authority, req.Authority)
+	}
+	addr, err := sdk.AccAddressFromBech32(req.Authority)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := ms.k.bankKeeper.SendCoinsFromAccountToModule(ctx, addr, types.ModuleName, msg.BurnCoins); err != nil {
-		return nil, fmt.Errorf("not enough balance to burn %s: %w", msg.BurnCoins, err)
+	if err := ms.k.bankKeeper.SendCoinsFromAccountToModule(ctx, addr, types.ModuleName, req.BurnCoins); err != nil {
+		return nil, fmt.Errorf("not enough balance to burn %s: %w", req.BurnCoins, err)
 	}
 
-	return &types.MsgBurnHeldBalanceResponse{}, ms.k.bankKeeper.BurnCoins(ctx, types.ModuleName, msg.BurnCoins)
+	return &types.MsgBurnHeldBalanceResponse{}, ms.k.bankKeeper.BurnCoins(ctx, types.ModuleName, req.BurnCoins)
 }

x/manifest/keeper/msg_server_test.go

@@ -129,13 +129,14 @@ func TestBurnCoins(t *testing.T) {
 	k := f.App.ManifestKeeper
 	k.SetAuthority(authority.String())
 	ms := keeper.NewMsgServerImpl(k)
+	_, _, acc := testdata.KeyTestPubAddr()
 
 	type tc struct {
 		name     string
 		initial  sdk.Coins
 		burn     sdk.Coins
 		expected sdk.Coins
-		address  string
+		address  sdk.AccAddress
 		success  bool
 	}
 
@@ -148,50 +149,55 @@ func TestBurnCoins(t *testing.T) {
 			initial:  sdk.NewCoins(),
 			burn:     sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(7))),
 			expected: sdk.NewCoins(),
+			address:  authority,
 		},
 		{
 			name:     "fail; bad address",
 			initial:  sdk.NewCoins(),
 			burn:     sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(7))),
 			expected: sdk.NewCoins(),
-			address:  "xyz",
+			address:  sdk.AccAddress{0x0},
 		},
 		{
-			name:     "success; burn 1 token successfully",
+			name:     "success; burn tokens successfully",
 			initial:  sdk.NewCoins(stake, mfx),
 			burn:     sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(7))),
 			expected: sdk.NewCoins(mfx, stake.SubAmount(sdkmath.NewInt(7))),
+			address:  authority,
 			success:  true,
 		},
 		{
 			name:     "success; burn many tokens successfully",
 			initial:  sdk.NewCoins(stake, mfx),
 			burn:     sdk.NewCoins(sdk.NewCoin("umfx", sdkmath.NewInt(9)), sdk.NewCoin("stake", sdkmath.NewInt(7))),
 			expected: sdk.NewCoins(mfx.SubAmount(sdkmath.NewInt(9)), stake.SubAmount(sdkmath.NewInt(7))),
+			address:  authority,
 			success:  true,
 		},
+		{
+			name:     "fail; invalid authority",
+			initial:  sdk.NewCoins(stake, mfx),
+			burn:     sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(7))),
+			expected: sdk.NewCoins(stake, mfx),
+			address:  acc,
+		},
 	}
 
 	for _, c := range cases {
 		c := c
 		t.Run(c.name, func(t *testing.T) {
-			_, _, acc := testdata.KeyTestPubAddr()
-			if c.address == "" {
-				c.address = acc.String()
-			}
-
 			// setup initial balances for the new account
 			if len(c.initial) > 0 {
 				require.NoError(t, f.App.BankKeeper.MintCoins(f.Ctx, "mint", c.initial))
-				require.NoError(t, f.App.BankKeeper.SendCoinsFromModuleToAccount(f.Ctx, "mint", acc, c.initial))
+				require.NoError(t, f.App.BankKeeper.SendCoinsFromModuleToAccount(f.Ctx, "mint", c.address, c.initial))
 			}
 
 			// validate initial balance
-			require.Equal(t, c.initial, f.App.BankKeeper.GetAllBalances(f.Ctx, acc))
+			require.Equal(t, c.initial, f.App.BankKeeper.GetAllBalances(f.Ctx, c.address))
 
 			// burn coins
 			_, err := ms.BurnHeldBalance(f.Ctx, &types.MsgBurnHeldBalance{
-				Sender:    c.address,
+				Authority: c.address.String(),
 				BurnCoins: c.burn,
 			})
 			if c.success {
@@ -200,7 +206,17 @@ func TestBurnCoins(t *testing.T) {
 				require.Error(t, err)
 			}
 
-			require.Equal(t, c.expected, f.App.BankKeeper.GetAllBalances(f.Ctx, acc))
+			allBalance := f.App.BankKeeper.GetAllBalances(f.Ctx, c.address)
+			require.Equal(t, c.expected, allBalance)
+
+			// burn the rest of the coins to reset the balance to 0 for the next test if the test was successful
+			if c.success {
+				_, err = ms.BurnHeldBalance(f.Ctx, &types.MsgBurnHeldBalance{
+					Authority: c.address.String(),
+					BurnCoins: allBalance,
+				})
+				require.NoError(t, err)
+			}
 		})
 	}
 }

x/manifest/types/msgs.go

@@ -133,7 +133,7 @@ func NewMsgBurnHeldBalance(
 	coins sdk.Coins,
 ) *MsgBurnHeldBalance {
 	return &MsgBurnHeldBalance{
-		Sender:    sender.String(),
+		Authority: sender.String(),
 		BurnCoins: coins,
 	}
 }
@@ -151,13 +151,13 @@ func (msg MsgBurnHeldBalance) GetSignBytes() []byte {
 
 // GetSigners returns the expected signers for the message.
 func (msg *MsgBurnHeldBalance) GetSigners() []sdk.AccAddress {
-	addr, _ := sdk.AccAddressFromBech32(msg.Sender)
+	addr, _ := sdk.AccAddressFromBech32(msg.Authority)
 	return []sdk.AccAddress{addr}
 }
 
 // ValidateBasic does a sanity check on the provided data.
 func (msg *MsgBurnHeldBalance) Validate() error {
-	if _, err := sdk.AccAddressFromBech32(msg.Sender); err != nil {
+	if _, err := sdk.AccAddressFromBech32(msg.Authority); err != nil {
 		return errors.Wrap(err, "invalid authority address")
 	}