From 03cc9b27623e26900e8bb33a79c440b9ae3f1055 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 22 Aug 2022 18:50:35 +0300 Subject: [PATCH] rpcsrv: execute all witnesses for calculatenetworkfee Try to get as much data as possible, fix #2654. --- docs/rpc.md | 15 ++++ pkg/services/rpcsrv/server.go | 55 ++++++++----- pkg/services/rpcsrv/server_test.go | 117 +++++++++++++++++++++++++++ pkg/smartcontract/param_type.go | 31 +++++++ pkg/smartcontract/param_type_test.go | 25 ++++++ 5 files changed, 225 insertions(+), 18 deletions(-) diff --git a/docs/rpc.md b/docs/rpc.md index a894a93f9..ea19fedda 100644 --- a/docs/rpc.md +++ b/docs/rpc.md @@ -98,6 +98,21 @@ following data types: Any call that takes any of these types for input in JSON format is affected. +##### `calculatenetworkfee` + +NeoGo tries to cover more cases with its calculatenetworkfee implementation, +whereas C# node support only standard signature contracts and deployed +contracts that can execute `verify` successfully on incomplete (not yet signed +properly) transaction, NeoGo also works with deployed contracts that fail at +this stage and executes non-standard contracts (that can fail +too). It's ignoring the result of any verification script (since the method +calculates fee and doesn't care about transaction validity). Invocation script +is used as is when provided, but absent it the system will try to infer one +based on the `verify` method signature (pushing dummy signatures or +hashes). If signature has some types which contents can't be adequately +guessed (arrays, maps, interop, void) they're ignored. See +neo-project/neo#2805 as well. + ##### `invokefunction`, `invokescript` neo-go implementation of `invokefunction` does not return `tx` diff --git a/pkg/services/rpcsrv/server.go b/pkg/services/rpcsrv/server.go index 69befacb8..c984293f9 100644 --- a/pkg/services/rpcsrv/server.go +++ b/pkg/services/rpcsrv/server.go @@ -25,7 +25,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/config/netmode" "github.com/nspcc-dev/neo-go/pkg/core" "github.com/nspcc-dev/neo-go/pkg/core/block" - "github.com/nspcc-dev/neo-go/pkg/core/fee" "github.com/nspcc-dev/neo-go/pkg/core/interop" "github.com/nspcc-dev/neo-go/pkg/core/interop/iterator" "github.com/nspcc-dev/neo-go/pkg/core/mempool" @@ -45,9 +44,12 @@ import ( "github.com/nspcc-dev/neo-go/pkg/network/payload" "github.com/nspcc-dev/neo-go/pkg/services/oracle/broadcaster" "github.com/nspcc-dev/neo-go/pkg/services/rpcsrv/params" + "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" "github.com/nspcc-dev/neo-go/pkg/util" + "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" @@ -758,26 +760,43 @@ func (s *Server) calculateNetworkFee(reqParams params.Params) (interface{}, *neo return 0, neorpc.WrapErrorWithData(neorpc.ErrInvalidParams, fmt.Sprintf("failed to compute tx size: %s", err)) } size := len(hashablePart) + io.GetVarSize(len(tx.Signers)) - var ( - ef int64 - netFee int64 - ) + var netFee int64 for i, signer := range tx.Signers { w := tx.Scripts[i] - if len(w.VerificationScript) == 0 { // then it still might be a contract-based verification - gasConsumed, _ := s.chain.VerifyWitness(signer.Account, tx, &tx.Scripts[i], int64(s.config.MaxGasInvoke)) - netFee += gasConsumed - size += io.GetVarSize([]byte{}) + // verification script is empty (contract-based witness) - io.GetVarSize(tx.Scripts[i].InvocationScript) // invocation script might not be empty (args for `verify`) - continue + if len(w.InvocationScript) == 0 { // No invocation provided, try to infer one. + var paramz []manifest.Parameter + if len(w.VerificationScript) == 0 { // Contract-based verification + cs := s.chain.GetContractState(signer.Account) + if cs == nil { + return 0, neorpc.WrapErrorWithData(neorpc.ErrInvalidParams, fmt.Sprintf("signer %d has no verification script and no deployed contract", i)) + } + md := cs.Manifest.ABI.GetMethod(manifest.MethodVerify, -1) + if md == nil || md.ReturnType != smartcontract.BoolType { + return 0, neorpc.WrapErrorWithData(neorpc.ErrInvalidParams, fmt.Sprintf("signer %d has no verify method in deployed contract", i)) + } + paramz = md.Parameters // Might as well have none params and it's OK. + } else { // Regular signature verification. + if vm.IsSignatureContract(w.VerificationScript) { + paramz = []manifest.Parameter{{Type: smartcontract.SignatureType}} + } else if nSigs, _, ok := vm.ParseMultiSigContract(w.VerificationScript); ok { + paramz = make([]manifest.Parameter, nSigs) + for j := 0; j < nSigs; j++ { + paramz[j] = manifest.Parameter{Type: smartcontract.SignatureType} + } + } + } + inv := io.NewBufBinWriter() + for _, p := range paramz { + p.Type.EncodeDefaultValue(inv.BinWriter) + } + if inv.Err != nil { + return nil, neorpc.NewInternalServerError(fmt.Sprintf("failed to create dummy invocation script (signer %d): %s", i, inv.Err.Error())) + } + w.InvocationScript = inv.Bytes() } - - if ef == 0 { - ef = s.chain.GetBaseExecFee() - } - fee, sizeDelta := fee.Calculate(ef, w.VerificationScript) - netFee += fee - size += sizeDelta + gasConsumed, _ := s.chain.VerifyWitness(signer.Account, tx, &w, int64(s.config.MaxGasInvoke)) + netFee += gasConsumed + size += io.GetVarSize(w.VerificationScript) + io.GetVarSize(w.InvocationScript) } if s.chain.P2PSigExtensionsEnabled() { attrs := tx.GetAttributes(transaction.NotaryAssistedT) diff --git a/pkg/services/rpcsrv/server_test.go b/pkg/services/rpcsrv/server_test.go index d524e1d5b..d5a9fce4a 100644 --- a/pkg/services/rpcsrv/server_test.go +++ b/pkg/services/rpcsrv/server_test.go @@ -39,6 +39,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/network/payload" rpc2 "github.com/nspcc-dev/neo-go/pkg/services/oracle/broadcaster" "github.com/nspcc-dev/neo-go/pkg/services/rpcsrv/params" + "github.com/nspcc-dev/neo-go/pkg/smartcontract" "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" @@ -2453,6 +2454,122 @@ func testRPCProtocol(t *testing.T, doRPCCall func(string, string, *testing.T) [] }, 2*time.Duration(rpcSrv.config.SessionExpirationTime)*time.Second, 10*time.Millisecond) }) }) + t.Run("calculatenetworkfee", func(t *testing.T) { + t.Run("no parameters", func(t *testing.T) { + body := doRPCCall(`{"jsonrpc": "2.0", "id": 1, "method": "calculatenetworkfee", "params": []}"`, httpSrv.URL, t) + _ = checkErrGetResult(t, body, true, "Invalid Params") + }) + t.Run("non-base64 parameter", func(t *testing.T) { + body := doRPCCall(`{"jsonrpc": "2.0", "id": 1, "method": "calculatenetworkfee", "params": ["noatbase64"]}"`, httpSrv.URL, t) + _ = checkErrGetResult(t, body, true, "Invalid Params") + }) + t.Run("non-transaction parameter", func(t *testing.T) { + body := doRPCCall(`{"jsonrpc": "2.0", "id": 1, "method": "calculatenetworkfee", "params": ["bm90IGEgdHJhbnNhY3Rpb24K"]}"`, httpSrv.URL, t) + _ = checkErrGetResult(t, body, true, "Invalid Params") + }) + calcReq := func(t *testing.T, tx *transaction.Transaction) []byte { + rpc := fmt.Sprintf(`{"jsonrpc": "2.0", "id": 1, "method": "calculatenetworkfee", "params": ["%s"]}"`, base64.StdEncoding.EncodeToString(tx.Bytes())) + return doRPCCall(rpc, httpSrv.URL, t) + } + t.Run("non-contract with zero verification", func(t *testing.T) { + tx := &transaction.Transaction{ + Script: []byte{byte(opcode.RET)}, + Signers: []transaction.Signer{{Account: util.Uint160{1, 2, 3}, Scopes: transaction.CalledByEntry}}, + Scripts: []transaction.Witness{{ + InvocationScript: []byte{}, + VerificationScript: []byte{}, + }}, + } + body := calcReq(t, tx) + _ = checkErrGetResult(t, body, true, "signer 0 has no verification script and no deployed contract") + }) + t.Run("contract with no verify", func(t *testing.T) { + tx := &transaction.Transaction{ + Script: []byte{byte(opcode.RET)}, + Signers: []transaction.Signer{{Account: nnsHash, Scopes: transaction.CalledByEntry}}, + Scripts: []transaction.Witness{{ + InvocationScript: []byte{}, + VerificationScript: []byte{}, + }}, + } + body := calcReq(t, tx) + _ = checkErrGetResult(t, body, true, "signer 0 has no verify method in deployed contract") + }) + checkCalc := func(t *testing.T, tx *transaction.Transaction, fee int64) { + resp := checkErrGetResult(t, calcReq(t, tx), false) + res := new(result.NetworkFee) + require.NoError(t, json.Unmarshal(resp, res)) + require.Equal(t, fee, res.Value) + } + t.Run("simple GAS transfer", func(t *testing.T) { + priv0 := testchain.PrivateKeyByID(0) + script, err := smartcontract.CreateCallWithAssertScript(chain.UtilityTokenHash(), "transfer", + priv0.GetScriptHash(), priv0.GetScriptHash(), 1, nil) + require.NoError(t, err) + tx := &transaction.Transaction{ + Script: script, + Signers: []transaction.Signer{{Account: priv0.GetScriptHash(), Scopes: transaction.CalledByEntry}}, + Scripts: []transaction.Witness{{ + InvocationScript: []byte{}, + VerificationScript: priv0.PublicKey().GetVerificationScript(), + }}, + } + checkCalc(t, tx, 1228520) // Perfectly matches FeeIsSignatureContractDetailed() C# test. + }) + t.Run("multisignature tx", func(t *testing.T) { + priv0 := testchain.PrivateKeyByID(0) + priv1 := testchain.PrivateKeyByID(1) + accScript, err := smartcontract.CreateDefaultMultiSigRedeemScript(keys.PublicKeys{priv0.PublicKey(), priv1.PublicKey()}) + require.NoError(t, err) + multiAcc := hash.Hash160(accScript) + txScript, err := smartcontract.CreateCallWithAssertScript(chain.UtilityTokenHash(), "transfer", + multiAcc, priv0.GetScriptHash(), 1, nil) + require.NoError(t, err) + tx := &transaction.Transaction{ + Script: txScript, + Signers: []transaction.Signer{{Account: multiAcc, Scopes: transaction.CalledByEntry}}, + Scripts: []transaction.Witness{{ + InvocationScript: []byte{}, + VerificationScript: accScript, + }}, + } + checkCalc(t, tx, 2315100) // Perfectly matches FeeIsMultiSigContract() C# test. + }) + checkContract := func(t *testing.T, verAcc util.Uint160, invoc []byte, fee int64) { + txScript, err := smartcontract.CreateCallWithAssertScript(chain.UtilityTokenHash(), "transfer", + verAcc, verAcc, 1, nil) + require.NoError(t, err) + tx := &transaction.Transaction{ + Script: txScript, + Signers: []transaction.Signer{{Account: verAcc, Scopes: transaction.CalledByEntry}}, + Scripts: []transaction.Witness{{ + InvocationScript: invoc, + VerificationScript: []byte{}, + }}, + } + checkCalc(t, tx, fee) + } + t.Run("contract-based verification", func(t *testing.T) { + verAcc, err := util.Uint160DecodeStringLE(verifyContractHash) + require.NoError(t, err) + checkContract(t, verAcc, []byte{}, 636610) // No C# match, but we believe it's OK. + }) + t.Run("contract-based verification with parameters", func(t *testing.T) { + verAcc, err := util.Uint160DecodeStringLE(verifyWithArgsContractHash) + require.NoError(t, err) + checkContract(t, verAcc, []byte{}, 737530) // No C# match, but we believe it's OK and it differs from the one above. + }) + t.Run("contract-based verification with invocation script", func(t *testing.T) { + verAcc, err := util.Uint160DecodeStringLE(verifyWithArgsContractHash) + require.NoError(t, err) + invocWriter := io.NewBufBinWriter() + emit.Bool(invocWriter.BinWriter, false) + emit.Int(invocWriter.BinWriter, 5) + emit.String(invocWriter.BinWriter, "") + invocScript := invocWriter.Bytes() + checkContract(t, verAcc, invocScript, 640360) // No C# match, but we believe it's OK and it has a specific invocation script overriding anything server-side. + }) + }) } func (e *executor) getHeader(s string) *block.Header { diff --git a/pkg/smartcontract/param_type.go b/pkg/smartcontract/param_type.go index 8db285e26..0160e357c 100644 --- a/pkg/smartcontract/param_type.go +++ b/pkg/smartcontract/param_type.go @@ -12,6 +12,8 @@ import ( "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/io" "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" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" ) @@ -139,6 +141,35 @@ func (pt *ParamType) DecodeBinary(r *io.BinReader) { *pt = ParamType(r.ReadB()) } +// EncodeDefaultValue writes a script to push the default parameter value onto +// the evaluation stack into the given writer. It's mostly useful for constructing +// dummy invocation scripts when parameter types are known, but they can't be +// filled in. A best effort approach is used, it can't be perfect since for many +// types the exact values can be arbitrarily long, but it tries to do something +// reasonable in each case. For signatures, strings, arrays and "any" type a 64-byte +// zero-filled value is used, hash160 and hash256 use appropriately sized values, +// public key is represented by 33-byte value while 32 bytes are used for integer +// and a simple push+convert is used for boolean. Other types produce no code at all. +func (pt ParamType) EncodeDefaultValue(w *io.BinWriter) { + var b [64]byte + + switch pt { + case AnyType, SignatureType, StringType, ByteArrayType: + emit.Bytes(w, b[:]) + case BoolType: + emit.Bool(w, true) + case IntegerType: + emit.Instruction(w, opcode.PUSHINT256, b[:32]) + case Hash160Type: + emit.Bytes(w, b[:20]) + case Hash256Type: + emit.Bytes(w, b[:32]) + case PublicKeyType: + emit.Bytes(w, b[:33]) + case ArrayType, MapType, InteropInterfaceType, VoidType: + } +} + // ParseParamType is a user-friendly string to ParamType converter, it's // case-insensitive and makes the following conversions: // diff --git a/pkg/smartcontract/param_type_test.go b/pkg/smartcontract/param_type_test.go index 0620e52e1..181c584f8 100644 --- a/pkg/smartcontract/param_type_test.go +++ b/pkg/smartcontract/param_type_test.go @@ -5,6 +5,7 @@ import ( "math/big" "testing" + "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" "github.com/stretchr/testify/assert" @@ -335,6 +336,30 @@ func TestAdjustValToType(t *testing.T) { } } +func TestEncodeDefaultValue(t *testing.T) { + for p, l := range map[ParamType]int{ + UnknownType: 0, + AnyType: 66, + BoolType: 3, + IntegerType: 33, + ByteArrayType: 66, + StringType: 66, + Hash160Type: 22, + Hash256Type: 34, + PublicKeyType: 35, + SignatureType: 66, + ArrayType: 0, + MapType: 0, + InteropInterfaceType: 0, + VoidType: 0, + } { + b := io.NewBufBinWriter() + p.EncodeDefaultValue(b.BinWriter) + require.NoError(t, b.Err) + require.Equalf(t, l, len(b.Bytes()), p.String()) + } +} + func mustHex(s string) []byte { b, err := hex.DecodeString(s) if err != nil {