From 4671fbb3be8390f6f3a4e238cd33014d0b0b89ab Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Sat, 18 Mar 2023 10:44:12 +0300 Subject: [PATCH 1/2] interop: drop deprecated util.FromAddress It still was used in a number of places, surprisingly. --- ROADMAP.md | 11 --- cli/vm/cli_test.go | 4 +- examples/nft-d/nft.go | 3 +- examples/nft-nd/nft.go | 3 +- examples/runtime/runtime.go | 4 +- examples/timer/timer.go | 4 +- examples/token/token.go | 4 +- internal/basicchain/testdata/test_contract.go | 4 +- .../testdata/verify/verification_contract.go | 3 +- pkg/compiler/analysis.go | 16 ----- pkg/compiler/codegen.go | 12 ++-- pkg/compiler/function_call_test.go | 6 +- pkg/compiler/interop_test.go | 67 +++++-------------- pkg/core/blockchain_neotest_test.go | 4 +- pkg/interop/util/util.go | 10 --- 15 files changed, 42 insertions(+), 113 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index ae96679a7..669b0b259 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -40,17 +40,6 @@ While a lot of the code is already converted to new APIs, old ones still can be used in some code not known to us. Therefore we will remove old APIs not earlier than May 2023, with 0.103.0 release. -## util.FromAddress smart contract helper - -`util` smart contract library has a FromAddress function that is one of the -oldest lines in the entire NeoGo code base, dating back to 2018. Version -0.99.4 of NeoGo (October 2022) has introduced a new `address` package with -`ToHash160` function, it covers a bit more use cases but can be used as a -direct replacement of the old function, so please update your code. - -util.FromAddress is expected to be removed around March 2023 (~0.102.0 -release). - ## WSClient Notifications channel and SubscribeFor* APIs Version 0.99.5 of NeoGo introduces a new set of subscription APIs that gives diff --git a/cli/vm/cli_test.go b/cli/vm/cli_test.go index 2b6e7db45..6cdd5c3eb 100644 --- a/cli/vm/cli_test.go +++ b/cli/vm/cli_test.go @@ -432,10 +432,10 @@ go 1.17`) srcCheckWitness := `package kek import ( "github.com/nspcc-dev/neo-go/pkg/interop/runtime" - "github.com/nspcc-dev/neo-go/pkg/interop/util" + "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" ) func Main() bool { - var owner = util.FromAddress("` + ownerAddress + `") + var owner = address.ToHash160("` + ownerAddress + `") return runtime.CheckWitness(owner) } ` diff --git a/examples/nft-d/nft.go b/examples/nft-d/nft.go index 66afd2ef6..26313f716 100644 --- a/examples/nft-d/nft.go +++ b/examples/nft-d/nft.go @@ -10,6 +10,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/interop" "github.com/nspcc-dev/neo-go/pkg/interop/contract" "github.com/nspcc-dev/neo-go/pkg/interop/iterator" + "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" "github.com/nspcc-dev/neo-go/pkg/interop/native/crypto" "github.com/nspcc-dev/neo-go/pkg/interop/native/gas" "github.com/nspcc-dev/neo-go/pkg/interop/native/management" @@ -39,7 +40,7 @@ var ( // contractOwner is a special address that can perform some management // functions on this contract like updating/destroying it and can also // be used for contract address verification. - contractOwner = util.FromAddress("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB") + contractOwner = address.ToHash160("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB") ) // ObjectIdentifier represents NFT structure and contains the container ID and diff --git a/examples/nft-nd/nft.go b/examples/nft-nd/nft.go index f4f87b743..3dd5f201c 100644 --- a/examples/nft-nd/nft.go +++ b/examples/nft-nd/nft.go @@ -11,6 +11,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/interop" "github.com/nspcc-dev/neo-go/pkg/interop/contract" "github.com/nspcc-dev/neo-go/pkg/interop/iterator" + "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" "github.com/nspcc-dev/neo-go/pkg/interop/native/crypto" "github.com/nspcc-dev/neo-go/pkg/interop/native/gas" "github.com/nspcc-dev/neo-go/pkg/interop/native/management" @@ -35,7 +36,7 @@ var ( // contractOwner is a special address that can perform some management // functions on this contract like updating/destroying it and can also // be used for contract address verification. - contractOwner = util.FromAddress("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB") + contractOwner = address.ToHash160("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB") ) // Symbol returns token symbol, it's HASHY. diff --git a/examples/runtime/runtime.go b/examples/runtime/runtime.go index 9ecdd559d..6c9eb4b6c 100644 --- a/examples/runtime/runtime.go +++ b/examples/runtime/runtime.go @@ -1,14 +1,14 @@ package runtimecontract import ( + "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" "github.com/nspcc-dev/neo-go/pkg/interop/native/management" "github.com/nspcc-dev/neo-go/pkg/interop/runtime" - "github.com/nspcc-dev/neo-go/pkg/interop/util" ) var ( // Check if the invoker of the contract is the specified owner - owner = util.FromAddress("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB") + owner = address.ToHash160("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB") ) // init is transformed into _initialize method that is called whenever contract diff --git a/examples/timer/timer.go b/examples/timer/timer.go index 3e09d39ba..7f7e600ee 100644 --- a/examples/timer/timer.go +++ b/examples/timer/timer.go @@ -3,10 +3,10 @@ package timer import ( "github.com/nspcc-dev/neo-go/pkg/interop" "github.com/nspcc-dev/neo-go/pkg/interop/contract" + "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" "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" ) const defaultTicks = 3 @@ -16,7 +16,7 @@ var ( // ctx holds storage context for contract methods ctx storage.Context // Check if the invoker of the contract is the specified owner - owner = util.FromAddress("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB") + owner = address.ToHash160("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB") // ticksKey is a storage key for ticks counter ticksKey = []byte("ticks") ) diff --git a/examples/token/token.go b/examples/token/token.go index 7cede5d3f..f9d690a16 100644 --- a/examples/token/token.go +++ b/examples/token/token.go @@ -3,8 +3,8 @@ package tokencontract import ( "github.com/nspcc-dev/neo-go/examples/token/nep17" "github.com/nspcc-dev/neo-go/pkg/interop" + "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" "github.com/nspcc-dev/neo-go/pkg/interop/storage" - "github.com/nspcc-dev/neo-go/pkg/interop/util" ) const ( @@ -13,7 +13,7 @@ const ( ) var ( - owner = util.FromAddress("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB") + owner = address.ToHash160("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB") token nep17.Token ctx storage.Context ) diff --git a/internal/basicchain/testdata/test_contract.go b/internal/basicchain/testdata/test_contract.go index e6b98ab57..0b534f9dd 100644 --- a/internal/basicchain/testdata/test_contract.go +++ b/internal/basicchain/testdata/test_contract.go @@ -3,12 +3,12 @@ package testdata import ( "github.com/nspcc-dev/neo-go/pkg/interop" "github.com/nspcc-dev/neo-go/pkg/interop/contract" + "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" "github.com/nspcc-dev/neo-go/pkg/interop/native/ledger" "github.com/nspcc-dev/neo-go/pkg/interop/native/management" "github.com/nspcc-dev/neo-go/pkg/interop/native/neo" "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" ) const ( @@ -16,7 +16,7 @@ const ( decimals = 2 ) -var owner = util.FromAddress("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB") +var owner = address.ToHash160("NbrUYaZgyhSkNoRo9ugRyEMdUZxrhkNaWB") func Init() bool { ctx := storage.GetContext() diff --git a/internal/basicchain/testdata/verify/verification_contract.go b/internal/basicchain/testdata/verify/verification_contract.go index a12bcb186..d03b15615 100644 --- a/internal/basicchain/testdata/verify/verification_contract.go +++ b/internal/basicchain/testdata/verify/verification_contract.go @@ -1,6 +1,7 @@ package verify import ( + "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" "github.com/nspcc-dev/neo-go/pkg/interop/runtime" "github.com/nspcc-dev/neo-go/pkg/interop/util" ) @@ -9,6 +10,6 @@ import ( // It returns true iff it is signed by Nhfg3TbpwogLvDGVvAvqyThbsHgoSUKwtn (id-0 private key from testchain). func Verify() bool { tx := runtime.GetScriptContainer() - addr := util.FromAddress("Nhfg3TbpwogLvDGVvAvqyThbsHgoSUKwtn") + addr := address.ToHash160("Nhfg3TbpwogLvDGVvAvqyThbsHgoSUKwtn") return util.Equals(string(tx.Sender), string(addr)) } diff --git a/pkg/compiler/analysis.go b/pkg/compiler/analysis.go index f725bc771..0dfefd2e6 100644 --- a/pkg/compiler/analysis.go +++ b/pkg/compiler/analysis.go @@ -24,10 +24,6 @@ var ( var ( // Go language builtin functions. goBuiltins = []string{"len", "append", "panic", "make", "copy", "recover", "delete"} - // Custom builtin utility functions. - 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. @@ -642,18 +638,6 @@ func isGoBuiltin(name string) bool { return false } -func isCustomBuiltin(f *funcScope) bool { - if !isInteropPath(f.pkg.Path()) { - return false - } - for _, n := range customBuiltins { - if f.name == n { - return true - } - } - return false -} - func isPotentialCustomBuiltin(f *funcScope, expr ast.Expr) bool { if !isInteropPath(f.pkg.Path()) { return false diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go index 50edd84c9..14793c8ad 100644 --- a/pkg/compiler/codegen.go +++ b/pkg/compiler/codegen.go @@ -458,10 +458,10 @@ func (c *codegen) convertFuncDecl(file ast.Node, decl *ast.FuncDecl, pkg *types. } else { 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 this function is a syscall 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) { + if isSyscall(f) { return f } c.setLabel(f.label) @@ -978,7 +978,7 @@ func (c *codegen) Visit(node ast.Node) ast.Visitor { f, ok = c.funcs[name] if ok { f.selector = fun.X - isBuiltin = isCustomBuiltin(f) || isPotentialCustomBuiltin(f, n) + isBuiltin = isPotentialCustomBuiltin(f, n) if canInline(f.pkg.Path(), f.decl.Name.Name, isBuiltin) { c.inlineCall(f, n) return nil @@ -1926,7 +1926,7 @@ func (c *codegen) convertBuiltin(expr *ast.CallExpr) { c.emitStoreByIndex(varGlobal, c.exceptionIndex) case "delete": emit.Opcodes(c.prog.BinWriter, opcode.REMOVE) - case "FromAddress", "ToHash160": + case "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. @@ -1946,7 +1946,7 @@ 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 and with ToHash160 in case if it behaves like builtin, +// 1. 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; @@ -1954,7 +1954,7 @@ func (c *codegen) convertBuiltin(expr *ast.CallExpr) { 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" || (isBuiltin && f.Sel.Name == "ToHash160") { + if isBuiltin && f.Sel.Name == "ToHash160" { return args[1:] } if fs != nil && isSyscall(fs) { diff --git a/pkg/compiler/function_call_test.go b/pkg/compiler/function_call_test.go index c409108be..f1848be4f 100644 --- a/pkg/compiler/function_call_test.go +++ b/pkg/compiler/function_call_test.go @@ -89,10 +89,10 @@ func TestNotAssignedFunctionCall(t *testing.T) { }) t.Run("Builtin", func(t *testing.T) { src := `package foo - import "github.com/nspcc-dev/neo-go/pkg/interop/util" + import "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" func Main() int { - util.FromAddress("NPAsqZkx9WhNd4P72uhZxBhLinSuNkxfB8") - util.FromAddress("NPAsqZkx9WhNd4P72uhZxBhLinSuNkxfB8") + address.ToHash160("NPAsqZkx9WhNd4P72uhZxBhLinSuNkxfB8") + address.ToHash160("NPAsqZkx9WhNd4P72uhZxBhLinSuNkxfB8") return 1 }` eval(t, src, big.NewInt(1)) diff --git a/pkg/compiler/interop_test.go b/pkg/compiler/interop_test.go index 2ba7a5f2f..a18e97003 100644 --- a/pkg/compiler/interop_test.go +++ b/pkg/compiler/interop_test.go @@ -71,62 +71,13 @@ func TestTypeConstantSize(t *testing.T) { }) } -func TestFromAddress(t *testing.T) { - as1 := "NQRLhCpAru9BjGsMwk67vdMwmzKMRgsnnN" - addr1, err := address.StringToUint160(as1) - require.NoError(t, err) - - as2 := "NPAsqZkx9WhNd4P72uhZxBhLinSuNkxfB8" - addr2, err := address.StringToUint160(as2) - require.NoError(t, err) - - t.Run("append 2 addresses", func(t *testing.T) { - src := ` - package foo - import "github.com/nspcc-dev/neo-go/pkg/interop/util" - func Main() []byte { - addr1 := util.FromAddress("` + as1 + `") - addr2 := util.FromAddress("` + as2 + `") - sum := append(addr1, addr2...) - return sum - } - ` - - eval(t, src, append(addr1.BytesBE(), addr2.BytesBE()...)) - }) - - t.Run("append 2 addresses inline", func(t *testing.T) { - src := ` - package foo - import "github.com/nspcc-dev/neo-go/pkg/interop/util" - func Main() []byte { - addr1 := util.FromAddress("` + as1 + `") - sum := append(addr1, util.FromAddress("` + as2 + `")...) - return sum - } - ` - - eval(t, src, append(addr1.BytesBE(), addr2.BytesBE()...)) - }) - - t.Run("AliasPackage", func(t *testing.T) { - src := ` - package foo - import uu "github.com/nspcc-dev/neo-go/pkg/interop/util" - func Main() []byte { - addr1 := uu.FromAddress("` + as1 + `") - addr2 := uu.FromAddress("` + as2 + `") - sum := append(addr1, addr2...) - return sum - }` - eval(t, src, append(addr1.BytesBE(), addr2.BytesBE()...)) - }) -} - func TestAddressToHash160BuiltinConversion(t *testing.T) { a := "NQRLhCpAru9BjGsMwk67vdMwmzKMRgsnnN" h, err := address.StringToUint160(a) require.NoError(t, err) + a2 := "NPAsqZkx9WhNd4P72uhZxBhLinSuNkxfB8" + addr2, err := address.StringToUint160(a2) + require.NoError(t, err) t.Run("builtin conversion", func(t *testing.T) { src := `package foo import ( @@ -163,6 +114,18 @@ func TestAddressToHash160BuiltinConversion(t *testing.T) { // On the contrary, there should be an address string. require.True(t, strings.Contains(string(prog), a)) }) + t.Run("AliasPackage", func(t *testing.T) { + src := ` + package foo + import ad "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" + func Main() []byte { + addr1 := ad.ToHash160("` + a + `") + addr2 := ad.ToHash160("` + a2 + `") + sum := append(addr1, addr2...) + return sum + }` + eval(t, src, append(h.BytesBE(), addr2.BytesBE()...)) + }) } func TestInvokeAddressToFromHash160(t *testing.T) { diff --git a/pkg/core/blockchain_neotest_test.go b/pkg/core/blockchain_neotest_test.go index f71b8890b..03be30191 100644 --- a/pkg/core/blockchain_neotest_test.go +++ b/pkg/core/blockchain_neotest_test.go @@ -650,10 +650,10 @@ func TestBlockchain_IsTxStillRelevant(t *testing.T) { src := fmt.Sprintf(`package verify import ( "github.com/nspcc-dev/neo-go/pkg/interop/contract" - "github.com/nspcc-dev/neo-go/pkg/interop/util" + "github.com/nspcc-dev/neo-go/pkg/interop/lib/address" ) func Verify() bool { - addr := util.FromAddress("`+address.Uint160ToString(e.NativeHash(t, nativenames.Ledger))+`") + addr := address.ToHash160("`+address.Uint160ToString(e.NativeHash(t, nativenames.Ledger))+`") currentHeight := contract.Call(addr, "currentIndex", contract.ReadStates) return currentHeight.(int) < %d }`, bc.BlockHeight()+2) // deploy + next block diff --git a/pkg/interop/util/util.go b/pkg/interop/util/util.go index 038b17aa9..41ff5ae97 100644 --- a/pkg/interop/util/util.go +++ b/pkg/interop/util/util.go @@ -14,16 +14,6 @@ func Abort() { neogointernal.Opcode0NoReturn("ABORT") } -// FromAddress is an utility function that converts a Neo address to its hash -// (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 -} - // Equals compares a with b and will return true when a and b are equal. It's // implemented as an EQUAL VM opcode, so the rules of comparison are those // of EQUAL. From 6e27883c106678aa37ae1fe2d57e8cae0df5d0a4 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Sat, 18 Mar 2023 10:52:24 +0300 Subject: [PATCH 2/2] rpcsrv: remove deprecated RPC counters --- ROADMAP.md | 9 --------- pkg/services/rpcsrv/prometheus.go | 17 +---------------- 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 669b0b259..dbeabf41d 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -50,15 +50,6 @@ Receive* APIs. Removal of these APIs is scheduled for May 2023 (~0.103.0 release). -## Prometheus RPC counters - -A number of neogo_${method}_called Prometheus counters are marked as -deprecated since version 0.99.5, neogo_rpc_${method}_time histograms can be -used instead (that also have a counter). - -It's not a frequently used thing and it's easy to replace it, so removal of -old counters is scheduled for January-February 2023 (~0.100.X release). - ## SecondsPerBlock protocol configuration With 0.100.0 version SecondsPerBlock protocol configuration setting was diff --git a/pkg/services/rpcsrv/prometheus.go b/pkg/services/rpcsrv/prometheus.go index b90f1ae94..4e24a4e9d 100644 --- a/pkg/services/rpcsrv/prometheus.go +++ b/pkg/services/rpcsrv/prometheus.go @@ -1,7 +1,6 @@ package rpcsrv import ( - "fmt" "strings" "time" @@ -10,8 +9,7 @@ import ( // Metrics used in monitoring service. var ( - rpcCounter = map[string]prometheus.Counter{} - rpcTimes = map[string]prometheus.Histogram{} + rpcTimes = map[string]prometheus.Histogram{} ) func addReqTimeMetric(name string, t time.Duration) { @@ -19,22 +17,9 @@ func addReqTimeMetric(name string, t time.Duration) { if ok { hist.Observe(t.Seconds()) } - ctr, ok := rpcCounter[name] - if ok { - ctr.Inc() - } } func regCounter(call string) { - ctr := prometheus.NewCounter( - prometheus.CounterOpts{ - Help: fmt.Sprintf("Number of calls to %s rpc endpoint (obsolete, to be removed)", call), - Name: fmt.Sprintf("%s_called", call), - Namespace: "neogo", - }, - ) - prometheus.MustRegister(ctr) - rpcCounter[call] = ctr rpcTimes[call] = prometheus.NewHistogram( prometheus.HistogramOpts{ Help: "RPC " + call + " call handling time",