From 090bee86248e22fcf34d0400c7c1968946df3cf2 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Sun, 13 Dec 2020 21:36:06 +0300 Subject: [PATCH] native: change OnPersist/PostPersist handling Every contract now has these and they're always invoked. See neo-project/neo#1913 and neo-project/neo#2119. --- pkg/core/interop/context.go | 2 + pkg/core/interop/interopnames/names.go | 4 ++ pkg/core/interops.go | 2 + pkg/core/native/contract.go | 52 +++----------------------- pkg/core/native/designate.go | 18 +++++---- pkg/core/native/interop.go | 29 ++++++++++++++ pkg/core/native/native_gas.go | 23 +++--------- pkg/core/native/native_neo.go | 10 ----- pkg/core/native/native_nep17.go | 31 ++------------- pkg/core/native/notary.go | 13 +++---- pkg/core/native/oracle.go | 14 +++---- pkg/core/native/policy.go | 12 +++--- pkg/core/native_contract_test.go | 28 +++++++++----- 13 files changed, 95 insertions(+), 143 deletions(-) diff --git a/pkg/core/interop/context.go b/pkg/core/interop/context.go index 0f06e96d9..f1d858901 100644 --- a/pkg/core/interop/context.go +++ b/pkg/core/interop/context.go @@ -89,6 +89,8 @@ type MethodAndPrice struct { type Contract interface { Initialize(*Context) error Metadata() *ContractMD + OnPersist(*Context) error + PostPersist(*Context) error } // ContractMD represents native contract instance. diff --git a/pkg/core/interop/interopnames/names.go b/pkg/core/interop/interopnames/names.go index c3021014d..44536d706 100644 --- a/pkg/core/interop/interopnames/names.go +++ b/pkg/core/interop/interopnames/names.go @@ -27,6 +27,8 @@ const ( SystemContractDestroy = "System.Contract.Destroy" SystemContractIsStandard = "System.Contract.IsStandard" SystemContractGetCallFlags = "System.Contract.GetCallFlags" + SystemContractNativeOnPersist = "System.Contract.NativeOnPersist" + SystemContractNativePostPersist = "System.Contract.NativePostPersist" SystemContractUpdate = "System.Contract.Update" SystemEnumeratorConcat = "System.Enumerator.Concat" SystemEnumeratorCreate = "System.Enumerator.Create" @@ -96,6 +98,8 @@ var names = []string{ SystemContractDestroy, SystemContractIsStandard, SystemContractGetCallFlags, + SystemContractNativeOnPersist, + SystemContractNativePostPersist, SystemContractUpdate, SystemEnumeratorConcat, SystemEnumeratorCreate, diff --git a/pkg/core/interops.go b/pkg/core/interops.go index 83ac95824..12f4439bf 100644 --- a/pkg/core/interops.go +++ b/pkg/core/interops.go @@ -67,6 +67,8 @@ var systemInterops = []interop.Function{ {Name: interopnames.SystemContractDestroy, Func: contractDestroy, Price: 1000000, RequiredFlags: smartcontract.WriteStates, DisallowCallback: true}, {Name: interopnames.SystemContractIsStandard, Func: contractIsStandard, Price: 30000, RequiredFlags: smartcontract.ReadStates, ParamCount: 1}, {Name: interopnames.SystemContractGetCallFlags, Func: contractGetCallFlags, Price: 30000, DisallowCallback: true}, + {Name: interopnames.SystemContractNativeOnPersist, Func: native.OnPersist, Price: 0, DisallowCallback: true}, + {Name: interopnames.SystemContractNativePostPersist, Func: native.PostPersist, Price: 0, DisallowCallback: true}, {Name: interopnames.SystemContractUpdate, Func: contractUpdate, Price: 0, RequiredFlags: smartcontract.WriteStates, ParamCount: 2, DisallowCallback: true}, {Name: interopnames.SystemEnumeratorConcat, Func: enumerator.Concat, Price: 400, ParamCount: 2, DisallowCallback: true}, diff --git a/pkg/core/native/contract.go b/pkg/core/native/contract.go index 29f7e58b7..0e925f450 100644 --- a/pkg/core/native/contract.go +++ b/pkg/core/native/contract.go @@ -1,15 +1,13 @@ package native import ( - "errors" "strings" "github.com/nspcc-dev/neo-go/pkg/core/interop" + "github.com/nspcc-dev/neo-go/pkg/core/interop/interopnames" "github.com/nspcc-dev/neo-go/pkg/io" - "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/emit" - "github.com/nspcc-dev/neo-go/pkg/vm/opcode" ) // reservedContractID represents the upper bound of the reserved IDs for native contracts. @@ -62,9 +60,9 @@ func NewContracts(p2pSigExtensionsEnabled bool) *Contracts { gas.NEO = neo cs.GAS = gas - cs.Contracts = append(cs.Contracts, gas) cs.NEO = neo cs.Contracts = append(cs.Contracts, neo) + cs.Contracts = append(cs.Contracts, gas) policy := newPolicy() cs.Policy = policy @@ -93,62 +91,24 @@ func NewContracts(p2pSigExtensionsEnabled bool) *Contracts { return cs } -// GetPersistScript returns VM script calling "onPersist" method of every native contract. +// GetPersistScript returns VM script calling "onPersist" syscall for native contracts. func (cs *Contracts) GetPersistScript() []byte { if cs.persistScript != nil { return cs.persistScript } w := io.NewBufBinWriter() - for i := range cs.Contracts { - md := cs.Contracts[i].Metadata() - // Not every contract is persisted: - // https://github.com/neo-project/neo/blob/master/src/neo/Ledger/Blockchain.cs#L90 - if md.ContractID == policyContractID || md.ContractID == oracleContractID || md.ContractID == designateContractID { - continue - } - emit.Int(w.BinWriter, 0) - emit.Opcodes(w.BinWriter, opcode.NEWARRAY) - emit.String(w.BinWriter, "onPersist") - emit.AppCall(w.BinWriter, md.Hash) - emit.Opcodes(w.BinWriter, opcode.DROP) - } + emit.Syscall(w.BinWriter, interopnames.SystemContractNativeOnPersist) cs.persistScript = w.Bytes() return cs.persistScript } -// GetPostPersistScript returns VM script calling "postPersist" method of some native contracts. +// GetPostPersistScript returns VM script calling "postPersist" syscall for native contracts. func (cs *Contracts) GetPostPersistScript() []byte { if cs.postPersistScript != nil { return cs.postPersistScript } w := io.NewBufBinWriter() - for i := range cs.Contracts { - md := cs.Contracts[i].Metadata() - // Not every contract is persisted: - // https://github.com/neo-project/neo/blob/master/src/neo/Ledger/Blockchain.cs#L103 - if md.ContractID == policyContractID || md.ContractID == gasContractID || md.ContractID == designateContractID || md.ContractID == notaryContractID { - continue - } - emit.Int(w.BinWriter, 0) - emit.Opcodes(w.BinWriter, opcode.NEWARRAY) - emit.String(w.BinWriter, "postPersist") - emit.AppCall(w.BinWriter, md.Hash) - emit.Opcodes(w.BinWriter, opcode.DROP) - } + emit.Syscall(w.BinWriter, interopnames.SystemContractNativePostPersist) cs.postPersistScript = w.Bytes() return cs.postPersistScript } - -func postPersistBase(ic *interop.Context) error { - if ic.Trigger != trigger.PostPersist { - return errors.New("postPersist must be trigered by system") - } - return nil -} - -func onPersistBase(ic *interop.Context) error { - if ic.Trigger != trigger.OnPersist { - return errors.New("onPersist must be trigered by system") - } - return nil -} diff --git a/pkg/core/native/designate.go b/pkg/core/native/designate.go index bc4f4f4f0..d57864266 100644 --- a/pkg/core/native/designate.go +++ b/pkg/core/native/designate.go @@ -87,14 +87,6 @@ func newDesignate(p2pSigExtensionsEnabled bool) *Designate { md = newMethodAndPrice(s.designateAsRole, 0, smartcontract.WriteStates) s.AddMethod(md, desc) - desc = newDescriptor("onPersist", smartcontract.VoidType) - md = newMethodAndPrice(getOnPersistWrapper(onPersistBase), 0, smartcontract.WriteStates) - s.AddMethod(md, desc) - - desc = newDescriptor("postPersist", smartcontract.VoidType) - md = newMethodAndPrice(getOnPersistWrapper(postPersistBase), 0, smartcontract.WriteStates) - s.AddMethod(md, desc) - return s } @@ -103,6 +95,16 @@ func (s *Designate) Initialize(ic *interop.Context) error { return nil } +// OnPersist implements Contract interface. +func (s *Designate) OnPersist(ic *interop.Context) error { + return nil +} + +// PostPersist implements Contract interface. +func (s *Designate) PostPersist(ic *interop.Context) error { + return nil +} + // OnPersistEnd updates cached values if they've been changed. func (s *Designate) OnPersistEnd(d dao.DAO) error { if !s.rolesChanged() { diff --git a/pkg/core/native/interop.go b/pkg/core/native/interop.go index 4fbcf2ffa..ecd529fc4 100644 --- a/pkg/core/native/interop.go +++ b/pkg/core/native/interop.go @@ -7,6 +7,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/interop" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/smartcontract" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" ) // Deploy deploys native contract. @@ -70,3 +71,31 @@ func Call(ic *interop.Context) error { } return nil } + +// OnPersist calls OnPersist methods for all native contracts. +func OnPersist(ic *interop.Context) error { + if ic.Trigger != trigger.OnPersist { + return errors.New("onPersist must be trigered by system") + } + for _, c := range ic.Natives { + err := c.OnPersist(ic) + if err != nil { + return err + } + } + return nil +} + +// PostPersist calls PostPersist methods for all native contracts. +func PostPersist(ic *interop.Context) error { + if ic.Trigger != trigger.PostPersist { + return errors.New("postPersist must be trigered by system") + } + for _, c := range ic.Natives { + err := c.PostPersist(ic) + if err != nil { + return err + } + } + return nil +} diff --git a/pkg/core/native/native_gas.go b/pkg/core/native/native_gas.go index 8965f0236..2d6122712 100644 --- a/pkg/core/native/native_gas.go +++ b/pkg/core/native/native_gas.go @@ -31,16 +31,11 @@ func newGAS() *GAS { nep17.symbol = "gas" nep17.decimals = 8 nep17.factor = GASFactor - nep17.onPersist = chainOnPersist(onPersistBase, g.OnPersist) nep17.incBalance = g.increaseBalance nep17.ContractID = gasContractID g.nep17TokenNative = *nep17 - onp := g.Methods["onPersist"] - onp.Func = getOnPersistWrapper(g.onPersist) - g.Methods["onPersist"] = onp - return g } @@ -98,6 +93,11 @@ func (g *GAS) OnPersist(ic *interop.Context) error { return nil } +// PostPersist implements Contract interface. +func (g *GAS) PostPersist(ic *interop.Context) error { + return nil +} + func getStandbyValidatorsHash(ic *interop.Context) (util.Uint160, error) { s, err := smartcontract.CreateDefaultMultiSigRedeemScript(ic.Chain.GetStandByValidators()) if err != nil { @@ -105,16 +105,3 @@ func getStandbyValidatorsHash(ic *interop.Context) (util.Uint160, error) { } return hash.Hash160(s), nil } - -func chainOnPersist(fs ...func(*interop.Context) error) func(*interop.Context) error { - return func(ic *interop.Context) error { - for i := range fs { - if fs[i] != nil { - if err := fs[i](ic); err != nil { - return err - } - } - } - return nil - } -} diff --git a/pkg/core/native/native_neo.go b/pkg/core/native/native_neo.go index 40800dc1a..fd255a47b 100644 --- a/pkg/core/native/native_neo.go +++ b/pkg/core/native/native_neo.go @@ -97,8 +97,6 @@ func newNEO() *NEO { nep17.symbol = "neo" nep17.decimals = 0 nep17.factor = 1 - nep17.onPersist = chainOnPersist(onPersistBase, n.OnPersist) - nep17.postPersist = chainOnPersist(nep17.postPersist, n.PostPersist) nep17.incBalance = n.increaseBalance nep17.ContractID = neoContractID @@ -109,14 +107,6 @@ func newNEO() *NEO { n.committee.Store(keysWithVotes(nil)) n.committeeHash.Store(util.Uint160{}) - onp := n.Methods["onPersist"] - onp.Func = getOnPersistWrapper(n.onPersist) - n.Methods["onPersist"] = onp - - pp := n.Methods["postPersist"] - pp.Func = getOnPersistWrapper(n.postPersist) - n.Methods["postPersist"] = pp - desc := newDescriptor("unclaimedGas", smartcontract.IntegerType, manifest.NewParameter("account", smartcontract.Hash160Type), manifest.NewParameter("end", smartcontract.IntegerType)) diff --git a/pkg/core/native/native_nep17.go b/pkg/core/native/native_nep17.go index 8f42dc8da..9049bcbb7 100644 --- a/pkg/core/native/native_nep17.go +++ b/pkg/core/native/native_nep17.go @@ -2,7 +2,6 @@ package native import ( "errors" - "fmt" "math" "math/big" @@ -33,12 +32,10 @@ func makeAccountKey(h util.Uint160) []byte { // nep17TokenNative represents NEP-17 token contract. type nep17TokenNative struct { interop.ContractMD - symbol string - decimals int64 - factor int64 - onPersist func(*interop.Context) error - postPersist func(*interop.Context) error - incBalance func(*interop.Context, util.Uint160, *state.StorageItem, *big.Int) error + symbol string + decimals int64 + factor int64 + incBalance func(*interop.Context, util.Uint160, *state.StorageItem, *big.Int) error } // totalSupplyKey is the key used to store totalSupply value. @@ -48,8 +45,6 @@ func (c *nep17TokenNative) Metadata() *interop.ContractMD { return &c.ContractMD } -var _ interop.Contract = (*nep17TokenNative)(nil) - func newNEP17Native(name string) *nep17TokenNative { n := &nep17TokenNative{ContractMD: *interop.NewContractMD(name)} n.Manifest.SupportedStandards = []string{manifest.NEP17StandardName} @@ -82,14 +77,6 @@ func newNEP17Native(name string) *nep17TokenNative { md = newMethodAndPrice(n.Transfer, 8000000, smartcontract.WriteStates|smartcontract.AllowCall|smartcontract.AllowNotify) n.AddMethod(md, desc) - desc = newDescriptor("onPersist", smartcontract.VoidType) - md = newMethodAndPrice(getOnPersistWrapper(onPersistBase), 0, smartcontract.WriteStates) - n.AddMethod(md, desc) - - desc = newDescriptor("postPersist", smartcontract.VoidType) - md = newMethodAndPrice(getOnPersistWrapper(postPersistBase), 0, smartcontract.WriteStates) - n.AddMethod(md, desc) - n.AddEvent("Transfer", transferParams...) return n @@ -333,13 +320,3 @@ func toUint32(s stackitem.Item) uint32 { } return uint32(int64Value) } - -func getOnPersistWrapper(f func(ic *interop.Context) error) interop.Method { - return func(ic *interop.Context, _ []stackitem.Item) stackitem.Item { - err := f(ic) - if err != nil { - panic(fmt.Errorf("OnPersist for native contract: %w", err)) - } - return stackitem.Null{} - } -} diff --git a/pkg/core/native/notary.go b/pkg/core/native/notary.go index 3f29bca37..9a087b721 100644 --- a/pkg/core/native/notary.go +++ b/pkg/core/native/notary.go @@ -98,14 +98,6 @@ func newNotary() *Notary { md = newMethodAndPrice(n.setMaxNotValidBeforeDelta, 300_0000, smartcontract.WriteStates) n.AddMethod(md, desc) - desc = newDescriptor("onPersist", smartcontract.VoidType) - md = newMethodAndPrice(getOnPersistWrapper(n.OnPersist), 0, smartcontract.WriteStates) - n.AddMethod(md, desc) - - desc = newDescriptor("postPersist", smartcontract.VoidType) - md = newMethodAndPrice(getOnPersistWrapper(postPersistBase), 0, smartcontract.WriteStates) - n.AddMethod(md, desc) - return n } @@ -179,6 +171,11 @@ func (n *Notary) OnPersistEnd(dao dao.DAO) error { return nil } +// PostPersist implements Contract interface. +func (n *Notary) PostPersist(ic *interop.Context) error { + return nil +} + // onPayment records deposited amount as belonging to "from" address with a lock // till the specified chain's height. func (n *Notary) onPayment(ic *interop.Context, args []stackitem.Item) stackitem.Item { diff --git a/pkg/core/native/oracle.go b/pkg/core/native/oracle.go index 92ff3f57c..a2f1e8ac7 100644 --- a/pkg/core/native/oracle.go +++ b/pkg/core/native/oracle.go @@ -122,15 +122,6 @@ func newOracle() *Oracle { md = newMethodAndPrice(o.verify, 100_0000, smartcontract.NoneFlag) o.AddMethod(md, desc) - pp := chainOnPersist(postPersistBase, o.PostPersist) - desc = newDescriptor("postPersist", smartcontract.VoidType) - md = newMethodAndPrice(getOnPersistWrapper(pp), 0, smartcontract.WriteStates) - o.AddMethod(md, desc) - - desc = newDescriptor("onPersist", smartcontract.VoidType) - md = newMethodAndPrice(getOnPersistWrapper(onPersistBase), 0, smartcontract.WriteStates) - o.AddMethod(md, desc) - o.AddEvent("OracleRequest", manifest.NewParameter("Id", smartcontract.IntegerType), manifest.NewParameter("RequestContract", smartcontract.Hash160Type), manifest.NewParameter("Url", smartcontract.StringType), @@ -141,6 +132,11 @@ func newOracle() *Oracle { return o } +// OnPersist implements Contract interface. +func (o *Oracle) OnPersist(ic *interop.Context) error { + return nil +} + // PostPersist represents `postPersist` method. func (o *Oracle) PostPersist(ic *interop.Context) error { var nodes keys.PublicKeys diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 79a7897d9..2dc80b5fb 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -124,13 +124,6 @@ func newPolicy() *Policy { md = newMethodAndPrice(p.unblockAccount, 3000000, smartcontract.WriteStates) p.AddMethod(md, desc) - desc = newDescriptor("onPersist", smartcontract.VoidType) - md = newMethodAndPrice(getOnPersistWrapper(p.OnPersist), 0, smartcontract.WriteStates) - p.AddMethod(md, desc) - - desc = newDescriptor("postPersist", smartcontract.VoidType) - md = newMethodAndPrice(getOnPersistWrapper(postPersistBase), 0, smartcontract.WriteStates) - p.AddMethod(md, desc) return p } @@ -157,6 +150,11 @@ func (p *Policy) OnPersist(ic *interop.Context) error { return nil } +// PostPersist implements Contract interface. +func (p *Policy) PostPersist(ic *interop.Context) error { + return nil +} + // OnPersistEnd updates cached Policy values if they've been changed func (p *Policy) OnPersistEnd(dao dao.DAO) error { if p.isValid { diff --git a/pkg/core/native_contract_test.go b/pkg/core/native_contract_test.go index 97b70cad1..0e1f9069a 100644 --- a/pkg/core/native_contract_test.go +++ b/pkg/core/native_contract_test.go @@ -1,6 +1,7 @@ package core import ( + "errors" "math/big" "testing" @@ -33,18 +34,19 @@ func (tn *testNative) Metadata() *interop.ContractMD { return &tn.meta } -func (tn *testNative) OnPersist(ic *interop.Context, _ []stackitem.Item) stackitem.Item { - if ic.Trigger != trigger.OnPersist { - panic("invalid trigger") - } +func (tn *testNative) OnPersist(ic *interop.Context) error { select { case tn.blocks <- ic.Block.Index: - return stackitem.NewBool(true) + return nil default: - return stackitem.NewBool(false) + return errors.New("can't send index") } } +func (tn *testNative) PostPersist(ic *interop.Context) error { + return nil +} + var _ interop.Contract = (*testNative)(nil) // registerNative registers native contract in the blockchain. @@ -106,10 +108,6 @@ func newTestNative() *testNative { RequiredFlags: smartcontract.NoneFlag} tn.meta.AddMethod(md, desc) - desc = &manifest.Method{Name: "onPersist", ReturnType: smartcontract.BoolType} - md = &interop.MethodAndPrice{Func: tn.OnPersist, RequiredFlags: smartcontract.WriteStates} - tn.meta.AddMethod(md, desc) - return tn } @@ -252,18 +250,28 @@ func TestNativeContract_InvokeOtherContract(t *testing.T) { }) require.NoError(t, err) + var drainTN = func(t *testing.T) { + select { + case <-tn.blocks: + default: + require.Fail(t, "testNative didn't send us block") + } + } + cs, _ := getTestContractState() require.NoError(t, chain.dao.PutContractState(cs)) t.Run("non-native, no return", func(t *testing.T) { res, err := invokeContractMethod(chain, testSumPrice*4+10000, tn.Metadata().Hash, "callOtherContractNoReturn", cs.Hash, "justReturn", []interface{}{}) require.NoError(t, err) + drainTN(t) checkResult(t, res, stackitem.Null{}) // simple call is done with EnsureNotEmpty }) t.Run("non-native, with return", func(t *testing.T) { res, err := invokeContractMethod(chain, testSumPrice*4+10000, tn.Metadata().Hash, "callOtherContractWithReturn", cs.Hash, "ret7", []interface{}{}) require.NoError(t, err) + drainTN(t) checkResult(t, res, stackitem.Make(8)) }) }