From 059a3b8742d7a3a5be4648a9c511e78e9b79a597 Mon Sep 17 00:00:00 2001
From: Jeff Thompson <jeff@thefirst.org>
Date: Wed, 7 Feb 2024 13:56:30 +0100
Subject: [PATCH] fix(sdk/vm): improve MsgCall panic error message for wrong
 number of args (#1610)

If a realm function Call has too many arguments, VMKeeper Call panics
with an uninformative index error message like "index out of range [1]
with length 1" when it is [checking the
arguments](https://github.com/gnolang/gno/blob/12b4b458e1b13d491a5797aa11b2002242f012bd/gno.land/pkg/sdk/vm/keeper.go#L244).
One of our devs lost time trying to figure this out, as would other devs
presumably.

This PR adds explicit checks for number of arguments with informative
panic messages like "not enough arguments in call to Echo" or "too many
arguments in call to Echo" . (These are the same errors as the Go
compiler.)

(I couldn't find an existing issue for this. If you already have other
plans to improve this error message, then you can close this PR.)

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

---------

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
Co-authored-by: Morgan <git@howl.moe>
---
 gno.land/pkg/sdk/vm/keeper.go      |  3 +++
 gno.land/pkg/sdk/vm/keeper_test.go | 41 ++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/gno.land/pkg/sdk/vm/keeper.go b/gno.land/pkg/sdk/vm/keeper.go
index 16162e1004c..e7998168f6c 100644
--- a/gno.land/pkg/sdk/vm/keeper.go
+++ b/gno.land/pkg/sdk/vm/keeper.go
@@ -240,6 +240,9 @@ func (vm *VMKeeper) Call(ctx sdk.Context, msg MsgCall) (res string, err error) {
 	if cx.Varg {
 		panic("variadic calls not yet supported")
 	}
+	if len(msg.Args) != len(ft.Params) {
+		panic(fmt.Sprintf("wrong number of arguments in call to %s: want %d got %d", fnc, len(ft.Params), len(msg.Args)))
+	}
 	for i, arg := range msg.Args {
 		argType := ft.Params[i].Type
 		atv := convertArgToGno(arg, argType)
diff --git a/gno.land/pkg/sdk/vm/keeper_test.go b/gno.land/pkg/sdk/vm/keeper_test.go
index 294efa66fa5..bc6bc285704 100644
--- a/gno.land/pkg/sdk/vm/keeper_test.go
+++ b/gno.land/pkg/sdk/vm/keeper_test.go
@@ -395,3 +395,44 @@ func main() {
 	expectedString := fmt.Sprintf("hello world! %s\n", addr.String())
 	assert.Equal(t, res, expectedString)
 }
+
+func TestNumberOfArgsError(t *testing.T) {
+	env := setupTestEnv()
+	ctx := env.ctx
+
+	// Give "addr1" some gnots.
+	addr := crypto.AddressFromPreimage([]byte("addr1"))
+	acc := env.acck.NewAccountWithAddress(ctx, addr)
+	env.acck.SetAccount(ctx, acc)
+	env.bank.SetCoins(ctx, addr, std.MustParseCoins("10000000ugnot"))
+	assert.True(t, env.bank.GetCoins(ctx, addr).IsEqual(std.MustParseCoins("10000000ugnot")))
+
+	// Create test package.
+	files := []*std.MemFile{
+		{
+			Name: "test.gno",
+			Body: `package test
+
+import "std"
+
+func Echo(msg string) string {
+	return "echo:"+msg
+}`,
+		},
+	}
+	pkgPath := "gno.land/r/test"
+	msg1 := NewMsgAddPackage(addr, pkgPath, files)
+	err := env.vmk.AddPackage(ctx, msg1)
+	assert.NoError(t, err)
+
+	// Call Echo function with wrong number of arguments
+	coins := std.MustParseCoins("1ugnot")
+	msg2 := NewMsgCall(addr, coins, pkgPath, "Echo", []string{"hello world", "extra arg"})
+	assert.PanicsWithValue(
+		t,
+		func() {
+			env.vmk.Call(ctx, msg2)
+		},
+		"wrong number of arguments in call to Echo: want 1 got 2",
+	)
+}