rpcsrv: execute all witnesses for calculatenetworkfee

Try to get as much data as possible, fix #2654.
nspcc-dev · Aug 23, 2022 · 03cc9b2 · 03cc9b2
1 parent a2c4a7f
commit 03cc9b2
Show file tree

Hide file tree

Showing 5 changed files with 225 additions and 18 deletions.
diff --git 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
@@ -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 ef == 0 {
-			ef = s.chain.GetBaseExecFee()
+		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()
 		}
-		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
@@ -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
@@ -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
@@ -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 {