From eaf45d243b559f80a18ba2dc41ed7abed94e7748 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 20 Apr 2021 11:21:43 +0300 Subject: [PATCH 1/2] wallet: allow to sign with encrypted contract-based accounts --- pkg/wallet/account.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/wallet/account.go b/pkg/wallet/account.go index 66393f9ab..93f0c983f 100644 --- a/pkg/wallet/account.go +++ b/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() From 48ae1cc486dda0ad19453a84bcbc9ce350e7882e Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 20 Apr 2021 12:04:25 +0300 Subject: [PATCH 2/2] rpc: refactor SignAndPushInvocationTx We have a set of accounts provided via `cosigners` argument, so we should fill all transaction witnesses in (not only sender's witness). If we can't properly construct witnesses for all of the signers then an error should be returned. --- cli/contract_test.go | 45 +++++----- pkg/rpc/client/rpc.go | 28 +++++- pkg/rpc/server/client_test.go | 163 ++++++++++++++++++++++++++++++---- 3 files changed, 197 insertions(+), 39 deletions(-) diff --git a/cli/contract_test.go b/cli/contract_test.go index fffcb2d85..5e9202c9e 100644 --- a/cli/contract_test.go +++ b/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, diff --git a/pkg/rpc/client/rpc.go b/pkg/rpc/client/rpc.go index 884482c32..97deedfc1 100644 --- a/pkg/rpc/client/rpc.go +++ b/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 -// specified. It returns a hash of the invocation transaction and an error. +// transaction using given wif to sign it and given cosigners to cosign it if +// 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 { diff --git a/pkg/rpc/server/client_test.go b/pkg/rpc/server/client_test.go index 9aaa5d3df..e6014afd4 100644 --- a/pkg/rpc/server/client_test.go +++ b/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) - acc := wallet.NewAccountFromPrivateKey(priv) - 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) + priv0 := testchain.PrivateKeyByID(0) + acc0 := wallet.NewAccountFromPrivateKey(priv0) - mp := chain.GetMemPool() - tx, ok := mp.TryGetValue(h) - require.True(t, ok) - require.Equal(t, h, tx.Hash()) - require.EqualValues(t, 30, tx.SystemFee) + 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) {