From 6014dd720fbfddf5a20883e25c09e2a116db4cd3 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 6 Jul 2022 17:41:39 +0300 Subject: [PATCH 1/5] compiler: don't push X onto the stack for inlined method calls Regular methods need this, because it'll be packed into parameters, but inlined ones should deal with it in inlining code itself because method receiver will be some local (aliased) variable anyway. --- pkg/compiler/codegen.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go index f1d86748a..693bd7efd 100644 --- a/pkg/compiler/codegen.go +++ b/pkg/compiler/codegen.go @@ -914,15 +914,7 @@ func (c *codegen) Visit(node ast.Node) ast.Visitor { return nil } case *ast.SelectorExpr: - // If this is a method call we need to walk the AST to load the struct locally. - // Otherwise, this is a function call from an imported package and we can call it - // directly. name, isMethod := c.getFuncNameFromSelector(fun) - if isMethod { - ast.Walk(c, fun.X) - // Don't forget to add 1 extra argument when its a method. - numArgs++ - } f, ok = c.funcs[name] if ok { @@ -938,6 +930,14 @@ func (c *codegen) Visit(node ast.Node) ast.Visitor { c.emitExplicitConvert(c.typeOf(n.Args[0]), typ) return nil } + if isMethod { + // If this is a method call we need to walk the AST to load the struct locally. + // Otherwise, this is a function call from an imported package and we can call it + // directly. + ast.Walk(c, fun.X) + // Don't forget to add 1 extra argument when it's a method. + numArgs++ + } case *ast.ArrayType: // For now we will assume that there are only byte slice conversions. // E.g. []byte("foobar") or []byte(scriptHash). From b57dd2cad6668219ca14323f7618e4ce5562d420 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 6 Jul 2022 17:56:53 +0300 Subject: [PATCH 2/5] compiler: properly inline methods, use receiver Notice that this doesn't differentiate between (*T) and (T) receivers always treating them as is. But we have the same problem with arguments now and the number of inlined calls is limited, usually we want this behavior. --- pkg/compiler/inline.go | 9 +++++++++ pkg/compiler/inline_test.go | 19 +++++++++++++++++++ pkg/compiler/testdata/inline/inline.go | 10 ++++++++++ 3 files changed, 38 insertions(+) diff --git a/pkg/compiler/inline.go b/pkg/compiler/inline.go index 74e33c190..57b0ed707 100644 --- a/pkg/compiler/inline.go +++ b/pkg/compiler/inline.go @@ -55,6 +55,15 @@ func (c *codegen) inlineCall(f *funcScope, n *ast.CallExpr) { copy(newScope, c.scope.vars.locals) defer c.scope.vars.dropScope() + if f.decl.Recv != nil { + c.scope.vars.locals = newScope + name := f.decl.Recv.List[0].Names[0].Name + c.scope.vars.addAlias(name, -1, unspecifiedVarIndex, &varContext{ + importMap: c.importMap, + expr: f.selector, + scope: oldScope, + }) + } hasVarArgs := !n.Ellipsis.IsValid() needPack := sig.Variadic() && hasVarArgs for i := range n.Args { diff --git a/pkg/compiler/inline_test.go b/pkg/compiler/inline_test.go index 3bbf38415..d8734ebd9 100644 --- a/pkg/compiler/inline_test.go +++ b/pkg/compiler/inline_test.go @@ -336,3 +336,22 @@ func TestPackageVarsInInlinedCalls(t *testing.T) { }` eval(t, src, big.NewInt(13)) } + +func TestInlinedMethod(t *testing.T) { + src := `package foo + import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/inline" + func Main() int { + // It's important for this variable to not be named 't'. + var z inline.T + i := z.Inc(42) + if i != 0 || z.N != 42 { + return 0 + } + i = z.Inc(100500) + if i != 42 { + return 0 + } + return z.N + }` + eval(t, src, big.NewInt(100542)) +} diff --git a/pkg/compiler/testdata/inline/inline.go b/pkg/compiler/testdata/inline/inline.go index 62aaf2c79..dfc7aae4e 100644 --- a/pkg/compiler/testdata/inline/inline.go +++ b/pkg/compiler/testdata/inline/inline.go @@ -46,3 +46,13 @@ func SumVar(a, b int) int { func Concat(n int) int { return n*100 + b.A*10 + A } + +type T struct { + N int +} + +func (t *T) Inc(i int) int { + n := t.N + t.N += i + return n +} From ec3d1fae5985e04c3c8e8a408e9c08e244fbd991 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 6 Jul 2022 17:52:57 +0300 Subject: [PATCH 3/5] compiler: allow to find appropriate methods via selectors c.funcs contains function names using base types, while methods can be defined on pointers and the value returned from c.getFuncNameFromSelector will have an asterisk. We can't have the same name used for (*T) and (T) methods, so just stripping the asterisk allows to get the right one. --- pkg/compiler/codegen.go | 6 +++++- pkg/compiler/inline_test.go | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go index 693bd7efd..937e52bf4 100644 --- a/pkg/compiler/codegen.go +++ b/pkg/compiler/codegen.go @@ -2067,7 +2067,11 @@ func (c *codegen) getFuncNameFromSelector(e *ast.SelectorExpr) (string, bool) { ident := e.X.(*ast.Ident) if c.typeInfo.Selections[e] != nil { typ := c.typeInfo.Types[ident].Type.String() - return c.getIdentName(typ, e.Sel.Name), true + name := c.getIdentName(typ, e.Sel.Name) + if name[0] == '*' { + name = name[1:] + } + return name, true } return c.getIdentName(ident.Name, e.Sel.Name), false } diff --git a/pkg/compiler/inline_test.go b/pkg/compiler/inline_test.go index d8734ebd9..b68a816d3 100644 --- a/pkg/compiler/inline_test.go +++ b/pkg/compiler/inline_test.go @@ -355,3 +355,22 @@ func TestInlinedMethod(t *testing.T) { }` eval(t, src, big.NewInt(100542)) } + +func TestInlinedMethodWithPointer(t *testing.T) { + src := `package foo + import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/inline" + func Main() int { + // It's important for this variable to not be named 't'. + var z = &inline.T{} + i := z.Inc(42) + if i != 0 || z.N != 42 { + return 0 + } + i = z.Inc(100500) + if i != 42 { + return 0 + } + return z.N + }` + eval(t, src, big.NewInt(100542)) +} From 6deb77a77a102a8f313a1b9ec375d4d7c590bbb2 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 6 Jul 2022 18:04:25 +0300 Subject: [PATCH 4/5] compiler: make interface{}() conversions possible --- docs/compiler.md | 3 +++ pkg/compiler/codegen.go | 5 +++++ pkg/compiler/convert_test.go | 10 ++++++++++ 3 files changed, 18 insertions(+) diff --git a/docs/compiler.md b/docs/compiler.md index bd9375cdd..7d5810177 100644 --- a/docs/compiler.md +++ b/docs/compiler.md @@ -25,6 +25,9 @@ a dialect of Go rather than a complete port of the language: in variables and returning the result. * lambdas are supported, but closures are not. * maps are supported, but valid map keys are booleans, integers and strings with length <= 64 + * converting value to interface type doesn't change the underlying type, + original value will always be used, therefore it never panics and always "succeeds"; + it's up to the programmer whether it's a correct use of a value ## VM API (interop layer) Compiler translates interop function calls into NEO VM syscalls or (for custom diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go index 937e52bf4..5375feb64 100644 --- a/pkg/compiler/codegen.go +++ b/pkg/compiler/codegen.go @@ -944,6 +944,11 @@ func (c *codegen) Visit(node ast.Node) ast.Visitor { ast.Walk(c, n.Args[0]) c.emitConvert(stackitem.BufferT) return nil + case *ast.InterfaceType: + // It's a type conversion into some interface. Programmer is responsible + // for the conversion to be appropriate, just load the arg. + ast.Walk(c, n.Args[0]) + return nil case *ast.FuncLit: isLiteral = true } diff --git a/pkg/compiler/convert_test.go b/pkg/compiler/convert_test.go index fd3552afd..2793bfef8 100644 --- a/pkg/compiler/convert_test.go +++ b/pkg/compiler/convert_test.go @@ -147,3 +147,13 @@ func TestTypeConversionString(t *testing.T) { }` eval(t, src, []byte("lamao")) } + +func TestInterfaceTypeConversion(t *testing.T) { + src := `package foo + func Main() int { + a := 1 + b := interface{}(a).(int) + return b + }` + eval(t, src, big.NewInt(1)) +} From 251c9bd89b0938daa0d00808447da1e7d595a77b Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Wed, 6 Jul 2022 11:52:06 +0300 Subject: [PATCH 5/5] block: push PrevStateRoot data into stack item, fix #2551 And add compiler/interop support for this. --- pkg/compiler/analysis.go | 3 ++- pkg/core/block/block.go | 9 +++++++-- pkg/interop/native/ledger/block.go | 24 ++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/pkg/compiler/analysis.go b/pkg/compiler/analysis.go index 9e646e638..e0eaee510 100644 --- a/pkg/compiler/analysis.go +++ b/pkg/compiler/analysis.go @@ -371,7 +371,8 @@ func canConvert(s string) bool { s = s[len(interopPrefix):] return s != "/iterator.Iterator" && s != "/storage.Context" && s != "/native/ledger.Block" && s != "/native/ledger.Transaction" && - s != "/native/management.Contract" && s != "/native/neo.AccountState" + s != "/native/management.Contract" && s != "/native/neo.AccountState" && + s != "/native/ledger.BlockSR" } return true } diff --git a/pkg/core/block/block.go b/pkg/core/block/block.go index bdf3e0e57..421b01707 100644 --- a/pkg/core/block/block.go +++ b/pkg/core/block/block.go @@ -216,7 +216,7 @@ func (b *Block) GetExpectedBlockSizeWithoutTransactions(txCount int) int { // ToStackItem converts Block to stackitem.Item. func (b *Block) ToStackItem() stackitem.Item { - return stackitem.NewArray([]stackitem.Item{ + items := []stackitem.Item{ stackitem.NewByteArray(b.Hash().BytesBE()), stackitem.NewBigInteger(big.NewInt(int64(b.Version))), stackitem.NewByteArray(b.PrevHash.BytesBE()), @@ -226,5 +226,10 @@ func (b *Block) ToStackItem() stackitem.Item { stackitem.NewBigInteger(big.NewInt(int64(b.Index))), stackitem.NewByteArray(b.NextConsensus.BytesBE()), stackitem.NewBigInteger(big.NewInt(int64(len(b.Transactions)))), - }) + } + if b.StateRootEnabled { + items = append(items, stackitem.NewByteArray(b.PrevStateRoot.BytesBE())) + } + + return stackitem.NewArray(items) } diff --git a/pkg/interop/native/ledger/block.go b/pkg/interop/native/ledger/block.go index 01fbe1087..84e1a6407 100644 --- a/pkg/interop/native/ledger/block.go +++ b/pkg/interop/native/ledger/block.go @@ -29,3 +29,27 @@ type Block struct { // TransactionsLength represents the length of block's transactions array. TransactionsLength int } + +// BlockSR is a stateroot-enabled Neo block. It's returned from the Ledger contract's +// GetBlock method when StateRootInHeader NeoGo extension is used. Use it only when +// you have it enabled when you need to access PrevStateRoot field, Block is sufficient +// otherwise. To get this data type ToBlockSR method of Block should be used. All of +// the fields are same as in Block except PrevStateRoot. +type BlockSR struct { + Hash interop.Hash256 + Version int + PrevHash interop.Hash256 + MerkleRoot interop.Hash256 + Timestamp int + Nonce int + Index int + NextConsensus interop.Hash160 + TransactionsLength int + // PrevStateRoot is a hash of the previous block's state root. + PrevStateRoot interop.Hash256 +} + +// ToBlockSR converts Block into BlockSR for chains with StateRootInHeader option. +func (b *Block) ToBlockSR() *BlockSR { + return interface{}(b).(*BlockSR) +}