From f65485b73565f645268e361a3d3259e151db4248 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 4 Mar 2021 13:26:16 +0300 Subject: [PATCH] core: remove System.Binary.Itoa and System.Binary.Atoi syscalls And move their tests to native StdLib. --- examples/timer/timer.go | 6 +- pkg/compiler/inline_test.go | 17 +++-- pkg/compiler/syscall_test.go | 2 - pkg/compiler/vm_test.go | 30 +++----- pkg/core/interop/binary/itoa.go | 91 ------------------------ pkg/core/interop/binary/itoa_test.go | 98 -------------------------- pkg/core/interop/interopnames/names.go | 4 -- pkg/core/interops.go | 2 - pkg/core/native/std_test.go | 96 +++++++++++++++++++++++++ pkg/interop/binary/binary.go | 12 ---- 10 files changed, 117 insertions(+), 241 deletions(-) delete mode 100644 pkg/core/interop/binary/itoa.go delete mode 100644 pkg/core/interop/binary/itoa_test.go create mode 100644 pkg/core/native/std_test.go diff --git a/examples/timer/timer.go b/examples/timer/timer.go index a38d90ce1..6cbc02d2f 100644 --- a/examples/timer/timer.go +++ b/examples/timer/timer.go @@ -2,8 +2,8 @@ package timer import ( "github.com/nspcc-dev/neo-go/pkg/interop" - "github.com/nspcc-dev/neo-go/pkg/interop/binary" "github.com/nspcc-dev/neo-go/pkg/interop/contract" + "github.com/nspcc-dev/neo-go/pkg/interop/native/std" "github.com/nspcc-dev/neo-go/pkg/interop/runtime" "github.com/nspcc-dev/neo-go/pkg/interop/storage" "github.com/nspcc-dev/neo-go/pkg/interop/util" @@ -35,7 +35,7 @@ func _deploy(_ interface{}, isUpdate bool) { sh := runtime.GetCallingScriptHash() storage.Put(ctx, mgmtKey, sh) storage.Put(ctx, ticksKey, defaultTicks) - i := binary.Itoa(defaultTicks, 10) + i := std.Itoa(defaultTicks, 10) runtime.Log("Timer set to " + i + " ticks.") } @@ -61,7 +61,7 @@ func Tick() bool { return contract.Call(runtime.GetExecutingScriptHash(), "selfDestroy", contract.All).(bool) } storage.Put(ctx, ticksKey, ticksLeft) - i := binary.Itoa(ticksLeft.(int), 10) + i := std.Itoa(ticksLeft.(int), 10) runtime.Log(i + " ticks left.") return true } diff --git a/pkg/compiler/inline_test.go b/pkg/compiler/inline_test.go index c7dee4599..eeb9bfe3a 100644 --- a/pkg/compiler/inline_test.go +++ b/pkg/compiler/inline_test.go @@ -118,13 +118,14 @@ func TestInline(t *testing.T) { func TestInlineInLoop(t *testing.T) { t.Run("simple", func(t *testing.T) { src := `package foo - import "github.com/nspcc-dev/neo-go/pkg/interop/binary" + import "github.com/nspcc-dev/neo-go/pkg/interop/storage" import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/inline" func Main() int { sum := 0 values := []int{10, 11} for _, v := range values { - binary.Itoa(v, 10) + _ = v // use 'v' + storage.GetContext() // push something on stack to check it's dropped sum += inline.VarSum(1, 2, 3, 4) } return sum @@ -133,14 +134,16 @@ func TestInlineInLoop(t *testing.T) { }) t.Run("inlined argument", func(t *testing.T) { src := `package foo - import "github.com/nspcc-dev/neo-go/pkg/interop/binary" + import "github.com/nspcc-dev/neo-go/pkg/interop/runtime" + import "github.com/nspcc-dev/neo-go/pkg/interop/storage" import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/inline" func Main() int { sum := 0 values := []int{10, 11} for _, v := range values { - binary.Itoa(v, 10) - sum += inline.VarSum(1, 2, 3, binary.Atoi("4", 10)) + _ = v // use 'v' + storage.GetContext() // push something on stack to check it's dropped + sum += inline.VarSum(1, 2, 3, runtime.GetTime()) // runtime.GetTime always returns 4 in these tests } return sum }` @@ -148,12 +151,12 @@ func TestInlineInLoop(t *testing.T) { }) t.Run("check clean stack on return", func(t *testing.T) { src := `package foo - import "github.com/nspcc-dev/neo-go/pkg/interop/binary" + import "github.com/nspcc-dev/neo-go/pkg/interop/storage" import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/inline" func Main() int { values := []int{10, 11, 12} for _, v := range values { - binary.Itoa(v, 10) + storage.GetContext() // push something on stack to check it's dropped if v == 11 { return inline.VarSum(2, 20, 200) } diff --git a/pkg/compiler/syscall_test.go b/pkg/compiler/syscall_test.go index 7c756c627..01f91038e 100644 --- a/pkg/compiler/syscall_test.go +++ b/pkg/compiler/syscall_test.go @@ -61,13 +61,11 @@ func TestSyscallExecution(t *testing.T) { sigs := "[]interop.Signature{" + sig + "}" sctx := "storage.Context{}" interops := map[string]syscallTestCase{ - "binary.Atoi": {interopnames.SystemBinaryAtoi, []string{`"123"`, "10"}, false}, "binary.Base58Decode": {interopnames.SystemBinaryBase58Decode, []string{b}, false}, "binary.Base58Encode": {interopnames.SystemBinaryBase58Encode, []string{b}, false}, "binary.Base64Decode": {interopnames.SystemBinaryBase64Decode, []string{b}, false}, "binary.Base64Encode": {interopnames.SystemBinaryBase64Encode, []string{b}, false}, "binary.Deserialize": {interopnames.SystemBinaryDeserialize, []string{b}, false}, - "binary.Itoa": {interopnames.SystemBinaryItoa, []string{"123", "10"}, false}, "binary.Serialize": {interopnames.SystemBinarySerialize, []string{"10"}, false}, "contract.Call": {interopnames.SystemContractCall, []string{u160, `"m"`, "1", "3"}, false}, "contract.CreateMultisigAccount": {interopnames.SystemContractCreateMultisigAccount, []string{"1", pubs}, false}, diff --git a/pkg/compiler/vm_test.go b/pkg/compiler/vm_test.go index 1f414a42b..4752ef708 100644 --- a/pkg/compiler/vm_test.go +++ b/pkg/compiler/vm_test.go @@ -3,8 +3,6 @@ package compiler_test import ( "errors" "fmt" - "math/big" - "strconv" "strings" "testing" @@ -109,8 +107,7 @@ func newStoragePlugin() *storagePlugin { s.interops[interopnames.ToID([]byte(interopnames.SystemStoragePut))] = s.Put s.interops[interopnames.ToID([]byte(interopnames.SystemStorageGetContext))] = s.GetContext s.interops[interopnames.ToID([]byte(interopnames.SystemRuntimeNotify))] = s.Notify - s.interops[interopnames.ToID([]byte(interopnames.SystemBinaryAtoi))] = s.Atoi - s.interops[interopnames.ToID([]byte(interopnames.SystemBinaryItoa))] = s.Itoa + s.interops[interopnames.ToID([]byte(interopnames.SystemRuntimeGetTime))] = s.GetTime return s } @@ -126,24 +123,6 @@ func (s *storagePlugin) syscallHandler(v *vm.VM, id uint32) error { return errors.New("syscall not found") } -func (s *storagePlugin) Atoi(v *vm.VM) error { - str := v.Estack().Pop().String() - base := v.Estack().Pop().BigInt().Int64() - n, err := strconv.ParseInt(str, int(base), 64) - if err != nil { - return err - } - v.Estack().PushVal(big.NewInt(n)) - return nil -} - -func (s *storagePlugin) Itoa(v *vm.VM) error { - n := v.Estack().Pop().BigInt() - base := v.Estack().Pop().BigInt() - v.Estack().PushVal(n.Text(int(base.Int64()))) - return nil -} - func (s *storagePlugin) Notify(v *vm.VM) error { name := v.Estack().Pop().String() item := stackitem.NewArray(v.Estack().Pop().Array()) @@ -185,3 +164,10 @@ func (s *storagePlugin) GetContext(vm *vm.VM) error { vm.Estack().PushVal(10) return nil } + +func (s *storagePlugin) GetTime(vm *vm.VM) error { + // Pushing anything on the stack here will work. This is just to satisfy + // the compiler, thinking it has pushed the context ^^. + vm.Estack().PushVal(4) + return nil +} diff --git a/pkg/core/interop/binary/itoa.go b/pkg/core/interop/binary/itoa.go deleted file mode 100644 index b31918836..000000000 --- a/pkg/core/interop/binary/itoa.go +++ /dev/null @@ -1,91 +0,0 @@ -package binary - -import ( - "encoding/hex" - "errors" - "math/big" - "strings" - - "github.com/nspcc-dev/neo-go/pkg/core/interop" - "github.com/nspcc-dev/neo-go/pkg/encoding/bigint" -) - -var ( - // ErrInvalidBase is returned when base is invalid. - ErrInvalidBase = errors.New("invalid base") - // ErrInvalidFormat is returned when string is not a number. - ErrInvalidFormat = errors.New("invalid format") -) - -// Itoa converts number to string. -func Itoa(ic *interop.Context) error { - num := ic.VM.Estack().Pop().BigInt() - base := ic.VM.Estack().Pop().BigInt() - if !base.IsInt64() { - return ErrInvalidBase - } - var s string - switch b := base.Int64(); b { - case 10: - s = num.Text(10) - case 16: - if num.Sign() == 0 { - s = "0" - break - } - bs := bigint.ToBytes(num) - reverse(bs) - s = hex.EncodeToString(bs) - if pad := bs[0] & 0xF8; pad == 0 || pad == 0xF8 { - s = s[1:] - } - s = strings.ToUpper(s) - default: - return ErrInvalidBase - } - ic.VM.Estack().PushVal(s) - return nil -} - -// Atoi converts string to number. -func Atoi(ic *interop.Context) error { - num := ic.VM.Estack().Pop().String() - base := ic.VM.Estack().Pop().BigInt() - if !base.IsInt64() { - return ErrInvalidBase - } - var bi *big.Int - switch b := base.Int64(); b { - case 10: - var ok bool - bi, ok = new(big.Int).SetString(num, int(b)) - if !ok { - return ErrInvalidFormat - } - case 16: - changed := len(num)%2 != 0 - if changed { - num = "0" + num - } - bs, err := hex.DecodeString(num) - if err != nil { - return ErrInvalidFormat - } - if changed && bs[0]&0x8 != 0 { - bs[0] |= 0xF0 - } - reverse(bs) - bi = bigint.FromBytes(bs) - default: - return ErrInvalidBase - } - ic.VM.Estack().PushVal(bi) - return nil -} - -func reverse(b []byte) { - l := len(b) - for i := 0; i < l/2; i++ { - b[i], b[l-i-1] = b[l-i-1], b[i] - } -} diff --git a/pkg/core/interop/binary/itoa_test.go b/pkg/core/interop/binary/itoa_test.go deleted file mode 100644 index faf189076..000000000 --- a/pkg/core/interop/binary/itoa_test.go +++ /dev/null @@ -1,98 +0,0 @@ -package binary - -import ( - "errors" - "math" - "math/big" - "testing" - - "github.com/nspcc-dev/neo-go/pkg/core/interop" - "github.com/nspcc-dev/neo-go/pkg/vm" - "github.com/stretchr/testify/require" -) - -func TestItoa(t *testing.T) { - var testCases = []struct { - num *big.Int - base *big.Int - result string - }{ - {big.NewInt(0), big.NewInt(10), "0"}, - {big.NewInt(0), big.NewInt(16), "0"}, - {big.NewInt(1), big.NewInt(10), "1"}, - {big.NewInt(-1), big.NewInt(10), "-1"}, - {big.NewInt(1), big.NewInt(16), "1"}, - {big.NewInt(7), big.NewInt(16), "7"}, - {big.NewInt(8), big.NewInt(16), "08"}, - {big.NewInt(65535), big.NewInt(16), "0FFFF"}, - {big.NewInt(15), big.NewInt(16), "0F"}, - {big.NewInt(-1), big.NewInt(16), "F"}, - } - - for _, tc := range testCases { - ic := &interop.Context{VM: vm.New()} - ic.VM.Estack().PushVal(tc.base) - ic.VM.Estack().PushVal(tc.num) - require.NoError(t, Itoa(ic)) - require.Equal(t, tc.result, ic.VM.Estack().Pop().String()) - - ic = &interop.Context{VM: vm.New()} - ic.VM.Estack().PushVal(tc.base) - ic.VM.Estack().PushVal(tc.result) - - require.NoError(t, Atoi(ic)) - require.Equal(t, tc.num, ic.VM.Estack().Pop().BigInt()) - } - - t.Run("-1", func(t *testing.T) { - for _, s := range []string{"FF", "FFF", "FFFF"} { - ic := &interop.Context{VM: vm.New()} - ic.VM.Estack().PushVal(16) - ic.VM.Estack().PushVal(s) - - require.NoError(t, Atoi(ic)) - require.Equal(t, big.NewInt(-1), ic.VM.Estack().Pop().BigInt()) - } - }) -} - -func TestItoaError(t *testing.T) { - var testCases = []struct { - num *big.Int - base *big.Int - err error - }{ - {big.NewInt(1), big.NewInt(13), ErrInvalidBase}, - {big.NewInt(-1), new(big.Int).Add(big.NewInt(math.MaxInt64), big.NewInt(10)), ErrInvalidBase}, - } - - for _, tc := range testCases { - ic := &interop.Context{VM: vm.New()} - ic.VM.Estack().PushVal(tc.base) - ic.VM.Estack().PushVal(tc.num) - err := Itoa(ic) - require.True(t, errors.Is(err, tc.err), "got: %v", err) - } -} - -func TestAtoiError(t *testing.T) { - var testCases = []struct { - num string - base *big.Int - err error - }{ - {"1", big.NewInt(13), ErrInvalidBase}, - {"1", new(big.Int).Add(big.NewInt(math.MaxInt64), big.NewInt(16)), ErrInvalidBase}, - {"1_000", big.NewInt(10), ErrInvalidFormat}, - {"FE", big.NewInt(10), ErrInvalidFormat}, - {"XD", big.NewInt(16), ErrInvalidFormat}, - } - - for _, tc := range testCases { - ic := &interop.Context{VM: vm.New()} - ic.VM.Estack().PushVal(tc.base) - ic.VM.Estack().PushVal(tc.num) - err := Atoi(ic) - require.True(t, errors.Is(err, tc.err), "got: %v", err) - } -} diff --git a/pkg/core/interop/interopnames/names.go b/pkg/core/interop/interopnames/names.go index 9b58c2796..47dd802f9 100644 --- a/pkg/core/interop/interopnames/names.go +++ b/pkg/core/interop/interopnames/names.go @@ -2,13 +2,11 @@ package interopnames // Names of all used interops. const ( - SystemBinaryAtoi = "System.Binary.Atoi" SystemBinaryBase58Decode = "System.Binary.Base58Decode" SystemBinaryBase58Encode = "System.Binary.Base58Encode" SystemBinaryBase64Decode = "System.Binary.Base64Decode" SystemBinaryBase64Encode = "System.Binary.Base64Encode" SystemBinaryDeserialize = "System.Binary.Deserialize" - SystemBinaryItoa = "System.Binary.Itoa" SystemBinarySerialize = "System.Binary.Serialize" SystemCallbackCreate = "System.Callback.Create" SystemCallbackCreateFromMethod = "System.Callback.CreateFromMethod" @@ -54,13 +52,11 @@ const ( ) var names = []string{ - SystemBinaryAtoi, SystemBinaryBase58Decode, SystemBinaryBase58Encode, SystemBinaryBase64Decode, SystemBinaryBase64Encode, SystemBinaryDeserialize, - SystemBinaryItoa, SystemBinarySerialize, SystemCallbackCreate, SystemCallbackCreateFromMethod, diff --git a/pkg/core/interops.go b/pkg/core/interops.go index db4844216..6044af302 100644 --- a/pkg/core/interops.go +++ b/pkg/core/interops.go @@ -32,13 +32,11 @@ func SpawnVM(ic *interop.Context) *vm.VM { // All lists are sorted, keep 'em this way, please. var systemInterops = []interop.Function{ - {Name: interopnames.SystemBinaryAtoi, Func: binary.Atoi, Price: 1 << 12, ParamCount: 2}, {Name: interopnames.SystemBinaryBase58Decode, Func: binary.DecodeBase58, Price: 1 << 12, ParamCount: 1}, {Name: interopnames.SystemBinaryBase58Encode, Func: binary.EncodeBase58, Price: 1 << 12, ParamCount: 1}, {Name: interopnames.SystemBinaryBase64Decode, Func: binary.DecodeBase64, Price: 1 << 12, ParamCount: 1}, {Name: interopnames.SystemBinaryBase64Encode, Func: binary.EncodeBase64, Price: 1 << 12, ParamCount: 1}, {Name: interopnames.SystemBinaryDeserialize, Func: binary.Deserialize, Price: 1 << 14, ParamCount: 1}, - {Name: interopnames.SystemBinaryItoa, Func: binary.Itoa, Price: 1 << 12, ParamCount: 2}, {Name: interopnames.SystemBinarySerialize, Func: binary.Serialize, Price: 1 << 12, ParamCount: 1}, {Name: interopnames.SystemContractCall, Func: contract.Call, Price: 1 << 15, RequiredFlags: callflag.ReadStates | callflag.AllowCall, ParamCount: 4}, diff --git a/pkg/core/native/std_test.go b/pkg/core/native/std_test.go new file mode 100644 index 000000000..ee1baaca8 --- /dev/null +++ b/pkg/core/native/std_test.go @@ -0,0 +1,96 @@ +package native + +import ( + "math" + "math/big" + "testing" + + "github.com/nspcc-dev/neo-go/pkg/core/interop" + "github.com/nspcc-dev/neo-go/pkg/vm" + "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" + "github.com/stretchr/testify/require" +) + +func TestStdLibItoaAtoi(t *testing.T) { + s := newStd() + ic := &interop.Context{VM: vm.New()} + var actual stackitem.Item + + t.Run("itoa-atoi", func(t *testing.T) { + var testCases = []struct { + num *big.Int + base *big.Int + result string + }{ + {big.NewInt(0), big.NewInt(10), "0"}, + {big.NewInt(0), big.NewInt(16), "0"}, + {big.NewInt(1), big.NewInt(10), "1"}, + {big.NewInt(-1), big.NewInt(10), "-1"}, + {big.NewInt(1), big.NewInt(16), "1"}, + {big.NewInt(7), big.NewInt(16), "7"}, + {big.NewInt(8), big.NewInt(16), "08"}, + {big.NewInt(65535), big.NewInt(16), "0FFFF"}, + {big.NewInt(15), big.NewInt(16), "0F"}, + {big.NewInt(-1), big.NewInt(16), "F"}, + } + + for _, tc := range testCases { + require.NotPanics(t, func() { + actual = s.itoa(ic, []stackitem.Item{stackitem.Make(tc.num), stackitem.Make(tc.base)}) + }) + require.Equal(t, stackitem.Make(tc.result), actual) + + require.NotPanics(t, func() { + actual = s.atoi(ic, []stackitem.Item{stackitem.Make(tc.result), stackitem.Make(tc.base)}) + }) + require.Equal(t, stackitem.Make(tc.num), actual) + } + + t.Run("-1", func(t *testing.T) { + for _, str := range []string{"FF", "FFF", "FFFF"} { + require.NotPanics(t, func() { + actual = s.atoi(ic, []stackitem.Item{stackitem.Make(str), stackitem.Make(16)}) + }) + + require.Equal(t, stackitem.Make(-1), actual) + } + }) + }) + + t.Run("itoa error", func(t *testing.T) { + var testCases = []struct { + num *big.Int + base *big.Int + err error + }{ + {big.NewInt(1), big.NewInt(13), ErrInvalidBase}, + {big.NewInt(-1), new(big.Int).Add(big.NewInt(math.MaxInt64), big.NewInt(10)), ErrInvalidBase}, + } + + for _, tc := range testCases { + require.PanicsWithError(t, tc.err.Error(), func() { + _ = s.itoa(ic, []stackitem.Item{stackitem.Make(tc.num), stackitem.Make(tc.base)}) + }) + } + }) + + t.Run("atoi error", func(t *testing.T) { + var testCases = []struct { + num string + base *big.Int + err error + }{ + {"1", big.NewInt(13), ErrInvalidBase}, + {"1", new(big.Int).Add(big.NewInt(math.MaxInt64), big.NewInt(16)), ErrInvalidBase}, + {"1_000", big.NewInt(10), ErrInvalidFormat}, + {"FE", big.NewInt(10), ErrInvalidFormat}, + {"XD", big.NewInt(16), ErrInvalidFormat}, + } + + for _, tc := range testCases { + require.PanicsWithError(t, tc.err.Error(), func() { + _ = s.atoi(ic, []stackitem.Item{stackitem.Make(tc.num), stackitem.Make(tc.base)}) + }) + } + }) +} diff --git a/pkg/interop/binary/binary.go b/pkg/interop/binary/binary.go index 3cf244e22..9d25a589e 100644 --- a/pkg/interop/binary/binary.go +++ b/pkg/interop/binary/binary.go @@ -44,15 +44,3 @@ func Base58Encode(b []byte) string { func Base58Decode(b []byte) []byte { return neogointernal.Syscall1("System.Binary.Base58Decode", b).([]byte) } - -// Itoa converts num in a given base to string. Base should be either 10 or 16. -// It uses `System.Binary.Itoa` syscall. -func Itoa(num int, base int) string { - return neogointernal.Syscall2("System.Binary.Itoa", num, base).(string) -} - -// Atoi converts string to a number in a given base. Base should be either 10 or 16. -// It uses `System.Binary.Atoi` syscall. -func Atoi(s string, base int) int { - return neogointernal.Syscall2("System.Binary.Atoi", s, base).(int) -}