From 7e13140b04c2c9d388bf81a801574ee549caab4f Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 16 Sep 2022 19:18:47 +0300 Subject: [PATCH] interop: add Hash160 encoder\decoder helper Close #2690. --- pkg/compiler/analysis.go | 42 +++++++++++-- pkg/compiler/codegen.go | 25 ++++---- pkg/compiler/interop_test.go | 98 ++++++++++++++++++++++++++++++ pkg/compiler/vm_test.go | 8 +++ pkg/interop/lib/address/address.go | 36 +++++++++++ pkg/interop/util/util.go | 2 + 6 files changed, 196 insertions(+), 15 deletions(-) create mode 100644 pkg/interop/lib/address/address.go diff --git a/pkg/compiler/analysis.go b/pkg/compiler/analysis.go index 45fca37ab..f725bc771 100644 --- a/pkg/compiler/analysis.go +++ b/pkg/compiler/analysis.go @@ -28,6 +28,26 @@ var ( customBuiltins = []string{ "FromAddress", } + // Custom builtin utility functions that contain some meaningful code inside and + // require code generation using standard rules, but sometimes (depending on + // the expression usage condition) may be optimized at compile time. + potentialCustomBuiltins = map[string]func(f ast.Expr) bool{ + "ToHash160": func(f ast.Expr) bool { + c, ok := f.(*ast.CallExpr) + if !ok { + return false + } + if len(c.Args) != 1 { + return false + } + switch c.Args[0].(type) { + case *ast.BasicLit: + return true + default: + return false + } + }, + } ) // newGlobal creates a new global variable. @@ -634,6 +654,18 @@ func isCustomBuiltin(f *funcScope) bool { return false } +func isPotentialCustomBuiltin(f *funcScope, expr ast.Expr) bool { + if !isInteropPath(f.pkg.Path()) { + return false + } + for name, isBuiltin := range potentialCustomBuiltins { + if f.name == name && isBuiltin(expr) { + return true + } + } + return false +} + func isSyscall(fun *funcScope) bool { if fun.selector == nil || fun.pkg == nil || !isInteropPath(fun.pkg.Path()) { return false @@ -664,9 +696,10 @@ func canConvert(s string) bool { } // canInline returns true if the function is to be inlined. -// Currently, there is a static list of functions which are inlined, -// this may change in future. -func canInline(s string, name string) bool { +// The list of functions that can be inlined is not static, it depends on the function usages. +// isBuiltin denotes whether code generation for dynamic builtin function will be performed +// manually. +func canInline(s string, name string, isBuiltin bool) bool { if strings.HasPrefix(s, "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/inline") { return true } @@ -674,5 +707,6 @@ func canInline(s string, name string) bool { return false } return !strings.HasPrefix(s[len(interopPrefix):], "/neogointernal") && - !(strings.HasPrefix(s[len(interopPrefix):], "/util") && name == "FromAddress") + !(strings.HasPrefix(s[len(interopPrefix):], "/util") && name == "FromAddress") && + !(strings.HasPrefix(s[len(interopPrefix):], "/lib/address") && name == "ToHash160" && isBuiltin) } diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go index d74a69700..1d56a2e2c 100644 --- a/pkg/compiler/codegen.go +++ b/pkg/compiler/codegen.go @@ -456,6 +456,8 @@ func (c *codegen) convertFuncDecl(file ast.Node, decl *ast.FuncDecl, pkg *types. f, ok = c.funcs[c.getFuncNameFromDecl("", decl)] if ok { // If this function is a syscall or builtin we will not convert it to bytecode. + // If it's a potential custom builtin then it needs more specific usages research, + // thus let's emit the code for it. if isSyscall(f) || isCustomBuiltin(f) { return f } @@ -947,7 +949,7 @@ func (c *codegen) Visit(node ast.Node) ast.Visitor { if fun.Obj != nil && fun.Obj.Kind == ast.Var { isFunc = true } - if ok && canInline(f.pkg.Path(), f.decl.Name.Name) { + if ok && canInline(f.pkg.Path(), f.decl.Name.Name, false) { c.inlineCall(f, n) return nil } @@ -957,8 +959,8 @@ func (c *codegen) Visit(node ast.Node) ast.Visitor { f, ok = c.funcs[name] if ok { f.selector = fun.X - isBuiltin = isCustomBuiltin(f) - if canInline(f.pkg.Path(), f.decl.Name.Name) { + isBuiltin = isCustomBuiltin(f) || isPotentialCustomBuiltin(f, n) + if canInline(f.pkg.Path(), f.decl.Name.Name, isBuiltin) { c.inlineCall(f, n) return nil } @@ -993,7 +995,7 @@ func (c *codegen) Visit(node ast.Node) ast.Visitor { c.saveSequencePoint(n) - args := transformArgs(f, n.Fun, n.Args) + args := transformArgs(f, n.Fun, isBuiltin, n.Args) // Handle the arguments for _, arg := range args { @@ -1870,8 +1872,8 @@ func (c *codegen) convertBuiltin(expr *ast.CallExpr) { c.emitStoreByIndex(varGlobal, c.exceptionIndex) case "delete": emit.Opcodes(c.prog.BinWriter, opcode.REMOVE) - case "FromAddress": - // We can be sure that this is a ast.BasicLit just containing a simple + case "FromAddress", "ToHash160": + // We can be sure that this is an ast.BasicLit just containing a simple // address string. Note that the string returned from calling Value will // contain double quotes that need to be stripped. addressStr := expr.Args[0].(*ast.BasicLit).Value @@ -1890,14 +1892,15 @@ func (c *codegen) convertBuiltin(expr *ast.CallExpr) { // transformArgs returns a list of function arguments // which should be put on stack. // There are special cases for builtins: -// 1. With FromAddress, parameter conversion is happening at compile-time -// so there is no need to push parameters on stack and perform an actual call +// 1. With FromAddress and with ToHash160 in case if it behaves like builtin, +// parameter conversion is happening at compile-time so there is no need to +// push parameters on stack and perform an actual call // 2. With panic, the generated code depends on the fact if an argument was nil or a string; // so, it should be handled accordingly. -func transformArgs(fs *funcScope, fun ast.Expr, args []ast.Expr) []ast.Expr { +func transformArgs(fs *funcScope, fun ast.Expr, isBuiltin bool, args []ast.Expr) []ast.Expr { switch f := fun.(type) { case *ast.SelectorExpr: - if f.Sel.Name == "FromAddress" { + if f.Sel.Name == "FromAddress" || (isBuiltin && f.Sel.Name == "ToHash160") { return args[1:] } if fs != nil && isSyscall(fs) { @@ -2178,7 +2181,7 @@ func (c *codegen) compile(info *buildInfo, pkg *packages.Package) error { } name := c.getFuncNameFromDecl(pkgPath, n) if !isInitFunc(n) && !isDeployFunc(n) && funUsage.funcUsed(name) && - (!isInteropPath(pkg.Path()) && !canInline(pkg.Path(), n.Name.Name)) { + (!isInteropPath(pkg.Path()) && !canInline(pkg.Path(), n.Name.Name, false)) { c.convertFuncDecl(f, n, pkg) } } diff --git a/pkg/compiler/interop_test.go b/pkg/compiler/interop_test.go index d737a3058..74904dcb1 100644 --- a/pkg/compiler/interop_test.go +++ b/pkg/compiler/interop_test.go @@ -17,13 +17,17 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/storage" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/encoding/address" + "github.com/nspcc-dev/neo-go/pkg/encoding/base58" cinterop "github.com/nspcc-dev/neo-go/pkg/interop" + "github.com/nspcc-dev/neo-go/pkg/neotest" + "github.com/nspcc-dev/neo-go/pkg/neotest/chain" "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" + "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" ) @@ -116,6 +120,100 @@ func TestFromAddress(t *testing.T) { }) } +func TestAddressToHash160BuiltinConversion(t *testing.T) { + a := "NQRLhCpAru9BjGsMwk67vdMwmzKMRgsnnN" + h, err := address.StringToUint160(a) + require.NoError(t, err) + t.Run("builtin conversion", func(t *testing.T) { + src := `package foo + import ( + "github.com/nspcc-dev/neo-go/pkg/interop" + "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" + ) + var addr = address.ToHash160("` + a + `") + func Main() interop.Hash160 { + return addr + }` + prog := eval(t, src, h.BytesBE()) + // Address BE bytes expected to be present at program, which indicates that address conversion + // was performed at compile-time. + require.True(t, strings.Contains(string(prog), string(h.BytesBE()))) + // On the contrary, there should be no address string. + require.False(t, strings.Contains(string(prog), a)) + }) + t.Run("generate code", func(t *testing.T) { + src := `package foo + import ( + "github.com/nspcc-dev/neo-go/pkg/interop" + "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" + ) + var addr = "` + a + `" + func Main() interop.Hash160 { + return address.ToHash160(addr) + }` + // Error on CALLT (std.Base58CheckDecode - method of StdLib native contract) is expected, which means + // that address.ToHash160 code was honestly generated by the compiler without any optimisations. + prog := evalWithError(t, src, "(CALLT): runtime error: invalid memory address or nil pointer dereference") + // Address BE bytes expected not to be present at program, which indicates that address conversion + // was not performed at compile-time. + require.False(t, strings.Contains(string(prog), string(h.BytesBE()))) + // On the contrary, there should be an address string. + require.True(t, strings.Contains(string(prog), a)) + }) +} + +func TestInvokeAddressToFromHash160(t *testing.T) { + a := "NQRLhCpAru9BjGsMwk67vdMwmzKMRgsnnN" + h, err := address.StringToUint160(a) + require.NoError(t, err) + + bc, acc := chain.NewSingle(t) + e := neotest.NewExecutor(t, bc, acc, acc) + src := `package foo + import ( + "github.com/nspcc-dev/neo-go/pkg/interop" + "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" + ) + const addr = "` + a + `" + func ToHash160(a string) interop.Hash160 { + return address.ToHash160(a) + } + func ToHash160AtCompileTime() interop.Hash160 { + return address.ToHash160(addr) + } + func FromHash160(hash interop.Hash160) string { + return address.FromHash160(hash) + }` + ctr := neotest.CompileSource(t, e.CommitteeHash, strings.NewReader(src), &compiler.Options{Name: "Helper"}) + e.DeployContract(t, ctr, nil) + c := e.CommitteeInvoker(ctr.Hash) + + t.Run("ToHash160", func(t *testing.T) { + t.Run("invalid address length", func(t *testing.T) { + c.InvokeFail(t, "invalid address length", "toHash160", base58.CheckEncode(make([]byte, util.Uint160Size+1+1))) + }) + t.Run("invalid prefix", func(t *testing.T) { + c.InvokeFail(t, "invalid address prefix", "toHash160", base58.CheckEncode(append([]byte{address.NEO2Prefix}, h.BytesBE()...))) + }) + t.Run("good", func(t *testing.T) { + c.Invoke(t, stackitem.NewBuffer(h.BytesBE()), "toHash160", a) + }) + }) + t.Run("ToHash160Constant", func(t *testing.T) { + t.Run("good", func(t *testing.T) { + c.Invoke(t, stackitem.NewBuffer(h.BytesBE()), "toHash160AtCompileTime") + }) + }) + t.Run("FromHash160", func(t *testing.T) { + t.Run("good", func(t *testing.T) { + c.Invoke(t, stackitem.NewByteArray([]byte(a)), "fromHash160", h.BytesBE()) + }) + t.Run("invalid length", func(t *testing.T) { + c.InvokeFail(t, "invalid Hash160 length", "fromHash160", h.BytesBE()[:15]) + }) + }) +} + func TestAbort(t *testing.T) { src := `package foo import "github.com/nspcc-dev/neo-go/pkg/interop/util" diff --git a/pkg/compiler/vm_test.go b/pkg/compiler/vm_test.go index 867f3aa5a..a6826a028 100644 --- a/pkg/compiler/vm_test.go +++ b/pkg/compiler/vm_test.go @@ -56,6 +56,14 @@ func eval(t *testing.T, src string, result interface{}, expectedOps ...interface return script } +func evalWithError(t *testing.T, src string, e string) []byte { + vm, _, prog := vmAndCompileInterop(t, src) + err := vm.Run() + require.Error(t, err) + require.True(t, strings.Contains(err.Error(), e), err) + return prog +} + func runAndCheck(t *testing.T, v *vm.VM, result interface{}) { err := v.Run() require.NoError(t, err) diff --git a/pkg/interop/lib/address/address.go b/pkg/interop/lib/address/address.go new file mode 100644 index 000000000..d501220b5 --- /dev/null +++ b/pkg/interop/lib/address/address.go @@ -0,0 +1,36 @@ +package address + +import ( + "github.com/nspcc-dev/neo-go/pkg/interop" + "github.com/nspcc-dev/neo-go/pkg/interop/native/std" + "github.com/nspcc-dev/neo-go/pkg/interop/runtime" +) + +// ToHash160 is a utility function that converts a Neo address to its hash +// (160 bit BE value in a 20 byte slice). When parameter is known at compile time +// (it's a constant string) the output is calculated by the compiler and this +// function is optimized out completely. Otherwise, standard library and system +// calls are used to perform the conversion and checks (panic will happen on +// invalid input). +func ToHash160(address string) interop.Hash160 { + b := std.Base58CheckDecode([]byte(address)) + if len(b) != interop.Hash160Len+1 { + panic("invalid address length") + } + if int(b[0]) != runtime.GetAddressVersion() { + panic("invalid address prefix") + } + return b[1:21] +} + +// FromHash160 is a utility function that converts given Hash160 to +// Base58-encoded Neo address. +func FromHash160(hash interop.Hash160) string { + if len(hash) != interop.Hash160Len { + panic("invalid Hash160 length") + } + var res = make([]byte, interop.Hash160Len+1) + res[0] = byte(runtime.GetAddressVersion()) + copy(res[1:], hash) // @fixme #2696 + return std.Base58CheckEncode(res) +} diff --git a/pkg/interop/util/util.go b/pkg/interop/util/util.go index 32f0abead..038b17aa9 100644 --- a/pkg/interop/util/util.go +++ b/pkg/interop/util/util.go @@ -18,6 +18,8 @@ func Abort() { // (160 bit BE value in a 20 byte slice). It can only be used for strings known // at compilation time, because the conversion is actually being done by the // compiler. +// +// Deprecated: use address.ToHash160 instead. func FromAddress(address string) interop.Hash160 { return nil }