From 60795a899f92b8bca0bd0b3e3c523c2be2f92ebe Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 10 Aug 2023 13:54:07 +0300 Subject: [PATCH 1/5] smartcontract: improve invalid notification error text Signed-off-by: Anna Shaleva --- pkg/smartcontract/manifest/event.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/smartcontract/manifest/event.go b/pkg/smartcontract/manifest/event.go index 04a1f348d..4c8e8e2d0 100644 --- a/pkg/smartcontract/manifest/event.go +++ b/pkg/smartcontract/manifest/event.go @@ -66,11 +66,11 @@ func (e *Event) FromStackItem(item stackitem.Item) error { // current event. func (e *Event) CheckCompliance(items []stackitem.Item) error { if len(items) != len(e.Parameters) { - return errors.New("mismatch between the number of parameters and items") + return fmt.Errorf("mismatch between the number of parameters and items: %d vs %d", len(e.Parameters), len(items)) } for i := range items { if !e.Parameters[i].Type.Match(items[i]) { - return fmt.Errorf("parameter %d type mismatch: %s vs %s", i, e.Parameters[i].Type.String(), items[i].Type().String()) + return fmt.Errorf("parameter %d type mismatch: %s (manifest) vs %s (notification)", i, e.Parameters[i].Type.String(), items[i].Type().String()) } } return nil From 87aaaf1a677e2ecb5e7f602e9c04822d650a1381 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 10 Aug 2023 13:54:42 +0300 Subject: [PATCH 2/5] core: rename TestManagement_DeployUpdateHardfork So that hardfork name was explicitly present in the test name. We'll have a set of similar tests later. Signed-off-by: Anna Shaleva --- pkg/core/native/management_neotest_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/core/native/management_neotest_test.go b/pkg/core/native/management_neotest_test.go index f096b6fd9..f34f30fb4 100644 --- a/pkg/core/native/management_neotest_test.go +++ b/pkg/core/native/management_neotest_test.go @@ -37,7 +37,7 @@ func TestManagement_GetNEP17Contracts(t *testing.T) { }) } -func TestManagement_DeployUpdateHardfork(t *testing.T) { +func TestManagement_DeployUpdate_HFBasilisk(t *testing.T) { bc, acc := chain.NewSingleWithCustomConfig(t, func(c *config.Blockchain) { c.Hardforks = map[string]uint32{ config.HFBasilisk.String(): 2, From 2872c1c6687f8996215d688670475a02492aab6d Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 10 Aug 2023 14:05:32 +0300 Subject: [PATCH 3/5] core: move contract notifications check under HFBasilisk Follow the https://github.com/neo-project/neo/pull/2884. A part of the https://github.com/neo-project/neo/pull/2810. Fix failing tests along the way (a lot of them have invalid notifications format which differs from the one declared in manifest). Signed-off-by: Anna Shaleva --- docs/node-configuration.md | 2 +- internal/basicchain/testdata/test_contract.go | 3 +- internal/contracts/contracts_test.go | 61 +++++++++++++++- .../management_helper1.manifest.json | 2 +- .../management_helper/management_helper1.nef | Bin 506 -> 516 bytes .../management_helper2.manifest.json | 2 +- .../management_helper/management_helper2.nef | Bin 79 -> 79 bytes .../oracle_contract/oracle.manifest.json | 2 +- internal/contracts/oracle_contract/oracle.nef | Bin 467 -> 461 bytes pkg/compiler/interop_test.go | 5 +- pkg/config/hardfork.go | 5 +- pkg/core/interop/contract/call_test.go | 22 +++++- pkg/core/interop/runtime/engine.go | 16 ++++- pkg/core/interop/runtime/ext_test.go | 66 +++++++++++++++++- pkg/core/native/native_test/neo_test.go | 6 +- 15 files changed, 171 insertions(+), 21 deletions(-) diff --git a/docs/node-configuration.md b/docs/node-configuration.md index 1049732d4..6ff7d8e47 100644 --- a/docs/node-configuration.md +++ b/docs/node-configuration.md @@ -343,7 +343,7 @@ protocol-related settings described in the table below. | --- | --- | --- | --- | --- | | CommitteeHistory | map[uint32]uint32 | none | Number of committee members after the given height, for example `{0: 1, 20: 4}` sets up a chain with one committee member since the genesis and then changes the setting to 4 committee members at the height of 20. `StandbyCommittee` committee setting must have the number of keys equal or exceeding the highest value in this option. Blocks numbers where the change happens must be divisible by the old and by the new values simultaneously. If not set, committee size is derived from the `StandbyCommittee` setting and never changes. | | GarbageCollectionPeriod | `uint32` | 10000 | Controls MPT garbage collection interval (in blocks) for configurations with `RemoveUntraceableBlocks` enabled and `KeepOnlyLatestState` disabled. In this mode the node stores a number of MPT trees (corresponding to `MaxTraceableBlocks` and `StateSyncInterval`), but the DB needs to be clean from old entries from time to time. Doing it too often will cause too much processing overhead, doing it too rarely will leave more useless data in the DB. This setting is deprecated in favor of the same setting in the ApplicationConfiguration and will be removed in future node versions. If both settings are used, ApplicationConfiguration is prioritized over this one. | -| Hardforks | `map[string]uint32` | [] | The set of incompatible changes that affect node behaviour starting from the specified height. The default value is an empty set which should be interpreted as "each known hard-fork is applied from the zero blockchain height". The list of valid hard-fork names:
• `Aspidochelone` represents hard-fork introduced in [#2469](https://github.com/nspcc-dev/neo-go/pull/2469) (ported from the [reference](https://github.com/neo-project/neo/pull/2712)). It adjusts the prices of `System.Contract.CreateStandardAccount` and `System.Contract.CreateMultisigAccount` interops so that the resulting prices are in accordance with `sha256` method of native `CryptoLib` contract. It also includes [#2519](https://github.com/nspcc-dev/neo-go/pull/2519) (ported from the [reference](https://github.com/neo-project/neo/pull/2749)) that adjusts the price of `System.Runtime.GetRandom` interop and fixes its vulnerability. A special NeoGo-specific change is included as well for ContractManagement's update/deploy call flags behaviour to be compatible with pre-0.99.0 behaviour that was changed because of the [3.2.0 protocol change](https://github.com/neo-project/neo/pull/2653).
• `Basilisk` represents hard-fork introduced in [#3056](https://github.com/nspcc-dev/neo-go/pull/3056) (ported from the [reference](https://github.com/neo-project/neo/pull/2881)). It enables strict smart contract script check against a set of JMP instructions and against method boundaries enabled on contract deploy or update. It also includes [#3080](https://github.com/nspcc-dev/neo-go/pull/3080) (ported from the [reference](https://github.com/neo-project/neo/pull/2883)) that increases `stackitem.Integer` JSON parsing precision up to the maximum value supported by the NeoVM.| +| Hardforks | `map[string]uint32` | [] | The set of incompatible changes that affect node behaviour starting from the specified height. The default value is an empty set which should be interpreted as "each known hard-fork is applied from the zero blockchain height". The list of valid hard-fork names:
• `Aspidochelone` represents hard-fork introduced in [#2469](https://github.com/nspcc-dev/neo-go/pull/2469) (ported from the [reference](https://github.com/neo-project/neo/pull/2712)). It adjusts the prices of `System.Contract.CreateStandardAccount` and `System.Contract.CreateMultisigAccount` interops so that the resulting prices are in accordance with `sha256` method of native `CryptoLib` contract. It also includes [#2519](https://github.com/nspcc-dev/neo-go/pull/2519) (ported from the [reference](https://github.com/neo-project/neo/pull/2749)) that adjusts the price of `System.Runtime.GetRandom` interop and fixes its vulnerability. A special NeoGo-specific change is included as well for ContractManagement's update/deploy call flags behaviour to be compatible with pre-0.99.0 behaviour that was changed because of the [3.2.0 protocol change](https://github.com/neo-project/neo/pull/2653).
• `Basilisk` represents hard-fork introduced in [#3056](https://github.com/nspcc-dev/neo-go/pull/3056) (ported from the [reference](https://github.com/neo-project/neo/pull/2881)). It enables strict smart contract script check against a set of JMP instructions and against method boundaries enabled on contract deploy or update. It also includes [#3080](https://github.com/nspcc-dev/neo-go/pull/3080) (ported from the [reference](https://github.com/neo-project/neo/pull/2883)) that increases `stackitem.Integer` JSON parsing precision up to the maximum value supported by the NeoVM. It also includes [#3085](https://github.com/nspcc-dev/neo-go/pull/3085) (ported from the [reference](https://github.com/neo-project/neo/pull/2810)) that enables strict check for notifications emitted by a contract to precisely match the events specified in the contract manifest. | | KeepOnlyLatestState | `bool` | `false` | Specifies if MPT should only store the latest state (or a set of latest states, see `P2PStateExcangeExtensions` section for details). If true, DB size will be smaller, but older roots won't be accessible. This value should remain the same for the same database. | This setting is deprecated in favor of the same setting in the ApplicationConfiguration and will be removed in future node versions. If both settings are used, setting any of them to true enables the function. | | Magic | `uint32` | `0` | Magic number which uniquely identifies Neo network. | | MaxBlockSize | `uint32` | `262144` | Maximum block size in bytes. | diff --git a/internal/basicchain/testdata/test_contract.go b/internal/basicchain/testdata/test_contract.go index 7b0a7387a..765ac070e 100644 --- a/internal/basicchain/testdata/test_contract.go +++ b/internal/basicchain/testdata/test_contract.go @@ -23,7 +23,8 @@ func Init() bool { h := runtime.GetExecutingScriptHash() amount := totalSupply storage.Put(ctx, h, amount) - runtime.Notify("Transfer", interop.Hash160([]byte{}), h, amount) + runtime.Notify("Transfer", interop.Hash160(nil), // should use `nil` (not `[]byte{}`) due to notifications manifest compliance check. + h, amount) return true } diff --git a/internal/contracts/contracts_test.go b/internal/contracts/contracts_test.go index 52fe24baa..426b184ef 100644 --- a/internal/contracts/contracts_test.go +++ b/internal/contracts/contracts_test.go @@ -132,14 +132,14 @@ func generateManagementHelperContracts(t *testing.T, saveState bool) { emit.Syscall(w.BinWriter, interopnames.SystemRuntimeGetCallingScriptHash) emit.Int(w.BinWriter, 4) emit.Opcodes(w.BinWriter, opcode.PACK) - emit.String(w.BinWriter, "LastPayment") + emit.String(w.BinWriter, "LastPaymentNEP17") emit.Syscall(w.BinWriter, interopnames.SystemRuntimeNotify) emit.Opcodes(w.BinWriter, opcode.RET) onNEP11PaymentOff := w.Len() emit.Syscall(w.BinWriter, interopnames.SystemRuntimeGetCallingScriptHash) emit.Int(w.BinWriter, 5) emit.Opcodes(w.BinWriter, opcode.PACK) - emit.String(w.BinWriter, "LostPayment") + emit.String(w.BinWriter, "LastPaymentNEP11") emit.Syscall(w.BinWriter, interopnames.SystemRuntimeNotify) emit.Opcodes(w.BinWriter, opcode.RET) update3Off := w.Len() @@ -352,6 +352,63 @@ func generateManagementHelperContracts(t *testing.T, saveState bool) { ReturnType: smartcontract.IntegerType, }, } + m.ABI.Events = []manifest.Event{ + { + Name: "LastPaymentNEP17", + Parameters: []manifest.Parameter{ + { + Name: "from", + Type: smartcontract.Hash160Type, + }, + { + Name: "to", + Type: smartcontract.Hash160Type, + }, + { + Name: "amount", + Type: smartcontract.IntegerType, + }, + { + Name: "data", + Type: smartcontract.AnyType, + }, + }, + }, + { + Name: "LastPaymentNEP11", + Parameters: []manifest.Parameter{ + { + Name: "from", + Type: smartcontract.Hash160Type, + }, + { + Name: "to", + Type: smartcontract.Hash160Type, + }, + { + Name: "amount", + Type: smartcontract.IntegerType, + }, + { + Name: "tokenId", + Type: smartcontract.ByteArrayType, + }, + { + Name: "data", + Type: smartcontract.AnyType, + }, + }, + }, + { + Name: "event", // This event is not emitted by the contract code and needed for System.Runtime.Notify tests. + Parameters: []manifest.Parameter{ + { + Name: "any", + Type: smartcontract.AnyType, + }, + }, + }, + } m.Permissions = make([]manifest.Permission, 2) m.Permissions[0].Contract.Type = manifest.PermissionHash m.Permissions[0].Contract.Value = neoHash diff --git a/internal/contracts/management_helper/management_helper1.manifest.json b/internal/contracts/management_helper/management_helper1.manifest.json index 181b16b7c..99ccb0d65 100644 --- a/internal/contracts/management_helper/management_helper1.manifest.json +++ b/internal/contracts/management_helper/management_helper1.manifest.json @@ -1 +1 @@ -{"name":"TestMain","abi":{"methods":[{"name":"add","offset":1,"parameters":[{"name":"addend1","type":"Integer"},{"name":"addend2","type":"Integer"}],"returntype":"Integer","safe":false},{"name":"add","offset":3,"parameters":[{"name":"addend1","type":"Integer"},{"name":"addend2","type":"Integer"},{"name":"addend3","type":"Integer"}],"returntype":"Integer","safe":false},{"name":"ret7","offset":6,"parameters":[],"returntype":"Integer","safe":false},{"name":"drop","offset":8,"parameters":null,"returntype":"Void","safe":false},{"name":"_initialize","offset":10,"parameters":null,"returntype":"Void","safe":false},{"name":"add3","offset":15,"parameters":[{"name":"addend","type":"Integer"}],"returntype":"Integer","safe":false},{"name":"invalidReturn","offset":18,"parameters":null,"returntype":"Integer","safe":false},{"name":"justReturn","offset":21,"parameters":null,"returntype":"Void","safe":false},{"name":"verify","offset":22,"parameters":null,"returntype":"Boolean","safe":false},{"name":"_deploy","offset":27,"parameters":[{"name":"data","type":"Any"},{"name":"isUpdate","type":"Boolean"}],"returntype":"Void","safe":false},{"name":"getValue","offset":158,"parameters":null,"returntype":"String","safe":false},{"name":"putValue","offset":138,"parameters":[{"name":"value","type":"String"}],"returntype":"Void","safe":false},{"name":"delValue","offset":178,"parameters":[{"name":"key","type":"String"}],"returntype":"Void","safe":false},{"name":"onNEP11Payment","offset":215,"parameters":[{"name":"from","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"tokenid","type":"ByteArray"},{"name":"data","type":"Any"}],"returntype":"Void","safe":false},{"name":"onNEP17Payment","offset":189,"parameters":[{"name":"from","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"data","type":"Any"}],"returntype":"Void","safe":false},{"name":"update","offset":244,"parameters":[{"name":"nef","type":"ByteArray"},{"name":"manifest","type":"ByteArray"}],"returntype":"Void","safe":false},{"name":"update","offset":241,"parameters":[{"name":"nef","type":"ByteArray"},{"name":"manifest","type":"ByteArray"},{"name":"data","type":"Any"}],"returntype":"Void","safe":false},{"name":"destroy","offset":284,"parameters":null,"returntype":"Void","safe":false},{"name":"invalidStack1","offset":324,"parameters":null,"returntype":"Any","safe":false},{"name":"invalidStack2","offset":329,"parameters":null,"returntype":"Any","safe":false},{"name":"callT0","offset":335,"parameters":[{"name":"address","type":"Hash160"}],"returntype":"Integer","safe":false},{"name":"callT1","offset":341,"parameters":null,"returntype":"Integer","safe":false},{"name":"callT2","offset":345,"parameters":null,"returntype":"Integer","safe":false},{"name":"burnGas","offset":349,"parameters":[{"name":"amount","type":"Integer"}],"returntype":"Void","safe":false},{"name":"invocCounter","offset":355,"parameters":null,"returntype":"Integer","safe":false}],"events":[]},"features":{},"groups":[],"permissions":[{"contract":"0xef4073a0f2b305a38ec4050e4d3d28bc40ea63f5","methods":["balanceOf"]},{"contract":"0x0000000000000000000000000000000000000000","methods":["method"]}],"supportedstandards":[],"trusts":[],"extra":null} \ No newline at end of file +{"name":"TestMain","abi":{"methods":[{"name":"add","offset":1,"parameters":[{"name":"addend1","type":"Integer"},{"name":"addend2","type":"Integer"}],"returntype":"Integer","safe":false},{"name":"add","offset":3,"parameters":[{"name":"addend1","type":"Integer"},{"name":"addend2","type":"Integer"},{"name":"addend3","type":"Integer"}],"returntype":"Integer","safe":false},{"name":"ret7","offset":6,"parameters":[],"returntype":"Integer","safe":false},{"name":"drop","offset":8,"parameters":null,"returntype":"Void","safe":false},{"name":"_initialize","offset":10,"parameters":null,"returntype":"Void","safe":false},{"name":"add3","offset":15,"parameters":[{"name":"addend","type":"Integer"}],"returntype":"Integer","safe":false},{"name":"invalidReturn","offset":18,"parameters":null,"returntype":"Integer","safe":false},{"name":"justReturn","offset":21,"parameters":null,"returntype":"Void","safe":false},{"name":"verify","offset":22,"parameters":null,"returntype":"Boolean","safe":false},{"name":"_deploy","offset":27,"parameters":[{"name":"data","type":"Any"},{"name":"isUpdate","type":"Boolean"}],"returntype":"Void","safe":false},{"name":"getValue","offset":158,"parameters":null,"returntype":"String","safe":false},{"name":"putValue","offset":138,"parameters":[{"name":"value","type":"String"}],"returntype":"Void","safe":false},{"name":"delValue","offset":178,"parameters":[{"name":"key","type":"String"}],"returntype":"Void","safe":false},{"name":"onNEP11Payment","offset":220,"parameters":[{"name":"from","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"tokenid","type":"ByteArray"},{"name":"data","type":"Any"}],"returntype":"Void","safe":false},{"name":"onNEP17Payment","offset":189,"parameters":[{"name":"from","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"data","type":"Any"}],"returntype":"Void","safe":false},{"name":"update","offset":254,"parameters":[{"name":"nef","type":"ByteArray"},{"name":"manifest","type":"ByteArray"}],"returntype":"Void","safe":false},{"name":"update","offset":251,"parameters":[{"name":"nef","type":"ByteArray"},{"name":"manifest","type":"ByteArray"},{"name":"data","type":"Any"}],"returntype":"Void","safe":false},{"name":"destroy","offset":294,"parameters":null,"returntype":"Void","safe":false},{"name":"invalidStack1","offset":334,"parameters":null,"returntype":"Any","safe":false},{"name":"invalidStack2","offset":339,"parameters":null,"returntype":"Any","safe":false},{"name":"callT0","offset":345,"parameters":[{"name":"address","type":"Hash160"}],"returntype":"Integer","safe":false},{"name":"callT1","offset":351,"parameters":null,"returntype":"Integer","safe":false},{"name":"callT2","offset":355,"parameters":null,"returntype":"Integer","safe":false},{"name":"burnGas","offset":359,"parameters":[{"name":"amount","type":"Integer"}],"returntype":"Void","safe":false},{"name":"invocCounter","offset":365,"parameters":null,"returntype":"Integer","safe":false}],"events":[{"name":"LastPaymentNEP17","parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"data","type":"Any"}]},{"name":"LastPaymentNEP11","parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"tokenId","type":"ByteArray"},{"name":"data","type":"Any"}]},{"name":"event","parameters":[{"name":"any","type":"Any"}]}]},"features":{},"groups":[],"permissions":[{"contract":"0xef4073a0f2b305a38ec4050e4d3d28bc40ea63f5","methods":["balanceOf"]},{"contract":"0x0000000000000000000000000000000000000000","methods":["method"]}],"supportedstandards":[],"trusts":[],"extra":null} \ No newline at end of file diff --git a/internal/contracts/management_helper/management_helper1.nef b/internal/contracts/management_helper/management_helper1.nef index 85288654f2c42bb4ded61bfb58acbc60a729bb2f..b2e11332c119476e5d65931e9066f0bcdf64a423 100644 GIT binary patch delta 92 zcmeyx+`=N{=jvvhmzuAep05j}N>YnUCJLHNbaQ1ap4fBDK)@%lxFjI4GB-7^#LqRr h(A;qC-nmWxp)cZ delta 49 zcmX@he3^Md52NG6-bOA~f!hiyKAAUEoF^}26q~HWsKp5qQeqXntuQ&7F^}bDfAD4i DnOhJh diff --git a/pkg/compiler/interop_test.go b/pkg/compiler/interop_test.go index b43e4824b..f27c1bd6b 100644 --- a/pkg/compiler/interop_test.go +++ b/pkg/compiler/interop_test.go @@ -10,6 +10,7 @@ import ( "github.com/nspcc-dev/neo-go/internal/fakechain" "github.com/nspcc-dev/neo-go/pkg/compiler" + "github.com/nspcc-dev/neo-go/pkg/config" "github.com/nspcc-dev/neo-go/pkg/core" "github.com/nspcc-dev/neo-go/pkg/core/dao" "github.com/nspcc-dev/neo-go/pkg/core/interop" @@ -572,7 +573,9 @@ func TestForcedNotifyArgumentsConversion(t *testing.T) { const methodWithEllipsis = "withEllipsis" const methodWithoutEllipsis = "withoutEllipsis" check := func(t *testing.T, method string, targetSCParamTypes []smartcontract.ParamType, expectedVMParamTypes []stackitem.Type, noEventsCheck bool) { - bc, acc := chain.NewSingle(t) + bc, acc := chain.NewSingleWithCustomConfig(t, func(blockchain *config.Blockchain) { + blockchain.Hardforks = map[string]uint32{config.HFBasilisk.String(): 100500} // Disable runtime notifications check to reuse the same contract for different event parameter types. + }) e := neotest.NewExecutor(t, bc, acc, acc) src := `package foo import "github.com/nspcc-dev/neo-go/pkg/interop/runtime" diff --git a/pkg/config/hardfork.go b/pkg/config/hardfork.go index 21409b60d..ac71b9032 100644 --- a/pkg/config/hardfork.go +++ b/pkg/config/hardfork.go @@ -11,8 +11,9 @@ const ( // https://github.com/neo-project/neo/pull/2749). HFAspidochelone Hardfork = 1 << iota // Aspidochelone // HFBasilisk represents hard-fork introduced in #3056 (ported from - // https://github.com/neo-project/neo/pull/2881) and #3080 (ported from - // https://github.com/neo-project/neo/pull/2883). + // https://github.com/neo-project/neo/pull/2881), #3080 (ported from + // https://github.com/neo-project/neo/pull/2883) and #3085 (ported from + // https://github.com/neo-project/neo/pull/2810). HFBasilisk // Basilisk ) diff --git a/pkg/core/interop/contract/call_test.go b/pkg/core/interop/contract/call_test.go index e51671562..451d5b41d 100644 --- a/pkg/core/interop/contract/call_test.go +++ b/pkg/core/interop/contract/call_test.go @@ -19,6 +19,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/transaction" "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" "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" @@ -247,6 +248,9 @@ func TestSnapshotIsolation_Exceptions(t *testing.T) { NoPermissionsCheck: true, Name: "contractA", Permissions: []manifest.Permission{{Methods: manifest.WildStrings{Value: nil}}}, + ContractEvents: []compiler.HybridEvent{ + {Name: "NotificationFromA", Parameters: []compiler.HybridParameter{{Parameter: manifest.Parameter{Name: "i", Type: smartcontract.IntegerType}}}}, + }, }) e.DeployContract(t, ctrA, nil) @@ -319,6 +323,10 @@ func TestSnapshotIsolation_Exceptions(t *testing.T) { NoEventsCheck: true, NoPermissionsCheck: true, Permissions: []manifest.Permission{{Methods: manifest.WildStrings{Value: nil}}}, + ContractEvents: []compiler.HybridEvent{ + {Name: "NotificationFromB before panic", Parameters: []compiler.HybridParameter{{Parameter: manifest.Parameter{Name: "i", Type: smartcontract.IntegerType}}}}, + {Name: "NotificationFromB after panic", Parameters: []compiler.HybridParameter{{Parameter: manifest.Parameter{Name: "i", Type: smartcontract.IntegerType}}}}, + }, }) e.DeployContract(t, ctrB, nil) @@ -382,6 +390,16 @@ func TestSnapshotIsolation_NestedContextException(t *testing.T) { NoPermissionsCheck: true, Name: "contractA", Permissions: []manifest.Permission{{Methods: manifest.WildStrings{Value: nil}}}, + ContractEvents: []compiler.HybridEvent{ + {Name: "Calling A"}, + {Name: "Finish"}, + {Name: "Caught"}, + {Name: "A"}, + {Name: "Unreachable A"}, + {Name: "B"}, + {Name: "Unreachable B"}, + {Name: "C"}, + }, }) e.DeployContract(t, ctrA, nil) @@ -515,10 +533,10 @@ func TestRET_after_FINALLY_CallNonVoidAfterVoidMethod(t *testing.T) { srcA := `package contractA import "github.com/nspcc-dev/neo-go/pkg/interop/runtime" func NoRet() { - runtime.Notify("no ret") + runtime.Log("no ret") } func HasRet() int { - runtime.Notify("ret") + runtime.Log("ret") return 5 }` ctrA := neotest.CompileSource(t, acc.ScriptHash(), strings.NewReader(srcA), &compiler.Options{ diff --git a/pkg/core/interop/runtime/engine.go b/pkg/core/interop/runtime/engine.go index be9e78764..05ee4a86d 100644 --- a/pkg/core/interop/runtime/engine.go +++ b/pkg/core/interop/runtime/engine.go @@ -5,6 +5,7 @@ import ( "fmt" "math/big" + "github.com/nspcc-dev/neo-go/pkg/config" "github.com/nspcc-dev/neo-go/pkg/core/interop" "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" "github.com/nspcc-dev/neo-go/pkg/vm" @@ -83,15 +84,24 @@ func Notify(ic *interop.Context) error { if err != nil { return errors.New("notifications are not allowed in dynamic scripts") } - ev := ctr.Manifest.ABI.GetEvent(name) + var ( + ev = ctr.Manifest.ABI.GetEvent(name) + checkErr error + ) if ev == nil { - ic.Log.Info("bad notification", zap.String("contract", curHash.StringLE()), zap.String("event", name), zap.Error(fmt.Errorf("event %s does not exist", name))) + checkErr = fmt.Errorf("notification %s does not exist", name) } else { err = ev.CheckCompliance(args) if err != nil { - ic.Log.Info("bad notification", zap.String("contract", curHash.StringLE()), zap.String("event", name), zap.Error(err)) + checkErr = fmt.Errorf("notification %s is invalid: %w", name, err) } } + if checkErr != nil { + if ic.IsHardforkEnabled(config.HFBasilisk) { + return checkErr + } + ic.Log.Info("bad notification", zap.String("contract", curHash.StringLE()), zap.String("event", name), zap.Error(checkErr)) + } // But it has to be serializable, otherwise we either have some broken // (recursive) structure inside or an interop item that can't be used diff --git a/pkg/core/interop/runtime/ext_test.go b/pkg/core/interop/runtime/ext_test.go index 64feb3ecb..997d97728 100644 --- a/pkg/core/interop/runtime/ext_test.go +++ b/pkg/core/interop/runtime/ext_test.go @@ -655,7 +655,7 @@ func TestNotify(t *testing.T) { t.Run("recursive struct", func(t *testing.T) { arr := stackitem.NewArray([]stackitem.Item{stackitem.Null{}}) arr.Append(arr) - ic := newIC("event", arr) + ic := newIC("event", stackitem.NewArray([]stackitem.Item{arr})) // upper array is needed to match manifest event signature. require.Error(t, runtime.Notify(ic)) }) t.Run("big notification", func(t *testing.T) { @@ -666,13 +666,13 @@ func TestNotify(t *testing.T) { }) t.Run("good", func(t *testing.T) { arr := stackitem.NewArray([]stackitem.Item{stackitem.Make(42)}) - ic := newIC("good event", arr) + ic := newIC("event", arr) require.NoError(t, runtime.Notify(ic)) require.Equal(t, 1, len(ic.Notifications)) arr.MarkAsReadOnly() // tiny hack for test to be able to compare object references. ev := ic.Notifications[0] - require.Equal(t, "good event", ev.Name) + require.Equal(t, "event", ev.Name) require.Equal(t, ic.VM.GetCurrentScriptHash(), ev.ScriptHash) require.Equal(t, arr, ev.Item) // Check deep copy. @@ -680,3 +680,63 @@ func TestNotify(t *testing.T) { require.NotEqual(t, arr, ev.Item) }) } + +func TestSystemRuntimeNotify_HFBasilisk(t *testing.T) { + const ntfName = "Hello, world!" + + bc, acc := chain.NewSingleWithCustomConfig(t, func(c *config.Blockchain) { + c.Hardforks = map[string]uint32{ + config.HFBasilisk.String(): 2, + } + }) + e := neotest.NewExecutor(t, bc, acc, acc) + + script := io.NewBufBinWriter() + emit.Array(script.BinWriter, stackitem.Make(true)) // Boolean instead of Integer declared in manifest + emit.String(script.BinWriter, ntfName) + emit.Syscall(script.BinWriter, interopnames.SystemRuntimeNotify) + require.NoError(t, script.Err) + ne, err := nef.NewFile(script.Bytes()) + require.NoError(t, err) + + m := &manifest.Manifest{ + Name: "ctr", + ABI: manifest.ABI{ + Methods: []manifest.Method{ + { + Name: "main", + Offset: 0, + ReturnType: smartcontract.VoidType, + }, + }, + Events: []manifest.Event{ + { + Name: ntfName, + Parameters: []manifest.Parameter{ + { + Name: "int", + Type: smartcontract.IntegerType, + }, + }, + }, + }, + }, + } + ctr := &neotest.Contract{ + Hash: state.CreateContractHash(e.Validator.ScriptHash(), ne.Checksum, m.Name), + NEF: ne, + Manifest: m, + } + ctrInv := e.NewInvoker(ctr.Hash, e.Validator) + + // Block 1: deploy contract. + e.DeployContract(t, ctr, nil) + + // Block 2: bad event should be logged. + ctrInv.Invoke(t, nil, "main") + + // Block 3: bad event should fault the execution. + ctrInv.InvokeFail(t, + "System.Runtime.Notify failed: notification Hello, world! is invalid: parameter 0 type mismatch: Integer (manifest) vs Boolean (notification)", + "main") +} diff --git a/pkg/core/native/native_test/neo_test.go b/pkg/core/native/native_test/neo_test.go index dae6e1eb5..cbaefa883 100644 --- a/pkg/core/native/native_test/neo_test.go +++ b/pkg/core/native/native_test/neo_test.go @@ -470,7 +470,7 @@ func TestNEO_TransferOnPayment(t *testing.T) { require.Equal(t, 3, len(aer.Events)) // transfer + GAS claim for sender + onPayment e.CheckTxNotificationEvent(t, h, 1, state.NotificationEvent{ ScriptHash: cs.Hash, - Name: "LastPayment", + Name: "LastPaymentNEP17", Item: stackitem.NewArray([]stackitem.Item{ stackitem.NewByteArray(neoValidatorsInvoker.Hash.BytesBE()), stackitem.NewByteArray(e.Validator.ScriptHash().BytesBE()), @@ -484,7 +484,7 @@ func TestNEO_TransferOnPayment(t *testing.T) { require.Equal(t, 5, len(aer.Events)) // Now we must also have GAS claim for contract and corresponding `onPayment`. e.CheckTxNotificationEvent(t, h, 1, state.NotificationEvent{ // onPayment for NEO transfer ScriptHash: cs.Hash, - Name: "LastPayment", + Name: "LastPaymentNEP17", Item: stackitem.NewArray([]stackitem.Item{ stackitem.NewByteArray(e.NativeHash(t, nativenames.Neo).BytesBE()), stackitem.NewByteArray(e.Validator.ScriptHash().BytesBE()), @@ -494,7 +494,7 @@ func TestNEO_TransferOnPayment(t *testing.T) { }) e.CheckTxNotificationEvent(t, h, 4, state.NotificationEvent{ // onPayment for GAS claim ScriptHash: cs.Hash, - Name: "LastPayment", + Name: "LastPaymentNEP17", Item: stackitem.NewArray([]stackitem.Item{ stackitem.NewByteArray(e.NativeHash(t, nativenames.Gas).BytesBE()), stackitem.Null{}, From ed2c4b03190c8bc3908b54ebabe05e2c8dab0b1f Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 18 Aug 2023 16:38:40 +0300 Subject: [PATCH 4/5] core: improve error checks in TestNotify Ensure that the error returned from runtime.Notify is exactly the error that was expected. Signed-off-by: Anna Shaleva --- pkg/core/interop/runtime/ext_test.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pkg/core/interop/runtime/ext_test.go b/pkg/core/interop/runtime/ext_test.go index 997d97728..c0764fd9f 100644 --- a/pkg/core/interop/runtime/ext_test.go +++ b/pkg/core/interop/runtime/ext_test.go @@ -5,6 +5,7 @@ import ( "math" "math/big" "path/filepath" + "strings" "testing" "github.com/nspcc-dev/neo-go/internal/contracts" @@ -643,26 +644,34 @@ func TestNotify(t *testing.T) { } t.Run("big name", func(t *testing.T) { ic := newIC(string(make([]byte, runtime.MaxEventNameLen+1)), stackitem.NewArray([]stackitem.Item{stackitem.Null{}})) - require.Error(t, runtime.Notify(ic)) + err := runtime.Notify(ic) + require.Error(t, err) + require.True(t, strings.Contains(err.Error(), "event name must be less than 32"), err) }) t.Run("dynamic script", func(t *testing.T) { ic := newIC("some", stackitem.Null{}) ic.VM.LoadScriptWithHash([]byte{1}, random.Uint160(), callflag.NoneFlag) ic.VM.Estack().PushVal(stackitem.NewArray([]stackitem.Item{stackitem.Make(42)})) ic.VM.Estack().PushVal("event") - require.Error(t, runtime.Notify(ic)) + err := runtime.Notify(ic) + require.Error(t, err) + require.True(t, strings.Contains(err.Error(), "notifications are not allowed in dynamic scripts"), err) }) t.Run("recursive struct", func(t *testing.T) { arr := stackitem.NewArray([]stackitem.Item{stackitem.Null{}}) arr.Append(arr) ic := newIC("event", stackitem.NewArray([]stackitem.Item{arr})) // upper array is needed to match manifest event signature. - require.Error(t, runtime.Notify(ic)) + err := runtime.Notify(ic) + require.Error(t, err) + require.True(t, strings.Contains(err.Error(), "bad notification: recursive item"), err) }) t.Run("big notification", func(t *testing.T) { bs := stackitem.NewByteArray(make([]byte, runtime.MaxNotificationSize+1)) arr := stackitem.NewArray([]stackitem.Item{bs}) ic := newIC("event", arr) - require.Error(t, runtime.Notify(ic)) + err := runtime.Notify(ic) + require.Error(t, err) + require.True(t, strings.Contains(err.Error(), "notification size shouldn't exceed 1024"), err) }) t.Run("good", func(t *testing.T) { arr := stackitem.NewArray([]stackitem.Item{stackitem.Make(42)}) From bb2a99d4519e5f797d3c12d26c6eea22f1d15368 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 18 Aug 2023 15:10:04 +0300 Subject: [PATCH 5/5] smartcontract: disallow Null and non-utf8 String Follow the https://github.com/neo-project/neo/pull/2810#discussion_r1295900728. Signed-off-by: Anna Shaleva --- pkg/smartcontract/param_type.go | 5 +++- pkg/smartcontract/param_type_test.go | 42 +++++++++++++++------------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/pkg/smartcontract/param_type.go b/pkg/smartcontract/param_type.go index 403a48649..cb32c72ab 100644 --- a/pkg/smartcontract/param_type.go +++ b/pkg/smartcontract/param_type.go @@ -7,6 +7,7 @@ import ( "fmt" "math/big" "strings" + "unicode/utf8" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/encoding/address" @@ -203,8 +204,10 @@ func (pt ParamType) Match(v stackitem.Item) bool { return vt == stackitem.BooleanT case IntegerType: return vt == stackitem.IntegerT - case ByteArrayType, StringType: + case ByteArrayType: return vt == stackitem.ByteArrayT || vt == stackitem.BufferT || vt == stackitem.AnyT + case StringType: + return (vt == stackitem.ByteArrayT || vt == stackitem.BufferT) && utf8.Valid(v.Value().([]byte)) case Hash160Type: return checkBytesWithLen(vt, v, Hash160Len) case Hash256Type: diff --git a/pkg/smartcontract/param_type_test.go b/pkg/smartcontract/param_type_test.go index 9d3e51d2f..4f754d5ae 100644 --- a/pkg/smartcontract/param_type_test.go +++ b/pkg/smartcontract/param_type_test.go @@ -428,25 +428,27 @@ func TestConvertToStackitemType(t *testing.T) { func TestParamTypeMatch(t *testing.T) { for itm, pt := range map[stackitem.Item]ParamType{ - &stackitem.Pointer{}: BoolType, - &stackitem.Pointer{}: MapType, - stackitem.Make(0): BoolType, - stackitem.Make(0): ByteArrayType, - stackitem.Make(0): StringType, - stackitem.Make(false): ByteArrayType, - stackitem.Make(true): StringType, - stackitem.Make([]byte{1}): Hash160Type, - stackitem.Make([]byte{1}): Hash256Type, - stackitem.Make([]byte{1}): PublicKeyType, - stackitem.Make([]byte{1}): SignatureType, - stackitem.Make(0): Hash160Type, - stackitem.Make(0): Hash256Type, - stackitem.Make(0): PublicKeyType, - stackitem.Make(0): SignatureType, - stackitem.Make(0): ArrayType, - stackitem.Make(0): MapType, - stackitem.Make(0): InteropInterfaceType, - stackitem.Make(0): VoidType, + &stackitem.Pointer{}: BoolType, + &stackitem.Pointer{}: MapType, + stackitem.Make(0): BoolType, + stackitem.Make(0): ByteArrayType, + stackitem.Make(0): StringType, + stackitem.Make(false): ByteArrayType, + stackitem.Make(true): StringType, + stackitem.Make([]byte{1}): Hash160Type, + stackitem.Make([]byte{1}): Hash256Type, + stackitem.Make([]byte{1}): PublicKeyType, + stackitem.Make([]byte{1}): SignatureType, + stackitem.Make(0): Hash160Type, + stackitem.Make(0): Hash256Type, + stackitem.Make(0): PublicKeyType, + stackitem.Make(0): SignatureType, + stackitem.Make(0): ArrayType, + stackitem.Make(0): MapType, + stackitem.Make(0): InteropInterfaceType, + stackitem.Make(0): VoidType, + stackitem.Null{}: StringType, + stackitem.Make([]byte{0x80}): StringType, // non utf-8 } { require.Falsef(t, pt.Match(itm), "%s - %s", pt.String(), itm.String()) } @@ -456,11 +458,11 @@ func TestParamTypeMatch(t *testing.T) { stackitem.Make(0): IntegerType, stackitem.Make(100500): IntegerType, stackitem.Make([]byte{1}): ByteArrayType, + stackitem.Make([]byte{0x80}): ByteArrayType, // non utf-8 stackitem.Make([]byte{1}): StringType, stackitem.NewBuffer([]byte{1}): ByteArrayType, stackitem.NewBuffer([]byte{1}): StringType, stackitem.Null{}: ByteArrayType, - stackitem.Null{}: StringType, stackitem.Make(util.Uint160{}.BytesBE()): Hash160Type, stackitem.Make(util.Uint256{}.BytesBE()): Hash256Type, stackitem.Null{}: Hash160Type,