Merge pull request #1825 from nspcc-dev/fix-invokecontractverify

rpc: refactor invokecontractverify

internal/fakechain/fakechain.go

@@ -80,6 +80,11 @@ func (chain *FakeChain) IsTxStillRelevant(t *transaction.Transaction, txpool *me
 	panic("TODO")
 }
 
+// InitVerificationVM initializes VM for witness check.
+func (chain *FakeChain) InitVerificationVM(v *vm.VM, getContract func(util.Uint160) (*state.Contract, error), hash util.Uint160, witness *transaction.Witness) error {
+	panic("TODO")
+}
+
 // IsExtensibleAllowed implements Blockchainer interface.
 func (*FakeChain) IsExtensibleAllowed(uint160 util.Uint160) bool {
 	return true

pkg/core/blockchain.go

@@ -1698,9 +1698,8 @@ var (
 	ErrInvalidVerificationContract = errors.New("verification contract is missing `verify` method")
 )
 
-// initVerificationVM initializes VM for witness check.
-func (bc *Blockchain) initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transaction.Witness) error {
-	v := ic.VM
+// InitVerificationVM initializes VM for witness check.
+func (bc *Blockchain) InitVerificationVM(v *vm.VM, getContract func(util.Uint160) (*state.Contract, error), hash util.Uint160, witness *transaction.Witness) error {
 	if len(witness.VerificationScript) != 0 {
 		if witness.ScriptHash() != hash {
 			return ErrWitnessHashMismatch
@@ -1714,7 +1713,7 @@ func (bc *Blockchain) initVerificationVM(ic *interop.Context, hash util.Uint160,
 		}
 		v.LoadScriptWithFlags(witness.VerificationScript, callflag.ReadOnly)
 	} else {
-		cs, err := ic.GetContract(hash)
+		cs, err := getContract(hash)
 		if err != nil {
 			return ErrUnknownVerificationContract
 		}
@@ -1760,7 +1759,7 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa
 	vm.SetPriceGetter(interopCtx.GetPrice)
 	vm.LoadToken = contract.LoadToken(interopCtx)
 	vm.GasLimit = gas
-	if err := bc.initVerificationVM(interopCtx, hash, witness); err != nil {
+	if err := bc.InitVerificationVM(vm, interopCtx.GetContract, hash, witness); err != nil {
 		return 0, err
 	}
 	err := vm.Run()

pkg/core/blockchainer/blockchainer.go

@@ -25,6 +25,7 @@ type Blockchainer interface {
 	AddBlock(*block.Block) error
 	CalculateClaimable(h util.Uint160, endHeight uint32) (*big.Int, error)
 	Close()
+	InitVerificationVM(v *vm.VM, getContract func(util.Uint160) (*state.Contract, error), hash util.Uint160, witness *transaction.Witness) error
 	IsTxStillRelevant(t *transaction.Transaction, txpool *mempool.Pool, isPartialTx bool) bool
 	HeaderHeight() uint32
 	GetBlock(hash util.Uint256) (*block.Block, error)

pkg/core/helper_test.go

@@ -451,6 +451,15 @@ func initBasicChain(t *testing.T, bc *Blockchain) {
 	bc.setNodesByRole(t, true, native.RoleP2PNotary, keys.PublicKeys{ntr.Accounts[0].PrivateKey().PublicKey()})
 	t.Logf("Designated Notary node: %s", hex.EncodeToString(ntr.Accounts[0].PrivateKey().PublicKey().Bytes()))
 
+	// Push verification contract with arguments into the chain.
+	txDeploy3, _ := newDeployTx(t, bc, priv0ScriptHash, prefix+"verification_with_args_contract.go", "VerifyWithArgs")
+	txDeploy3.Nonce = getNextNonce()
+	txDeploy3.ValidUntilBlock = validUntilBlock
+	require.NoError(t, addNetworkFee(bc, txDeploy3, acc0))
+	require.NoError(t, acc0.SignTx(txDeploy3))
+	b = bc.newBlock(txDeploy3)
+	require.NoError(t, bc.AddBlock(b))
+
 	// Compile contract to test `invokescript` RPC call
 	_, _ = newDeployTx(t, bc, priv0ScriptHash, prefix+"invokescript_contract.go", "ContractForInvokescriptTest")
 }

pkg/rpc/request/txBuilder.go

@@ -14,9 +14,9 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/vm/opcode"
 )
 
-// expandArrayIntoScript pushes all FuncParam parameters from the given array
+// ExpandArrayIntoScript pushes all FuncParam parameters from the given array
 // into the given buffer in reverse order.
-func expandArrayIntoScript(script *io.BinWriter, slice []Param) error {
+func ExpandArrayIntoScript(script *io.BinWriter, slice []Param) error {
 	for j := len(slice) - 1; j >= 0; j-- {
 		fp, err := slice[j].GetFuncParam()
 		if err != nil {
@@ -87,7 +87,7 @@ func expandArrayIntoScript(script *io.BinWriter, slice []Param) error {
 			if err != nil {
 				return err
 			}
-			err = expandArrayIntoScript(script, val)
+			err = ExpandArrayIntoScript(script, val)
 			if err != nil {
 				return err
 			}
@@ -119,7 +119,7 @@ func CreateFunctionInvocationScript(contract util.Uint160, method string, params
 			if err != nil {
 				return nil, err
 			}
-			err = expandArrayIntoScript(script.BinWriter, slice)
+			err = ExpandArrayIntoScript(script.BinWriter, slice)
 			if err != nil {
 				return nil, err
 			}

pkg/rpc/request/tx_builder_test.go

@@ -105,7 +105,7 @@ func TestExpandArrayIntoScript(t *testing.T) {
 	}
 	for _, c := range testCases {
 		script := io.NewBufBinWriter()
-		err := expandArrayIntoScript(script.BinWriter, c.Input)
+		err := ExpandArrayIntoScript(script.BinWriter, c.Input)
 		require.NoError(t, err)
 		require.Equal(t, c.Expected, script.Bytes())
 	}
@@ -119,7 +119,7 @@ func TestExpandArrayIntoScript(t *testing.T) {
 	}
 	for _, c := range errorCases {
 		script := io.NewBufBinWriter()
-		err := expandArrayIntoScript(script.BinWriter, c)
+		err := ExpandArrayIntoScript(script.BinWriter, c)
 		require.Error(t, err)
 	}
 }

pkg/rpc/server/client_test.go

@@ -71,6 +71,7 @@ func TestAddNetworkFee(t *testing.T) {
 	chain, rpcSrv, httpSrv := initServerWithInMemoryChain(t)
 	defer chain.Close()
 	defer rpcSrv.Shutdown()
+	const extraFee = 10
 
 	c, err := client.New(context.Background(), httpSrv.URL, client.Options{})
 	require.NoError(t, err)
@@ -95,7 +96,7 @@ func TestAddNetworkFee(t *testing.T) {
 			Account: accs[0].PrivateKey().GetScriptHash(),
 			Scopes:  transaction.CalledByEntry,
 		}}
-		require.Error(t, c.AddNetworkFee(tx, 10, accs[0], accs[1]))
+		require.Error(t, c.AddNetworkFee(tx, extraFee, accs[0], accs[1]))
 	})
 	t.Run("Simple", func(t *testing.T) {
 		tx := transaction.New(testchain.Network(), []byte{byte(opcode.PUSH1)}, 0)
@@ -107,7 +108,7 @@ func TestAddNetworkFee(t *testing.T) {
 		require.NoError(t, c.AddNetworkFee(tx, 10, accs[0]))
 		require.NoError(t, accs[0].SignTx(tx))
 		cFee, _ := fee.Calculate(chain.GetBaseExecFee(), accs[0].Contract.Script)
-		require.Equal(t, int64(io.GetVarSize(tx))*feePerByte+cFee+10, tx.NetworkFee)
+		require.Equal(t, int64(io.GetVarSize(tx))*feePerByte+cFee+extraFee, tx.NetworkFee)
 	})
 
 	t.Run("Multi", func(t *testing.T) {
@@ -126,43 +127,69 @@ func TestAddNetworkFee(t *testing.T) {
 				Scopes:  transaction.Global,
 			},
 		}
-		require.NoError(t, c.AddNetworkFee(tx, 10, accs[0], accs[1]))
+		require.NoError(t, c.AddNetworkFee(tx, extraFee, accs[0], accs[1]))
 		require.NoError(t, accs[0].SignTx(tx))
 		require.NoError(t, accs[1].SignTx(tx))
 		require.NoError(t, accs[2].SignTx(tx))
 		cFee, _ := fee.Calculate(chain.GetBaseExecFee(), accs[0].Contract.Script)
 		cFeeM, _ := fee.Calculate(chain.GetBaseExecFee(), accs[1].Contract.Script)
-		require.Equal(t, int64(io.GetVarSize(tx))*feePerByte+cFee+cFeeM+10, tx.NetworkFee)
+		require.Equal(t, int64(io.GetVarSize(tx))*feePerByte+cFee+cFeeM+extraFee, tx.NetworkFee)
 	})
 	t.Run("Contract", func(t *testing.T) {
-		tx := transaction.New(testchain.Network(), []byte{byte(opcode.PUSH1)}, 0)
+		h, err := util.Uint160DecodeStringLE(verifyContractHash)
+		require.NoError(t, err)
 		priv := testchain.PrivateKeyByID(0)
-		acc1 := wallet.NewAccountFromPrivateKey(priv)
+		acc0 := wallet.NewAccountFromPrivateKey(priv)
+		acc1 := wallet.NewAccountFromPrivateKey(priv) // contract account
 		acc1.Contract.Deployed = true
 		acc1.Contract.Script, err = base64.StdEncoding.DecodeString(verifyContractAVM)
-		require.NoError(t, err)
-		h, err := util.Uint160DecodeStringLE(verifyContractHash)
-		require.NoError(t, err)
-		tx.ValidUntilBlock = chain.BlockHeight() + 10
+
+		newTx := func(t *testing.T) *transaction.Transaction {
+			tx := transaction.New(testchain.Network(), []byte{byte(opcode.PUSH1)}, 0)
+			require.NoError(t, err)
+			tx.ValidUntilBlock = chain.BlockHeight() + 10
+			return tx
+		}
 
 		t.Run("Valid", func(t *testing.T) {
-			acc0 := wallet.NewAccountFromPrivateKey(priv)
-			tx.Signers = []transaction.Signer{
-				{
-					Account: acc0.PrivateKey().GetScriptHash(),
-					Scopes:  transaction.CalledByEntry,
-				},
-				{
-					Account: h,
-					Scopes:  transaction.Global,
-				},
+			completeTx := func(t *testing.T) *transaction.Transaction {
+				tx := newTx(t)
+				tx.Signers = []transaction.Signer{
+					{
+						Account: acc0.PrivateKey().GetScriptHash(),
+						Scopes:  transaction.CalledByEntry,
+					},
+					{
+						Account: h,
+						Scopes:  transaction.Global,
+					},
+				}
+				require.NoError(t, c.AddNetworkFee(tx, extraFee, acc0, acc1))
+				return tx
 			}
-			require.NoError(t, c.AddNetworkFee(tx, 10, acc0, acc1))
-			require.NoError(t, acc0.SignTx(tx))
-			tx.Scripts = append(tx.Scripts, transaction.Witness{})
-			require.NoError(t, chain.VerifyTx(tx))
+
+			// check that network fee with extra value is enough
+			tx1 := completeTx(t)
+			require.NoError(t, acc0.SignTx(tx1))
+			tx1.Scripts = append(tx1.Scripts, transaction.Witness{})
+			require.NoError(t, chain.VerifyTx(tx1))
+
+			// check that network fee without extra value is enough
+			tx2 := completeTx(t)
+			tx2.NetworkFee -= extraFee
+			require.NoError(t, acc0.SignTx(tx2))
+			tx2.Scripts = append(tx2.Scripts, transaction.Witness{})
+			require.NoError(t, chain.VerifyTx(tx2))
+
+			// check that we don't add unexpected extra GAS
+			tx3 := completeTx(t)
+			tx3.NetworkFee -= extraFee + 1
+			require.NoError(t, acc0.SignTx(tx3))
+			tx3.Scripts = append(tx3.Scripts, transaction.Witness{})
+			require.Error(t, chain.VerifyTx(tx3))
 		})
 		t.Run("Invalid", func(t *testing.T) {
+			tx := newTx(t)
 			acc0, err := wallet.NewAccount()
 			require.NoError(t, err)
 			tx.Signers = []transaction.Signer{
@@ -178,6 +205,7 @@ func TestAddNetworkFee(t *testing.T) {
 			require.Error(t, c.AddNetworkFee(tx, 10, acc0, acc1))
 		})
 		t.Run("InvalidContract", func(t *testing.T) {
+			tx := newTx(t)
 			acc0 := wallet.NewAccountFromPrivateKey(priv)
 			tx.Signers = []transaction.Signer{
 				{

pkg/rpc/server/server.go

@@ -36,9 +36,9 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/services/oracle"
 	"github.com/nspcc-dev/neo-go/pkg/services/oracle/broadcaster"
 	"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/opcode"
 	"go.uber.org/zap"
 )
 
@@ -1118,7 +1118,7 @@ func (s *Server) invokeFunction(reqParams request.Params) (interface{}, *respons
 		return nil, response.NewInternalServerError("can't create invocation script", err)
 	}
 	tx.Script = script
-	return s.runScriptInVM(trigger.Application, script, tx)
+	return s.runScriptInVM(trigger.Application, script, util.Uint160{}, tx)
 }
 
 // invokescript implements the `invokescript` RPC call.
@@ -1144,7 +1144,7 @@ func (s *Server) invokescript(reqParams request.Params) (interface{}, *response.
 		tx.Signers = []transaction.Signer{{Account: util.Uint160{}, Scopes: transaction.None}}
 	}
 	tx.Script = script
-	return s.runScriptInVM(trigger.Application, script, tx)
+	return s.runScriptInVM(trigger.Application, script, util.Uint160{}, tx)
 }
 
 // invokeContractVerify implements the `invokecontractverify` RPC call.
@@ -1154,36 +1154,43 @@ func (s *Server) invokeContractVerify(reqParams request.Params) (interface{}, *r
 		return nil, responseErr
 	}
 
-	args := reqParams[1:2]
-	var tx *transaction.Transaction
+	bw := io.NewBufBinWriter()
+	if len(reqParams) > 1 {
+		args, err := reqParams[1].GetArray() // second `invokecontractverify` parameter is an array of arguments for `verify` method
+		if err != nil {
+			return nil, response.WrapErrorWithData(response.ErrInvalidParams, err)
+		}
+		if len(args) > 0 {
+			err := request.ExpandArrayIntoScript(bw.BinWriter, args)
+			if err != nil {
+				return nil, response.NewRPCError("can't create witness invocation script", err.Error(), err)
+			}
+		}
+	}
+	invocationScript := bw.Bytes()
+
+	tx := &transaction.Transaction{Script: []byte{byte(opcode.RET)}} // need something in script
 	if len(reqParams) > 2 {
 		signers, witnesses, err := reqParams[2].GetSignersWithWitnesses()
 		if err != nil {
 			return nil, response.ErrInvalidParams
 		}
-		tx = &transaction.Transaction{
-			Signers: signers,
-			Scripts: witnesses,
-		}
+		tx.Signers = signers
+		tx.Scripts = witnesses
+	} else { // fill the only known signer - the contract with `verify` method
+		tx.Signers = []transaction.Signer{{Account: scriptHash}}
+		tx.Scripts = []transaction.Witness{{InvocationScript: invocationScript, VerificationScript: []byte{}}}
 	}
 
-	cs := s.chain.GetContractState(scriptHash)
-	if cs == nil {
-		return nil, response.NewRPCError("unknown contract", scriptHash.StringBE(), nil)
-	}
-	script, err := request.CreateFunctionInvocationScript(cs.Hash, manifest.MethodVerify, args)
-	if err != nil {
-		return nil, response.NewInternalServerError("can't create invocation script", err)
-	}
-	if tx != nil {
-		tx.Script = script
-	}
-	return s.runScriptInVM(trigger.Verification, script, tx)
+	return s.runScriptInVM(trigger.Verification, invocationScript, scriptHash, tx)
 }
 
 // runScriptInVM runs given script in a new test VM and returns the invocation
-// result.
-func (s *Server) runScriptInVM(t trigger.Type, script []byte, tx *transaction.Transaction) (*result.Invoke, *response.Error) {
+// result. The script is either a simple script in case of `application` trigger
+// witness invocation script in case of `verification` trigger (it pushes `verify`
+// arguments on stack before verification). In case of contract verification
+// contractScriptHash should be specified.
+func (s *Server) runScriptInVM(t trigger.Type, script []byte, contractScriptHash util.Uint160, tx *transaction.Transaction) (*result.Invoke, *response.Error) {
 	// When transferring funds, script execution does no auto GAS claim,
 	// because it depends on persisting tx height.
 	// This is why we provide block here.
@@ -1197,7 +1204,27 @@ func (s *Server) runScriptInVM(t trigger.Type, script []byte, tx *transaction.Tr
 
 	vm := s.chain.GetTestVM(t, tx, b)
 	vm.GasLimit = int64(s.config.MaxGasInvoke)
-	vm.LoadScriptWithFlags(script, callflag.All)
+	if t == trigger.Verification {
+		// We need this special case because witnesses verification is not the simple System.Contract.Call,
+		// and we need to define exactly the amount of gas consumed for a contract witness verification.
+		gasPolicy := s.chain.GetPolicer().GetMaxVerificationGAS()
+		if vm.GasLimit > gasPolicy {
+			vm.GasLimit = gasPolicy
+		}
+
+		err := s.chain.InitVerificationVM(vm, func(h util.Uint160) (*state.Contract, error) {
+			res := s.chain.GetContractState(h)
+			if res == nil {
+				return nil, fmt.Errorf("unknown contract: %s", h.StringBE())
+			}
+			return res, nil
+		}, contractScriptHash, &transaction.Witness{InvocationScript: script, VerificationScript: []byte{}})
+		if err != nil {
+			return nil, response.NewInternalServerError("can't prepare verification VM", err)
+		}
+	} else {
+		vm.LoadScriptWithFlags(script, callflag.All)
+	}
 	err = vm.Run()
 	var faultException string
 	if err != nil {