Merge pull request #1912 from nspcc-dev/cli/invoke_with_contract_signer

rpc: properly construct cosigners' witnesses in SignAndPushInvocationTx

cli/contract_test.go

@@ -314,6 +314,25 @@ func TestComlileAndInvokeFunction(t *testing.T) {
 	require.Len(t, res.Stack, 1)
 	require.Equal(t, []byte("on create|sub create"), res.Stack[0].Value())
 
+	// deploy verification contract
+	nefName = path.Join(tmpDir, "verify.nef")
+	manifestName = path.Join(tmpDir, "verify.manifest.json")
+	e.Run(t, "neo-go", "contract", "compile",
+		"--in", "testdata/verify.go",
+		"--config", "testdata/verify.yml",
+		"--out", nefName, "--manifest", manifestName)
+	e.In.WriteString("one\r")
+	e.Run(t, "neo-go", "contract", "deploy",
+		"--rpc-endpoint", "http://"+e.RPC.Addr,
+		"--wallet", validatorWallet, "--address", validatorAddr,
+		"--in", nefName, "--manifest", manifestName)
+	line, err = e.Out.ReadString('\n')
+	require.NoError(t, err)
+	line = strings.TrimSpace(strings.TrimPrefix(line, "Contract: "))
+	hVerify, err := util.Uint160DecodeStringLE(line)
+	require.NoError(t, err)
+	e.checkTxPersisted(t)
+
 	t.Run("real invoke", func(t *testing.T) {
 		cmd := []string{"neo-go", "contract", "invokefunction",
 			"--rpc-endpoint", "http://" + e.RPC.Addr}
@@ -338,31 +357,17 @@ func TestComlileAndInvokeFunction(t *testing.T) {
 			e.In.WriteString("one\r")
 			e.Run(t, append(cmd, "--force", h.StringLE(), "fail")...)
 		})
+
+		t.Run("cosigner is deployed contract", func(t *testing.T) {
+			e.In.WriteString("one\r")
+			e.Run(t, append(cmd, h.StringLE(), "getValue",
+				"--", validatorAddr, hVerify.StringLE())...)
+		})
 	})
 
 	t.Run("real invoke and save tx", func(t *testing.T) {
 		txout := path.Join(tmpDir, "test_contract_tx.json")
 
-		nefName = path.Join(tmpDir, "verify.nef")
-		manifestName = path.Join(tmpDir, "verify.manifest.json")
-		e.Run(t, "neo-go", "contract", "compile",
-			"--in", "testdata/verify.go",
-			"--config", "testdata/verify.yml",
-			"--out", nefName, "--manifest", manifestName)
-
-		e.In.WriteString("one\r")
-		e.Run(t, "neo-go", "contract", "deploy",
-			"--rpc-endpoint", "http://"+e.RPC.Addr,
-			"--wallet", validatorWallet, "--address", validatorAddr,
-			"--in", nefName, "--manifest", manifestName)
-
-		line, err := e.Out.ReadString('\n')
-		require.NoError(t, err)
-		line = strings.TrimSpace(strings.TrimPrefix(line, "Contract: "))
-		hVerify, err := util.Uint160DecodeStringLE(line)
-		require.NoError(t, err)
-		e.checkTxPersisted(t)
-
 		cmd = []string{"neo-go", "contract", "invokefunction",
 			"--rpc-endpoint", "http://" + e.RPC.Addr,
 			"--out", txout,

pkg/rpc/client/rpc.go

@@ -530,8 +530,10 @@ func (c *Client) SubmitRawOracleResponse(ps request.RawParams) error {
 }
 
 // SignAndPushInvocationTx signs and pushes given script as an invocation
-// transaction  using given wif to sign it and spending the amount of gas
+// transaction using given wif to sign it and given cosigners to cosign it if
-// specified. It returns a hash of the invocation transaction and an error.
+// possible. It spends the amount of gas specified. It returns a hash of the
+// invocation transaction and an error. If one of the cosigners accounts is
+// neither contract-based nor unlocked an error is returned.
 func (c *Client) SignAndPushInvocationTx(script []byte, acc *wallet.Account, sysfee int64, netfee fixedn.Fixed8, cosigners []SignerAccount) (util.Uint256, error) {
 	var txHash util.Uint256
 	var err error
@@ -543,6 +545,28 @@ func (c *Client) SignAndPushInvocationTx(script []byte, acc *wallet.Account, sys
 	if err = acc.SignTx(c.GetNetwork(), tx); err != nil {
 		return txHash, fmt.Errorf("failed to sign tx: %w", err)
 	}
+	// try to add witnesses for the rest of the signers
+	for i, signer := range tx.Signers[1:] {
+		var isOk bool
+		for _, cosigner := range cosigners {
+			if signer.Account == cosigner.Signer.Account {
+				err = cosigner.Account.SignTx(c.GetNetwork(), tx)
+				if err != nil { // then account is non-contract-based and locked, but let's provide more detailed error
+					if paramNum := len(cosigner.Account.Contract.Parameters); paramNum != 0 && cosigner.Account.Contract.Deployed {
+						return txHash, fmt.Errorf("failed to add contract-based witness for signer #%d (%s): "+
+							"%d parameters must be provided to construct invocation script", i, address.Uint160ToString(signer.Account), paramNum)
+					}
+					return txHash, fmt.Errorf("failed to add witness for signer #%d (%s): account should be unlocked to add the signature. "+
+						"Store partially-signed transaction and then use 'wallet sign' command to cosign it", i, address.Uint160ToString(signer.Account))
+				}
+				isOk = true
+				break
+			}
+		}
+		if !isOk {
+			return txHash, fmt.Errorf("failed to add witness for signer #%d (%s): account wasn't provided", i, address.Uint160ToString(signer.Account))
+		}
+	}
 	txHash = tx.Hash()
 	actualHash, err := c.SendRawTransaction(tx)
 	if err != nil {

pkg/rpc/server/client_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/core/transaction"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/hash"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
+	"github.com/nspcc-dev/neo-go/pkg/encoding/address"
 	"github.com/nspcc-dev/neo-go/pkg/io"
 	"github.com/nspcc-dev/neo-go/pkg/rpc/client"
 	"github.com/nspcc-dev/neo-go/pkg/smartcontract"
@@ -396,24 +397,152 @@ func TestSignAndPushInvocationTx(t *testing.T) {
 	require.NoError(t, err)
 	require.NoError(t, c.Init())
 
-	priv := testchain.PrivateKey(0)
+	priv0 := testchain.PrivateKeyByID(0)
-	acc := wallet.NewAccountFromPrivateKey(priv)
+	acc0 := wallet.NewAccountFromPrivateKey(priv0)
-	h, err := c.SignAndPushInvocationTx([]byte{byte(opcode.PUSH1)}, acc, 30, 0, []client.SignerAccount{
-		{
-			Signer: transaction.Signer{
-				Account: priv.GetScriptHash(),
-				Scopes:  transaction.CalledByEntry,
-			},
-			Account: acc,
-		},
-	})
-	require.NoError(t, err)
 
+	verifyWithoutParamsCtr, err := util.Uint160DecodeStringLE(verifyContractHash)
+	require.NoError(t, err)
+	acc1 := &wallet.Account{
+		Address: address.Uint160ToString(verifyWithoutParamsCtr),
+		Contract: &wallet.Contract{
+			Parameters: []wallet.ContractParam{},
+			Deployed:   true,
+		},
+		Locked:  true,
+		Default: false,
+	}
+
+	verifyWithParamsCtr, err := util.Uint160DecodeStringLE(verifyWithArgsContractHash)
+	require.NoError(t, err)
+	acc2 := &wallet.Account{
+		Address: address.Uint160ToString(verifyWithParamsCtr),
+		Contract: &wallet.Contract{
+			Parameters: []wallet.ContractParam{
+				{Name: "argString", Type: smartcontract.StringType},
+				{Name: "argInt", Type: smartcontract.IntegerType},
+				{Name: "argBool", Type: smartcontract.BoolType},
+			},
+			Deployed: true,
+		},
+		Locked:  true,
+		Default: false,
+	}
+
+	priv3 := testchain.PrivateKeyByID(3)
+	acc3 := wallet.NewAccountFromPrivateKey(priv3)
+
+	check := func(t *testing.T, h util.Uint256) {
 		mp := chain.GetMemPool()
 		tx, ok := mp.TryGetValue(h)
 		require.True(t, ok)
 		require.Equal(t, h, tx.Hash())
 		require.EqualValues(t, 30, tx.SystemFee)
+	}
+
+	t.Run("good", func(t *testing.T) {
+		t.Run("signer0: sig", func(t *testing.T) {
+			h, err := c.SignAndPushInvocationTx([]byte{byte(opcode.PUSH1)}, acc0, 30, 0, []client.SignerAccount{
+				{
+					Signer: transaction.Signer{
+						Account: priv0.GetScriptHash(),
+						Scopes:  transaction.CalledByEntry,
+					},
+					Account: acc0,
+				},
+			})
+			require.NoError(t, err)
+			check(t, h)
+		})
+		t.Run("signer0: sig; signer1: sig", func(t *testing.T) {
+			h, err := c.SignAndPushInvocationTx([]byte{byte(opcode.PUSH1)}, acc0, 30, 0, []client.SignerAccount{
+				{
+					Signer: transaction.Signer{
+						Account: priv0.GetScriptHash(),
+						Scopes:  transaction.CalledByEntry,
+					},
+					Account: acc0,
+				},
+				{
+					Signer: transaction.Signer{
+						Account: priv3.GetScriptHash(),
+						Scopes:  transaction.CalledByEntry,
+					},
+					Account: acc3,
+				},
+			})
+			require.NoError(t, err)
+			check(t, h)
+		})
+		t.Run("signer0: sig; signer1: contract-based paramless", func(t *testing.T) {
+			h, err := c.SignAndPushInvocationTx([]byte{byte(opcode.PUSH1)}, acc0, 30, 0, []client.SignerAccount{
+				{
+					Signer: transaction.Signer{
+						Account: priv0.GetScriptHash(),
+						Scopes:  transaction.CalledByEntry,
+					},
+					Account: acc0,
+				},
+				{
+					Signer: transaction.Signer{
+						Account: verifyWithoutParamsCtr,
+						Scopes:  transaction.CalledByEntry,
+					},
+					Account: acc1,
+				},
+			})
+			require.NoError(t, err)
+			check(t, h)
+		})
+	})
+	t.Run("error", func(t *testing.T) {
+		t.Run("signer0: sig; signer1: contract-based with params", func(t *testing.T) {
+			_, err := c.SignAndPushInvocationTx([]byte{byte(opcode.PUSH1)}, acc0, 30, 0, []client.SignerAccount{
+				{
+					Signer: transaction.Signer{
+						Account: priv0.GetScriptHash(),
+						Scopes:  transaction.CalledByEntry,
+					},
+					Account: acc0,
+				},
+				{
+					Signer: transaction.Signer{
+						Account: verifyWithParamsCtr,
+						Scopes:  transaction.CalledByEntry,
+					},
+					Account: acc2,
+				},
+			})
+			require.Error(t, err)
+		})
+		t.Run("signer0: sig; signer1: locked sig", func(t *testing.T) {
+			pk, err := keys.NewPrivateKey()
+			require.NoError(t, err)
+			acc4 := &wallet.Account{
+				Address: address.Uint160ToString(pk.GetScriptHash()),
+				Contract: &wallet.Contract{
+					Script:     pk.PublicKey().GetVerificationScript(),
+					Parameters: []wallet.ContractParam{{Name: "parameter0", Type: smartcontract.SignatureType}},
+				},
+			}
+			_, err = c.SignAndPushInvocationTx([]byte{byte(opcode.PUSH1)}, acc0, 30, 0, []client.SignerAccount{
+				{
+					Signer: transaction.Signer{
+						Account: priv0.GetScriptHash(),
+						Scopes:  transaction.CalledByEntry,
+					},
+					Account: acc0,
+				},
+				{
+					Signer: transaction.Signer{
+						Account: util.Uint160{1, 2, 3},
+						Scopes:  transaction.CalledByEntry,
+					},
+					Account: acc4,
+				},
+			})
+			require.Error(t, err)
+		})
+	})
 }
 
 func TestSignAndPushP2PNotaryRequest(t *testing.T) {

pkg/wallet/account.go

@@ -96,13 +96,13 @@ func NewAccount() (*Account, error) {
 
 // SignTx signs transaction t and updates it's Witnesses.
 func (a *Account) SignTx(net netmode.Magic, t *transaction.Transaction) error {
-	if a.privateKey == nil {
-		return errors.New("account is not unlocked")
-	}
 	if len(a.Contract.Parameters) == 0 {
 		t.Scripts = append(t.Scripts, transaction.Witness{})
 		return nil
 	}
+	if a.privateKey == nil {
+		return errors.New("account is not unlocked")
+	}
 	sign := a.privateKey.SignHashable(uint32(net), t)
 
 	verif := a.GetVerificationScript()