From 9c0bbd0bfd1dbc573acc85ddd5405ec594be26f3 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Mon, 8 Feb 2021 18:17:18 +0300 Subject: [PATCH 1/6] smartcontract: add NEP-11 standard check The only method missing is name, because it is provided in manifest. --- pkg/core/native/nonfungible_test.go | 14 ++ pkg/smartcontract/manifest/manifest.go | 2 + pkg/smartcontract/manifest/standard/check.go | 15 +++ pkg/smartcontract/manifest/standard/comply.go | 70 +++++++--- .../manifest/standard/comply_test.go | 23 ++-- pkg/smartcontract/manifest/standard/nep11.go | 121 ++++++++++++++++++ pkg/smartcontract/manifest/standard/nep17.go | 70 +++++----- pkg/smartcontract/manifest/standard/token.go | 30 +++++ 8 files changed, 273 insertions(+), 72 deletions(-) create mode 100644 pkg/core/native/nonfungible_test.go create mode 100644 pkg/smartcontract/manifest/standard/check.go create mode 100644 pkg/smartcontract/manifest/standard/nep11.go create mode 100644 pkg/smartcontract/manifest/standard/token.go diff --git a/pkg/core/native/nonfungible_test.go b/pkg/core/native/nonfungible_test.go new file mode 100644 index 000000000..9b7e9a25a --- /dev/null +++ b/pkg/core/native/nonfungible_test.go @@ -0,0 +1,14 @@ +package native + +import ( + "testing" + + "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest/standard" + "github.com/stretchr/testify/require" +) + +func TestNonfungibleNEP11(t *testing.T) { + n := newNonFungible("NFToken", -100, "SYM", 1) + require.NoError(t, standard.Check(&n.ContractMD.Manifest, manifest.NEP11StandardName)) +} diff --git a/pkg/smartcontract/manifest/manifest.go b/pkg/smartcontract/manifest/manifest.go index 31a3011a4..4603cac00 100644 --- a/pkg/smartcontract/manifest/manifest.go +++ b/pkg/smartcontract/manifest/manifest.go @@ -16,6 +16,8 @@ const ( // NEP10StandardName represents the name of NEP10 smartcontract standard. NEP10StandardName = "NEP-10" + // NEP11StandardName represents the name of NEP11 smartcontract standard. + NEP11StandardName = "NEP-11" // NEP17StandardName represents the name of NEP17 smartcontract standard. NEP17StandardName = "NEP-17" ) diff --git a/pkg/smartcontract/manifest/standard/check.go b/pkg/smartcontract/manifest/standard/check.go new file mode 100644 index 000000000..2957c265d --- /dev/null +++ b/pkg/smartcontract/manifest/standard/check.go @@ -0,0 +1,15 @@ +package standard + +import "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" + +// Standard represents smart-contract standard. +type Standard struct { + // Manifest describes mandatory methods and events. + manifest.Manifest + // Base contains base standard. + Base *Standard + // Optional contains optional contract methods. + // If contract contains method with the same name and parameter count, + // it must have signature declared by this contract. + Optional []manifest.Method +} diff --git a/pkg/smartcontract/manifest/standard/comply.go b/pkg/smartcontract/manifest/standard/comply.go index 1d03c55f1..fbd6f4625 100644 --- a/pkg/smartcontract/manifest/standard/comply.go +++ b/pkg/smartcontract/manifest/standard/comply.go @@ -17,17 +17,24 @@ var ( ErrSafeMethodMismatch = errors.New("method has wrong safe flag") ) -var checks = map[string]*manifest.Manifest{ - manifest.NEP17StandardName: nep17, +var checks = map[string][]*Standard{ + manifest.NEP11StandardName: {nep11NonDivisible, nep11Divisible}, + manifest.NEP17StandardName: {nep17}, } // Check checks if manifest complies with all provided standards. // Currently only NEP-17 is supported. func Check(m *manifest.Manifest, standards ...string) error { for i := range standards { - s, ok := checks[standards[i]] + ss, ok := checks[standards[i]] if ok { - if err := Comply(m, s); err != nil { + var err error + for i := range ss { + if err = Comply(m, ss[i]); err == nil { + break + } + } + if err != nil { return fmt.Errorf("manifest is not compliant with '%s': %w", standards[i], err) } } @@ -37,24 +44,15 @@ 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, st *manifest.Manifest) error { +func Comply(m *manifest.Manifest, st *Standard) error { + if st.Base != nil { + if err := Comply(m, st.Base); err != nil { + return err + } + } for _, stm := range st.ABI.Methods { - name := stm.Name - md := m.ABI.GetMethod(name, len(stm.Parameters)) - if md == nil { - return fmt.Errorf("%w: '%s' with %d parameters", ErrMethodMissing, name, len(stm.Parameters)) - } else if stm.ReturnType != md.ReturnType { - return fmt.Errorf("%w: '%s' (expected %s, got %s)", ErrInvalidReturnType, - name, stm.ReturnType, md.ReturnType) - } - for i := range stm.Parameters { - if stm.Parameters[i].Type != md.Parameters[i].Type { - return fmt.Errorf("%w: '%s'[%d] (expected %s, got %s)", ErrInvalidParameterType, - name, i, stm.Parameters[i].Type, md.Parameters[i].Type) - } - } - if stm.Safe != md.Safe { - return fmt.Errorf("%w: expected %t", ErrSafeMethodMismatch, stm.Safe) + if err := checkMethod(m, &stm, false); err != nil { + return err } } for _, ste := range st.ABI.Events { @@ -73,5 +71,35 @@ func Comply(m, st *manifest.Manifest) error { } } } + for _, stm := range st.Optional { + if err := checkMethod(m, &stm, true); err != nil { + return err + } + } + return nil +} + +func checkMethod(m *manifest.Manifest, expected *manifest.Method, allowMissing bool) error { + actual := m.ABI.GetMethod(expected.Name, len(expected.Parameters)) + if actual == nil { + if allowMissing { + return nil + } + return fmt.Errorf("%w: '%s' with %d parameters", ErrMethodMissing, + expected.Name, len(expected.Parameters)) + } + if expected.ReturnType != actual.ReturnType { + return fmt.Errorf("%w: '%s' (expected %s, got %s)", ErrInvalidReturnType, + expected.Name, expected.ReturnType, actual.ReturnType) + } + for i := range expected.Parameters { + 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) + } + } + if expected.Safe != actual.Safe { + return fmt.Errorf("%w: expected %t", ErrSafeMethodMismatch, expected.Safe) + } return nil } diff --git a/pkg/smartcontract/manifest/standard/comply_test.go b/pkg/smartcontract/manifest/standard/comply_test.go index da05947c5..9d2f6bf76 100644 --- a/pkg/smartcontract/manifest/standard/comply_test.go +++ b/pkg/smartcontract/manifest/standard/comply_test.go @@ -38,14 +38,14 @@ func fooMethodBarEvent() *manifest.Manifest { func TestComplyMissingMethod(t *testing.T) { m := fooMethodBarEvent() m.ABI.GetMethod("foo", -1).Name = "notafoo" - err := Comply(m, fooMethodBarEvent()) + err := Comply(m, &Standard{Manifest: *fooMethodBarEvent()}) require.True(t, errors.Is(err, ErrMethodMissing)) } func TestComplyInvalidReturnType(t *testing.T) { m := fooMethodBarEvent() m.ABI.GetMethod("foo", -1).ReturnType = smartcontract.VoidType - err := Comply(m, fooMethodBarEvent()) + err := Comply(m, &Standard{Manifest: *fooMethodBarEvent()}) require.True(t, errors.Is(err, ErrInvalidReturnType)) } @@ -54,14 +54,14 @@ func TestComplyMethodParameterCount(t *testing.T) { m := fooMethodBarEvent() f := m.ABI.GetMethod("foo", -1) f.Parameters = append(f.Parameters, manifest.Parameter{Type: smartcontract.BoolType}) - err := Comply(m, fooMethodBarEvent()) + err := Comply(m, &Standard{Manifest: *fooMethodBarEvent()}) require.True(t, errors.Is(err, ErrMethodMissing)) }) t.Run("Event", func(t *testing.T) { m := fooMethodBarEvent() ev := m.ABI.GetEvent("bar") ev.Parameters = append(ev.Parameters[:0]) - err := Comply(m, fooMethodBarEvent()) + err := Comply(m, &Standard{Manifest: *fooMethodBarEvent()}) require.True(t, errors.Is(err, ErrInvalidParameterCount)) }) } @@ -70,13 +70,13 @@ func TestComplyParameterType(t *testing.T) { t.Run("Method", func(t *testing.T) { m := fooMethodBarEvent() m.ABI.GetMethod("foo", -1).Parameters[0].Type = smartcontract.InteropInterfaceType - err := Comply(m, fooMethodBarEvent()) + err := Comply(m, &Standard{Manifest: *fooMethodBarEvent()}) require.True(t, errors.Is(err, ErrInvalidParameterType)) }) t.Run("Event", func(t *testing.T) { m := fooMethodBarEvent() m.ABI.GetEvent("bar").Parameters[0].Type = smartcontract.InteropInterfaceType - err := Comply(m, fooMethodBarEvent()) + err := Comply(m, &Standard{Manifest: *fooMethodBarEvent()}) require.True(t, errors.Is(err, ErrInvalidParameterType)) }) } @@ -84,14 +84,14 @@ func TestComplyParameterType(t *testing.T) { func TestMissingEvent(t *testing.T) { m := fooMethodBarEvent() m.ABI.GetEvent("bar").Name = "notabar" - err := Comply(m, fooMethodBarEvent()) + err := Comply(m, &Standard{Manifest: *fooMethodBarEvent()}) require.True(t, errors.Is(err, ErrEventMissing)) } func TestSafeFlag(t *testing.T) { m := fooMethodBarEvent() m.ABI.GetMethod("foo", -1).Safe = false - err := Comply(m, fooMethodBarEvent()) + err := Comply(m, &Standard{Manifest: *fooMethodBarEvent()}) require.True(t, errors.Is(err, ErrSafeMethodMismatch)) } @@ -109,12 +109,15 @@ func TestComplyValid(t *testing.T) { Type: smartcontract.IntegerType, }}, }) - require.NoError(t, Comply(m, fooMethodBarEvent())) + require.NoError(t, Comply(m, &Standard{Manifest: *fooMethodBarEvent()})) } func TestCheck(t *testing.T) { m := manifest.NewManifest("Test") require.Error(t, Check(m, manifest.NEP17StandardName)) - require.NoError(t, Check(nep17, manifest.NEP17StandardName)) + m.ABI.Methods = append(m.ABI.Methods, decimalTokenBase.ABI.Methods...) + 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)) } diff --git a/pkg/smartcontract/manifest/standard/nep11.go b/pkg/smartcontract/manifest/standard/nep11.go new file mode 100644 index 000000000..676209b1d --- /dev/null +++ b/pkg/smartcontract/manifest/standard/nep11.go @@ -0,0 +1,121 @@ +package standard + +import ( + "github.com/nspcc-dev/neo-go/pkg/smartcontract" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" +) + +var nep11Base = &Standard{ + Base: decimalTokenBase, + Manifest: manifest.Manifest{ + ABI: manifest.ABI{ + Methods: []manifest.Method{ + { + Name: "balanceOf", + Parameters: []manifest.Parameter{ + {Type: smartcontract.Hash160Type}, + }, + ReturnType: smartcontract.IntegerType, + Safe: true, + }, + { + Name: "tokensOf", + ReturnType: smartcontract.AnyType, // Iterator + Parameters: []manifest.Parameter{ + {Type: smartcontract.Hash160Type}, + }, + Safe: true, + }, + { + Name: "transfer", + Parameters: []manifest.Parameter{ + {Type: smartcontract.Hash160Type}, + {Type: smartcontract.ByteArrayType}, + }, + ReturnType: smartcontract.BoolType, + }, + }, + Events: []manifest.Event{ + { + Name: "Transfer", + Parameters: []manifest.Parameter{ + {Type: smartcontract.Hash160Type}, + {Type: smartcontract.Hash160Type}, + {Type: smartcontract.IntegerType}, + {Type: smartcontract.ByteArrayType}, + }, + }, + }, + }, + }, + Optional: []manifest.Method{ + { + Name: "properties", + Parameters: []manifest.Parameter{ + {Type: smartcontract.ByteArrayType}, + }, + ReturnType: smartcontract.MapType, + Safe: true, + }, + { + Name: "tokens", + ReturnType: smartcontract.AnyType, + Safe: true, + }, + }, +} + +var nep11NonDivisible = &Standard{ + Base: nep11Base, + Manifest: manifest.Manifest{ + ABI: manifest.ABI{ + Methods: []manifest.Method{ + { + Name: "ownerOf", + Parameters: []manifest.Parameter{ + {Type: smartcontract.ByteArrayType}, + }, + ReturnType: smartcontract.Hash160Type, + Safe: true, + }, + }, + }, + }, +} + +var nep11Divisible = &Standard{ + Base: nep11Base, + Manifest: manifest.Manifest{ + ABI: manifest.ABI{ + Methods: []manifest.Method{ + { + Name: "balanceOf", + Parameters: []manifest.Parameter{ + {Type: smartcontract.Hash160Type}, + {Type: smartcontract.ByteArrayType}, + }, + ReturnType: smartcontract.IntegerType, + Safe: true, + }, + { + Name: "ownerOf", + Parameters: []manifest.Parameter{ + {Type: smartcontract.ByteArrayType}, + }, + ReturnType: smartcontract.AnyType, + Safe: true, + }, + { + Name: "transfer", + Parameters: []manifest.Parameter{ + {Type: smartcontract.Hash160Type}, + {Type: smartcontract.Hash160Type}, + {Type: smartcontract.IntegerType}, + {Type: smartcontract.ByteArrayType}, + }, + ReturnType: smartcontract.BoolType, + }, + }, + }, + }, +} diff --git a/pkg/smartcontract/manifest/standard/nep17.go b/pkg/smartcontract/manifest/standard/nep17.go index 098ce6cfc..685afba3d 100644 --- a/pkg/smartcontract/manifest/standard/nep17.go +++ b/pkg/smartcontract/manifest/standard/nep17.go @@ -5,50 +5,38 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" ) -var nep17 = &manifest.Manifest{ - ABI: manifest.ABI{ - Methods: []manifest.Method{ - { - Name: "balanceOf", - Parameters: []manifest.Parameter{ - {Type: smartcontract.Hash160Type}, +var nep17 = &Standard{ + Base: decimalTokenBase, + Manifest: manifest.Manifest{ + ABI: manifest.ABI{ + Methods: []manifest.Method{ + { + Name: "balanceOf", + Parameters: []manifest.Parameter{ + {Type: smartcontract.Hash160Type}, + }, + ReturnType: smartcontract.IntegerType, + Safe: true, }, - ReturnType: smartcontract.IntegerType, - Safe: true, - }, - { - Name: "decimals", - ReturnType: smartcontract.IntegerType, - Safe: true, - }, - { - Name: "symbol", - ReturnType: smartcontract.StringType, - Safe: true, - }, - { - Name: "totalSupply", - ReturnType: smartcontract.IntegerType, - Safe: true, - }, - { - Name: "transfer", - Parameters: []manifest.Parameter{ - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.IntegerType}, - {Type: smartcontract.AnyType}, + { + Name: "transfer", + Parameters: []manifest.Parameter{ + {Type: smartcontract.Hash160Type}, + {Type: smartcontract.Hash160Type}, + {Type: smartcontract.IntegerType}, + {Type: smartcontract.AnyType}, + }, + ReturnType: smartcontract.BoolType, }, - ReturnType: smartcontract.BoolType, }, - }, - Events: []manifest.Event{ - { - Name: "Transfer", - Parameters: []manifest.Parameter{ - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.Hash160Type}, - {Type: smartcontract.IntegerType}, + Events: []manifest.Event{ + { + Name: "Transfer", + Parameters: []manifest.Parameter{ + {Type: smartcontract.Hash160Type}, + {Type: smartcontract.Hash160Type}, + {Type: smartcontract.IntegerType}, + }, }, }, }, diff --git a/pkg/smartcontract/manifest/standard/token.go b/pkg/smartcontract/manifest/standard/token.go new file mode 100644 index 000000000..877050131 --- /dev/null +++ b/pkg/smartcontract/manifest/standard/token.go @@ -0,0 +1,30 @@ +package standard + +import ( + "github.com/nspcc-dev/neo-go/pkg/smartcontract" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" +) + +var decimalTokenBase = &Standard{ + Manifest: manifest.Manifest{ + ABI: manifest.ABI{ + Methods: []manifest.Method{ + { + Name: "decimals", + ReturnType: smartcontract.IntegerType, + Safe: true, + }, + { + Name: "symbol", + ReturnType: smartcontract.StringType, + Safe: true, + }, + { + Name: "totalSupply", + ReturnType: smartcontract.IntegerType, + Safe: true, + }, + }, + }, + }, +} From eb26a61078b9ce2fdf3079528d49496e1f57e0a5 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Fri, 19 Feb 2021 11:53:12 +0300 Subject: [PATCH 2/6] smartcontract: remove `IsNEP17()` --- pkg/smartcontract/manifest/standard/nep17.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/smartcontract/manifest/standard/nep17.go b/pkg/smartcontract/manifest/standard/nep17.go index 685afba3d..0b1abfb18 100644 --- a/pkg/smartcontract/manifest/standard/nep17.go +++ b/pkg/smartcontract/manifest/standard/nep17.go @@ -42,8 +42,3 @@ var nep17 = &Standard{ }, }, } - -// IsNEP17 checks if m is NEP-17 compliant. -func IsNEP17(m *manifest.Manifest) error { - return Comply(m, nep17) -} From ae30e30321628fd006d8aa49fcd1a6ff452cab28 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Fri, 19 Feb 2021 11:56:15 +0300 Subject: [PATCH 3/6] native: add NEP-11 check for name service --- pkg/core/native/name_service_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/core/native/name_service_test.go b/pkg/core/native/name_service_test.go index 978f294b3..c8fccf0e7 100644 --- a/pkg/core/native/name_service_test.go +++ b/pkg/core/native/name_service_test.go @@ -3,6 +3,8 @@ package native import ( "testing" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest/standard" "github.com/stretchr/testify/require" ) @@ -86,3 +88,8 @@ func TestNameService_CheckName(t *testing.T) { } } } + +func TestNameService_NEP11(t *testing.T) { + ns := newNameService() + require.NoError(t, standard.Check(&ns.Manifest, manifest.NEP11StandardName)) +} From 9d4ccf0fcc2a0256a2ffe76f88519b8cf3923af8 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Fri, 19 Feb 2021 12:05:30 +0300 Subject: [PATCH 4/6] smartcontract: add test for Optional methods They should be checked only if name + parameter count matches. This is how methods are identified in NEO. --- .../manifest/standard/comply_test.go | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/pkg/smartcontract/manifest/standard/comply_test.go b/pkg/smartcontract/manifest/standard/comply_test.go index 9d2f6bf76..a7f7e03e5 100644 --- a/pkg/smartcontract/manifest/standard/comply_test.go +++ b/pkg/smartcontract/manifest/standard/comply_test.go @@ -121,3 +121,39 @@ func TestCheck(t *testing.T) { m.ABI.Events = append(m.ABI.Events, nep17.ABI.Events...) require.NoError(t, Check(m, manifest.NEP17StandardName)) } + +func TestOptional(t *testing.T) { + var m Standard + m.Optional = []manifest.Method{{ + Name: "optMet", + Parameters: []manifest.Parameter{{Type: smartcontract.ByteArrayType}}, + ReturnType: smartcontract.IntegerType, + }} + + t.Run("wrong parameter count, do not check", func(t *testing.T) { + var actual manifest.Manifest + actual.ABI.Methods = []manifest.Method{{ + Name: "optMet", + ReturnType: smartcontract.IntegerType, + }} + require.NoError(t, Comply(&actual, &m)) + }) + t.Run("good parameter count, bad return", func(t *testing.T) { + var actual manifest.Manifest + actual.ABI.Methods = []manifest.Method{{ + Name: "optMet", + Parameters: []manifest.Parameter{{Type: smartcontract.ArrayType}}, + ReturnType: smartcontract.IntegerType, + }} + require.Error(t, Comply(&actual, &m)) + }) + t.Run("good parameter count, good return", func(t *testing.T) { + var actual manifest.Manifest + actual.ABI.Methods = []manifest.Method{{ + Name: "optMet", + Parameters: []manifest.Parameter{{Type: smartcontract.ByteArrayType}}, + ReturnType: smartcontract.IntegerType, + }} + require.NoError(t, Comply(&actual, &m)) + }) +} From d89f055697d30df203fb267ad32585c90d7d5133 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Fri, 19 Feb 2021 13:13:36 +0300 Subject: [PATCH 5/6] 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}, }, }, }, From b38443fe2042d5921ca196e998c81c246d7b3aff Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Fri, 19 Feb 2021 13:48:45 +0300 Subject: [PATCH 6/6] smartcontract: add checks for onNEP*Payable methods When they are present in contract, we want them to have correct signature. --- pkg/compiler/compiler.go | 77 ++++++++++++------- pkg/compiler/compiler_test.go | 35 +++++++++ pkg/smartcontract/manifest/manifest.go | 4 + pkg/smartcontract/manifest/standard/comply.go | 2 + .../manifest/standard/payable.go | 38 +++++++++ 5 files changed, 127 insertions(+), 29 deletions(-) create mode 100644 pkg/smartcontract/manifest/standard/payable.go diff --git a/pkg/compiler/compiler.go b/pkg/compiler/compiler.go index 8086006ef..0545c716e 100644 --- a/pkg/compiler/compiler.go +++ b/pkg/compiler/compiler.go @@ -222,36 +222,9 @@ func CompileAndSave(src string, o *Options) ([]byte, error) { } if o.ManifestFile != "" { - m, err := di.ConvertToManifest(o) + m, err := CreateManifest(di, o) if err != nil { - return b, fmt.Errorf("failed to convert debug info to manifest: %w", err) - } - if !o.NoStandardCheck { - if err := standard.CheckABI(m, o.ContractSupportedStandards...); err != nil { - return b, err - } - } - if !o.NoEventsCheck { - for name := range di.EmittedEvents { - ev := m.ABI.GetEvent(name) - if ev == nil { - return nil, fmt.Errorf("event '%s' is emitted but not specified in manifest", name) - } - argsList := di.EmittedEvents[name] - for i := range argsList { - if len(argsList[i]) != len(ev.Parameters) { - return nil, fmt.Errorf("event '%s' should have %d parameters but has %d", - name, len(ev.Parameters), len(argsList[i])) - } - for j := range ev.Parameters { - expected := ev.Parameters[j].Type.String() - if argsList[i][j] != expected { - return nil, fmt.Errorf("event '%s' should have '%s' as type of %d parameter, "+ - "got: %s", name, expected, j+1, argsList[i][j]) - } - } - } - } + return b, err } mData, err := json.Marshal(m) if err != nil { @@ -262,3 +235,49 @@ func CompileAndSave(src string, o *Options) ([]byte, error) { return b, nil } + +// CreateManifest creates manifest and checks that is is valid. +func CreateManifest(di *DebugInfo, o *Options) (*manifest.Manifest, error) { + m, err := di.ConvertToManifest(o) + if err != nil { + return m, fmt.Errorf("failed to convert debug info to manifest: %w", err) + } + if !o.NoStandardCheck { + if err := standard.CheckABI(m, o.ContractSupportedStandards...); err != nil { + return m, err + } + if m.ABI.GetMethod(manifest.MethodOnNEP11Payment, -1) != nil { + if err := standard.CheckABI(m, manifest.NEP11Payable); err != nil { + return m, err + } + } + if m.ABI.GetMethod(manifest.MethodOnNEP17Payment, -1) != nil { + if err := standard.CheckABI(m, manifest.NEP17Payable); err != nil { + return m, err + } + } + } + if !o.NoEventsCheck { + for name := range di.EmittedEvents { + ev := m.ABI.GetEvent(name) + if ev == nil { + return nil, fmt.Errorf("event '%s' is emitted but not specified in manifest", name) + } + argsList := di.EmittedEvents[name] + for i := range argsList { + if len(argsList[i]) != len(ev.Parameters) { + return nil, fmt.Errorf("event '%s' should have %d parameters but has %d", + name, len(ev.Parameters), len(argsList[i])) + } + for j := range ev.Parameters { + expected := ev.Parameters[j].Type.String() + if argsList[i][j] != expected { + return nil, fmt.Errorf("event '%s' should have '%s' as type of %d parameter, "+ + "got: %s", name, expected, j+1, argsList[i][j]) + } + } + } + } + } + return m, nil +} diff --git a/pkg/compiler/compiler_test.go b/pkg/compiler/compiler_test.go index c9aad096d..5ab8e614f 100644 --- a/pkg/compiler/compiler_test.go +++ b/pkg/compiler/compiler_test.go @@ -4,6 +4,7 @@ import ( "io/ioutil" "os" "path" + "strings" "testing" "github.com/nspcc-dev/neo-go/pkg/compiler" @@ -90,3 +91,37 @@ func compileFile(src string) error { _, err := compiler.Compile(src, nil) return err } + +func TestOnPayableChecks(t *testing.T) { + compileAndCheck := func(t *testing.T, src string) error { + _, di, err := compiler.CompileWithDebugInfo("payable", strings.NewReader(src)) + require.NoError(t, err) + _, err = compiler.CreateManifest(di, &compiler.Options{}) + return err + } + + t.Run("NEP-11, good", func(t *testing.T) { + src := `package payable + import "github.com/nspcc-dev/neo-go/pkg/interop" + func OnNEP11Payment(from interop.Hash160, amount int, tokenID []byte) {}` + require.NoError(t, compileAndCheck(t, src)) + }) + t.Run("NEP-11, bad", func(t *testing.T) { + src := `package payable + import "github.com/nspcc-dev/neo-go/pkg/interop" + func OnNEP11Payment(from interop.Hash160, amount int, oldParam string, tokenID []byte) {}` + require.Error(t, compileAndCheck(t, src)) + }) + t.Run("NEP-17, good", func(t *testing.T) { + src := `package payable + import "github.com/nspcc-dev/neo-go/pkg/interop" + func OnNEP17Payment(from interop.Hash160, amount int, data interface{}) {}` + require.NoError(t, compileAndCheck(t, src)) + }) + t.Run("NEP-17, bad", func(t *testing.T) { + src := `package payable + import "github.com/nspcc-dev/neo-go/pkg/interop" + func OnNEP17Payment(from interop.Hash160, amount int, data interface{}, extra int) {}` + require.Error(t, compileAndCheck(t, src)) + }) +} diff --git a/pkg/smartcontract/manifest/manifest.go b/pkg/smartcontract/manifest/manifest.go index 4603cac00..a1a2d4c9f 100644 --- a/pkg/smartcontract/manifest/manifest.go +++ b/pkg/smartcontract/manifest/manifest.go @@ -20,6 +20,10 @@ const ( NEP11StandardName = "NEP-11" // NEP17StandardName represents the name of NEP17 smartcontract standard. NEP17StandardName = "NEP-17" + // NEP11Payable represents the name of contract interface which can receive NEP-11 tokens. + NEP11Payable = "NEP-11-Payable" + // NEP17Payable represents the name of contract interface which can receive NEP-17 tokens. + NEP17Payable = "NEP-17-Payable" ) // Manifest represens contract metadata. diff --git a/pkg/smartcontract/manifest/standard/comply.go b/pkg/smartcontract/manifest/standard/comply.go index fd7eda2af..35986b225 100644 --- a/pkg/smartcontract/manifest/standard/comply.go +++ b/pkg/smartcontract/manifest/standard/comply.go @@ -21,6 +21,8 @@ var ( var checks = map[string][]*Standard{ manifest.NEP11StandardName: {nep11NonDivisible, nep11Divisible}, manifest.NEP17StandardName: {nep17}, + manifest.NEP11Payable: {nep11payable}, + manifest.NEP17Payable: {nep17payable}, } // Check checks if manifest complies with all provided standards. diff --git a/pkg/smartcontract/manifest/standard/payable.go b/pkg/smartcontract/manifest/standard/payable.go new file mode 100644 index 000000000..a588b45a8 --- /dev/null +++ b/pkg/smartcontract/manifest/standard/payable.go @@ -0,0 +1,38 @@ +package standard + +import ( + "github.com/nspcc-dev/neo-go/pkg/smartcontract" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" +) + +var nep11payable = &Standard{ + Manifest: manifest.Manifest{ + ABI: manifest.ABI{ + Methods: []manifest.Method{{ + Name: manifest.MethodOnNEP11Payment, + Parameters: []manifest.Parameter{ + {Name: "from", Type: smartcontract.Hash160Type}, + {Name: "amount", Type: smartcontract.IntegerType}, + {Name: "tokenid", Type: smartcontract.ByteArrayType}, + }, + ReturnType: smartcontract.VoidType, + }}, + }, + }, +} + +var nep17payable = &Standard{ + Manifest: manifest.Manifest{ + ABI: manifest.ABI{ + Methods: []manifest.Method{{ + Name: manifest.MethodOnNEP17Payment, + Parameters: []manifest.Parameter{ + {Name: "from", Type: smartcontract.Hash160Type}, + {Name: "amount", Type: smartcontract.IntegerType}, + {Name: "data", Type: smartcontract.AnyType}, + }, + ReturnType: smartcontract.VoidType, + }}, + }, + }, +}