From d89f055697d30df203fb267ad32585c90d7d5133 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Fri, 19 Feb 2021 13:13:36 +0300 Subject: [PATCH] smartcontract: add checks for parameter names It can be annoying to use parameter names as specified by standard, so a separate `CheckABI` is supported. --- pkg/compiler/compiler.go | 2 +- pkg/smartcontract/manifest/standard/comply.go | 38 ++++++++++++++++--- .../manifest/standard/comply_test.go | 20 ++++++++++ pkg/smartcontract/manifest/standard/nep11.go | 34 ++++++++--------- pkg/smartcontract/manifest/standard/nep17.go | 16 ++++---- 5 files changed, 79 insertions(+), 31 deletions(-) diff --git a/pkg/compiler/compiler.go b/pkg/compiler/compiler.go index 02987acb7..8086006ef 100644 --- a/pkg/compiler/compiler.go +++ b/pkg/compiler/compiler.go @@ -227,7 +227,7 @@ func CompileAndSave(src string, o *Options) ([]byte, error) { return b, fmt.Errorf("failed to convert debug info to manifest: %w", err) } if !o.NoStandardCheck { - if err := standard.Check(m, o.ContractSupportedStandards...); err != nil { + if err := standard.CheckABI(m, o.ContractSupportedStandards...); err != nil { return b, err } } diff --git a/pkg/smartcontract/manifest/standard/comply.go b/pkg/smartcontract/manifest/standard/comply.go index fbd6f4625..fd7eda2af 100644 --- a/pkg/smartcontract/manifest/standard/comply.go +++ b/pkg/smartcontract/manifest/standard/comply.go @@ -13,6 +13,7 @@ var ( ErrEventMissing = errors.New("event missing") ErrInvalidReturnType = errors.New("invalid return type") ErrInvalidParameterCount = errors.New("invalid parameter count") + ErrInvalidParameterName = errors.New("invalid parameter name") ErrInvalidParameterType = errors.New("invalid parameter type") ErrSafeMethodMismatch = errors.New("method has wrong safe flag") ) @@ -25,12 +26,21 @@ var checks = map[string][]*Standard{ // Check checks if manifest complies with all provided standards. // Currently only NEP-17 is supported. func Check(m *manifest.Manifest, standards ...string) error { + return check(m, true, standards...) +} + +// CheckABI is similar to Check but doesn't check parameter names. +func CheckABI(m *manifest.Manifest, standards ...string) error { + return check(m, false, standards...) +} + +func check(m *manifest.Manifest, checkNames bool, standards ...string) error { for i := range standards { ss, ok := checks[standards[i]] if ok { var err error for i := range ss { - if err = Comply(m, ss[i]); err == nil { + if err = comply(m, checkNames, ss[i]); err == nil { break } } @@ -45,13 +55,22 @@ func Check(m *manifest.Manifest, standards ...string) error { // Comply if m has all methods and event from st manifest and they have the same signature. // Parameter names are ignored. func Comply(m *manifest.Manifest, st *Standard) error { + return comply(m, true, st) +} + +// ComplyABI is similar to comply but doesn't check parameter names. +func ComplyABI(m *manifest.Manifest, st *Standard) error { + return comply(m, false, st) +} + +func comply(m *manifest.Manifest, checkNames bool, st *Standard) error { if st.Base != nil { - if err := Comply(m, st.Base); err != nil { + if err := comply(m, checkNames, st.Base); err != nil { return err } } for _, stm := range st.ABI.Methods { - if err := checkMethod(m, &stm, false); err != nil { + if err := checkMethod(m, &stm, false, checkNames); err != nil { return err } } @@ -65,6 +84,10 @@ func Comply(m *manifest.Manifest, st *Standard) error { name, len(ste.Parameters), len(ed.Parameters)) } for i := range ste.Parameters { + if checkNames && ste.Parameters[i].Name != ed.Parameters[i].Name { + return fmt.Errorf("%w: event '%s'[%d] (expected %s, got %s)", ErrInvalidParameterName, + name, i, ste.Parameters[i].Name, ed.Parameters[i].Name) + } if ste.Parameters[i].Type != ed.Parameters[i].Type { return fmt.Errorf("%w: event '%s' (expected %s, got %s)", ErrInvalidParameterType, name, ste.Parameters[i].Type, ed.Parameters[i].Type) @@ -72,14 +95,15 @@ func Comply(m *manifest.Manifest, st *Standard) error { } } for _, stm := range st.Optional { - if err := checkMethod(m, &stm, true); err != nil { + if err := checkMethod(m, &stm, true, checkNames); err != nil { return err } } return nil } -func checkMethod(m *manifest.Manifest, expected *manifest.Method, allowMissing bool) error { +func checkMethod(m *manifest.Manifest, expected *manifest.Method, + allowMissing bool, checkNames bool) error { actual := m.ABI.GetMethod(expected.Name, len(expected.Parameters)) if actual == nil { if allowMissing { @@ -93,6 +117,10 @@ func checkMethod(m *manifest.Manifest, expected *manifest.Method, allowMissing b expected.Name, expected.ReturnType, actual.ReturnType) } for i := range expected.Parameters { + if checkNames && expected.Parameters[i].Name != actual.Parameters[i].Name { + return fmt.Errorf("%w: '%s'[%d] (expected %s, got %s)", ErrInvalidParameterName, + expected.Name, i, expected.Parameters[i].Name, actual.Parameters[i].Name) + } if expected.Parameters[i].Type != actual.Parameters[i].Type { return fmt.Errorf("%w: '%s'[%d] (expected %s, got %s)", ErrInvalidParameterType, expected.Name, i, expected.Parameters[i].Type, actual.Parameters[i].Type) diff --git a/pkg/smartcontract/manifest/standard/comply_test.go b/pkg/smartcontract/manifest/standard/comply_test.go index a7f7e03e5..159815cc0 100644 --- a/pkg/smartcontract/manifest/standard/comply_test.go +++ b/pkg/smartcontract/manifest/standard/comply_test.go @@ -81,6 +81,25 @@ func TestComplyParameterType(t *testing.T) { }) } +func TestComplyParameterName(t *testing.T) { + t.Run("Method", func(t *testing.T) { + m := fooMethodBarEvent() + m.ABI.GetMethod("foo", -1).Parameters[0].Name = "hehe" + s := &Standard{Manifest: *fooMethodBarEvent()} + err := Comply(m, s) + require.True(t, errors.Is(err, ErrInvalidParameterName)) + require.NoError(t, ComplyABI(m, s)) + }) + t.Run("Event", func(t *testing.T) { + m := fooMethodBarEvent() + m.ABI.GetEvent("bar").Parameters[0].Name = "hehe" + s := &Standard{Manifest: *fooMethodBarEvent()} + err := Comply(m, s) + require.True(t, errors.Is(err, ErrInvalidParameterName)) + require.NoError(t, ComplyABI(m, s)) + }) +} + func TestMissingEvent(t *testing.T) { m := fooMethodBarEvent() m.ABI.GetEvent("bar").Name = "notabar" @@ -120,6 +139,7 @@ func TestCheck(t *testing.T) { m.ABI.Methods = append(m.ABI.Methods, nep17.ABI.Methods...) m.ABI.Events = append(m.ABI.Events, nep17.ABI.Events...) require.NoError(t, Check(m, manifest.NEP17StandardName)) + require.NoError(t, CheckABI(m, manifest.NEP17StandardName)) } func TestOptional(t *testing.T) { diff --git a/pkg/smartcontract/manifest/standard/nep11.go b/pkg/smartcontract/manifest/standard/nep11.go index 676209b1d..7d090a196 100644 --- a/pkg/smartcontract/manifest/standard/nep11.go +++ b/pkg/smartcontract/manifest/standard/nep11.go @@ -13,7 +13,7 @@ var nep11Base = &Standard{ { Name: "balanceOf", Parameters: []manifest.Parameter{ - {Type: smartcontract.Hash160Type}, + {Name: "owner", Type: smartcontract.Hash160Type}, }, ReturnType: smartcontract.IntegerType, Safe: true, @@ -22,15 +22,15 @@ var nep11Base = &Standard{ Name: "tokensOf", ReturnType: smartcontract.AnyType, // Iterator Parameters: []manifest.Parameter{ - {Type: smartcontract.Hash160Type}, + {Name: "owner", Type: smartcontract.Hash160Type}, }, Safe: true, }, { Name: "transfer", Parameters: []manifest.Parameter{ - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.ByteArrayType}, + {Name: "to", Type: smartcontract.Hash160Type}, + {Name: "tokenId", Type: smartcontract.ByteArrayType}, }, ReturnType: smartcontract.BoolType, }, @@ -39,10 +39,10 @@ var nep11Base = &Standard{ { Name: "Transfer", Parameters: []manifest.Parameter{ - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.IntegerType}, - {Type: smartcontract.ByteArrayType}, + {Name: "from", Type: smartcontract.Hash160Type}, + {Name: "to", Type: smartcontract.Hash160Type}, + {Name: "amount", Type: smartcontract.IntegerType}, + {Name: "tokenId", Type: smartcontract.ByteArrayType}, }, }, }, @@ -52,7 +52,7 @@ var nep11Base = &Standard{ { Name: "properties", Parameters: []manifest.Parameter{ - {Type: smartcontract.ByteArrayType}, + {Name: "tokenId", Type: smartcontract.ByteArrayType}, }, ReturnType: smartcontract.MapType, Safe: true, @@ -73,7 +73,7 @@ var nep11NonDivisible = &Standard{ { Name: "ownerOf", Parameters: []manifest.Parameter{ - {Type: smartcontract.ByteArrayType}, + {Name: "tokenId", Type: smartcontract.ByteArrayType}, }, ReturnType: smartcontract.Hash160Type, Safe: true, @@ -91,8 +91,8 @@ var nep11Divisible = &Standard{ { Name: "balanceOf", Parameters: []manifest.Parameter{ - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.ByteArrayType}, + {Name: "owner", Type: smartcontract.Hash160Type}, + {Name: "tokenId", Type: smartcontract.ByteArrayType}, }, ReturnType: smartcontract.IntegerType, Safe: true, @@ -100,7 +100,7 @@ var nep11Divisible = &Standard{ { Name: "ownerOf", Parameters: []manifest.Parameter{ - {Type: smartcontract.ByteArrayType}, + {Name: "tokenId", Type: smartcontract.ByteArrayType}, }, ReturnType: smartcontract.AnyType, Safe: true, @@ -108,10 +108,10 @@ var nep11Divisible = &Standard{ { Name: "transfer", Parameters: []manifest.Parameter{ - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.IntegerType}, - {Type: smartcontract.ByteArrayType}, + {Name: "from", Type: smartcontract.Hash160Type}, + {Name: "to", Type: smartcontract.Hash160Type}, + {Name: "amount", Type: smartcontract.IntegerType}, + {Name: "tokenId", Type: smartcontract.ByteArrayType}, }, ReturnType: smartcontract.BoolType, }, diff --git a/pkg/smartcontract/manifest/standard/nep17.go b/pkg/smartcontract/manifest/standard/nep17.go index 0b1abfb18..4e08c4120 100644 --- a/pkg/smartcontract/manifest/standard/nep17.go +++ b/pkg/smartcontract/manifest/standard/nep17.go @@ -13,7 +13,7 @@ var nep17 = &Standard{ { Name: "balanceOf", Parameters: []manifest.Parameter{ - {Type: smartcontract.Hash160Type}, + {Name: "account", Type: smartcontract.Hash160Type}, }, ReturnType: smartcontract.IntegerType, Safe: true, @@ -21,10 +21,10 @@ var nep17 = &Standard{ { Name: "transfer", Parameters: []manifest.Parameter{ - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.IntegerType}, - {Type: smartcontract.AnyType}, + {Name: "from", Type: smartcontract.Hash160Type}, + {Name: "to", Type: smartcontract.Hash160Type}, + {Name: "amount", Type: smartcontract.IntegerType}, + {Name: "data", Type: smartcontract.AnyType}, }, ReturnType: smartcontract.BoolType, }, @@ -33,9 +33,9 @@ var nep17 = &Standard{ { Name: "Transfer", Parameters: []manifest.Parameter{ - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.IntegerType}, + {Name: "from", Type: smartcontract.Hash160Type}, + {Name: "to", Type: smartcontract.Hash160Type}, + {Name: "amount", Type: smartcontract.IntegerType}, }, }, },