Better to sdk coin convertion (#1164)

* Better to sdk coin convertion

* Review feedback

x/wasm/keeper/handler_plugin.go

@@ -209,6 +209,9 @@ func NewBurnCoinMessageHandler(burner types.Burner) MessageHandlerFunc {
 			if err != nil {
 				return nil, nil, err
 			}
+			if coins.IsZero() {
+				return nil, nil, types.ErrEmpty.Wrap("amount")
+			}
 			if err := burner.SendCoinsFromAccountToModule(ctx, contractAddr, types.ModuleName, coins); err != nil {
 				return nil, nil, sdkerrors.Wrap(err, "transfer to module")
 			}

x/wasm/keeper/handler_plugin_encoders.go

@@ -329,10 +329,9 @@ func ConvertWasmCoinsToSdkCoins(coins []wasmvmtypes.Coin) (sdk.Coins, error) {
 		if err != nil {
 			return nil, err
 		}
-		toSend = append(toSend, c)
+		toSend = toSend.Add(c)
 	}
-	toSend.Sort()
-	return toSend, nil
+	return toSend.Sort(), nil
 }
 
 // ConvertWasmCoinToSdkCoin converts a wasm vm type coin to sdk type coin

x/wasm/keeper/handler_plugin_encoders_test.go

@@ -635,3 +635,85 @@ func TestConvertWasmCoinToSdkCoin(t *testing.T) {
 		})
 	}
 }
+
+func TestConvertWasmCoinsToSdkCoins(t *testing.T) {
+	specs := map[string]struct {
+		src    []wasmvmtypes.Coin
+		exp    sdk.Coins
+		expErr bool
+	}{
+		"empty": {
+			src: []wasmvmtypes.Coin{},
+			exp: nil,
+		},
+		"single coin": {
+			src: []wasmvmtypes.Coin{{Denom: "foo", Amount: "1"}},
+			exp: sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(1))),
+		},
+		"multiple coins": {
+			src: []wasmvmtypes.Coin{
+				{Denom: "foo", Amount: "1"},
+				{Denom: "bar", Amount: "2"},
+			},
+			exp: sdk.NewCoins(
+				sdk.NewCoin("bar", sdk.NewInt(2)),
+				sdk.NewCoin("foo", sdk.NewInt(1)),
+			),
+		},
+		"sorted": {
+			src: []wasmvmtypes.Coin{
+				{Denom: "foo", Amount: "1"},
+				{Denom: "other", Amount: "1"},
+				{Denom: "bar", Amount: "1"},
+			},
+			exp: []sdk.Coin{
+				sdk.NewCoin("bar", sdk.NewInt(1)),
+				sdk.NewCoin("foo", sdk.NewInt(1)),
+				sdk.NewCoin("other", sdk.NewInt(1)),
+			},
+		},
+		"zero amounts dropped": {
+			src: []wasmvmtypes.Coin{
+				{Denom: "foo", Amount: "1"},
+				{Denom: "bar", Amount: "0"},
+			},
+			exp: sdk.NewCoins(
+				sdk.NewCoin("foo", sdk.NewInt(1)),
+			),
+		},
+		"duplicate denoms merged": {
+			src: []wasmvmtypes.Coin{
+				{Denom: "foo", Amount: "1"},
+				{Denom: "foo", Amount: "1"},
+			},
+			exp: []sdk.Coin{sdk.NewCoin("foo", sdk.NewInt(2))},
+		},
+		"duplicate denoms with one 0 amount does not fail": {
+			src: []wasmvmtypes.Coin{
+				{Denom: "foo", Amount: "0"},
+				{Denom: "foo", Amount: "1"},
+			},
+			exp: []sdk.Coin{sdk.NewCoin("foo", sdk.NewInt(1))},
+		},
+		"empty denom rejected": {
+			src:    []wasmvmtypes.Coin{{Denom: "", Amount: "1"}},
+			expErr: true,
+		},
+		"invalid denom rejected": {
+			src:    []wasmvmtypes.Coin{{Denom: "!%&", Amount: "1"}},
+			expErr: true,
+		},
+	}
+	for name, spec := range specs {
+		t.Run(name, func(t *testing.T) {
+			gotCoins, gotErr := ConvertWasmCoinsToSdkCoins(spec.src)
+			if spec.expErr {
+				require.Error(t, gotErr)
+				return
+			}
+			require.NoError(t, gotErr)
+			assert.Equal(t, spec.exp, gotCoins)
+			assert.NoError(t, gotCoins.Validate())
+		})
+	}
+}